[PATCH] D145510: [SCEV] Fix finite loop non-strict predicate simplification (PR60944)

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 20:46:39 PDT 2023


mkazantsev added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:9172
+      break;
+    RHS = getAddExpr(getOne(RHS->getType()), RHS);
+    [[fallthrough]];
----------------
nikic wrote:
> mkazantsev wrote:
> > StephenFan wrote:
> > > mkazantsev wrote:
> > > > Can this overflow?
> > > Yes, it can. Although it can't overflow in loop `L`, it still may overflow somewhere out of the loop. See test file `pr60944.ll` below.
> > > In `loop2`, `%iv + 1` can not overflow as a result of `mustprogress` attribute, but `%iv + 1` can still overflow in `loop`.
> > I don't quite get the logic here. Imagine that the loops has two exits, one exit is `iv <= MAX_INT` (so it is never taken) and another exit is guaranteed to exit in 100 iterations (so the loop is finite). Then you just turn this first exit condition into `iv < MIN_INT` which is trivially false and immediately exit on the 1st iteration. To me it doesn't matter if the loop is finite or not, you cannot add one here.
> `ControllingFiniteLoop` is `ControlsExit && loopHasNoAbnormalExits(L) && loopIsFiniteByAssumption(L)`. This means that the lopo is finite *and* only has one exit.
I don't understand. Here is what `ControlsExit` is from the comment of `computeExitLimitFromCond`:
```
  /// \p ControlsExit is true if ExitCond directly controls the exit
  /// branch. In this case, we can assume that the loop exits only if the
  /// condition is true and can infer that failing to meet the condition prior
  /// to integer wraparound results in undefined behavior.
```
It doesn't mean that there is only one exit. It only means that, no matter what exit is taken, the condition is going to be true. How do you infer ability to add `1` and guarantee the new (non-equivalent) condition under the same circumstances?


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

https://reviews.llvm.org/D145510



More information about the llvm-commits mailing list