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

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added reviewers: fhahn, mkazantsev.
nikic added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:1029
                               bool RecomputePoisonFlags) {
-  if (SE.DT.dominates(IncV, InsertPos))
+  if (SE.DT.dominates(IncV, InsertPos) && !RecomputePoisonFlags)
       return true;
----------------
We can't just fall through to the code below if RecomputePoisonFlags is set. This both does more than we need, and I believe it will produce broken IR in some cases, because it may end up moving the instruction down, making it no longer dominate other users.

I believe what you'd want to do is extract the poison flag handling code below and then call it in this short-circuit code path as well.


================
Comment at: llvm/test/Transforms/IndVarSimplify/iv-poison.ll:4
+
+define i2 @iv_hoist_nsw_poison(i2 %0) {
+; CHECK-LABEL: @iv_hoist_nsw_poison(
----------------
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`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140836