[PATCH] D24651: [SCEV] Try harder to find UB for NSW/NUW instr

[Christof Douma via llvm-commits]

christof added a comment.

Let me elaborate a bit more on the UB of infinite loops. There existing code lists 2 cases of infinite loops triggered by poison:

1. A loop with side effects in it. This is easy as any side effect in the loop is control-dependent on poison.
2. A loop without side effects in it. The contested one.

The latter case is stated in the existing code now part of isAlwaysLatchControlDependentOnPoison() as being UB. The reasoning behind that could be: No side effect after the loop happens any more, this can be observed.

There is one part of this patch that I am not certain about. I have added code in isAlwaysLatchControlDependentOnPoison to look through some types of phi node. I've annotated it with an in-line comment.

I've noticed the discussion of Sanjoy on llvm-dev that tries to address the defect I attempt to fix here in a different way. However, I would still much appreciate some feedback on this work in the mean time.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:4989
@@ -4948,1 +4988,3 @@
         }
+      } else if (auto *PHI = dyn_cast<PHINode>(PoisonUser)) {
+          // Propagate through PHI nodes that only depend on 'Poison' and the
----------------
I am looking through PHI nodes. I am curious if I missed a case where this is not ok.


Repository:
  rL LLVM

https://reviews.llvm.org/D24651