[PATCH] D140836: Do not short circuit hoistIVInc when recomputation of poison flags is needed.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:1044
+  if (SE.DT.dominates(IncV, InsertPos)) {
+    if (RecomputePoisonFlags) FixupPoisonFlags(IncV);
+    return true;
----------------
nit: Should not be on one line.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:1072
     I->moveBefore(InsertPos);
-    if (!RecomputePoisonFlags)
-      continue;
-    // Drop flags that are potentially inferred from old context and infer flags
-    // in new context.
-    I->dropPoisonGeneratingFlags();
-    if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
-      if (auto Flags = SE.getStrengthenedNoWrapFlagsFromBinOp(OBO)) {
-        auto *BO = cast<BinaryOperator>(I);
-        BO->setHasNoUnsignedWrap(
-            ScalarEvolution::maskFlags(*Flags, SCEV::FlagNUW) == SCEV::FlagNUW);
-        BO->setHasNoSignedWrap(
-            ScalarEvolution::maskFlags(*Flags, SCEV::FlagNSW) == SCEV::FlagNSW);
-      }
+    if (RecomputePoisonFlags) FixupPoisonFlags(I);
   }
----------------
Same


================
Comment at: llvm/test/Transforms/IndVarSimplify/iv-poison.ll:4
+
+define i2 @iv_hoist_nsw_poison(i2 %0) {
+; CHECK-LABEL: @iv_hoist_nsw_poison(
----------------
nikic wrote:
> Include a reference to PR59777 here (e.g. in a comment).
> 
> Instructions in tests should always be named, you can achieve this by running the test through `-passes=instnamer`.
Not addressed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140836/new/

https://reviews.llvm.org/D140836