[PATCH] D60935: [IndVarSimplify] Fixup nowrap flags during LFTR when moving to post-inc (PR31181)

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

I'm not sure if the current fix is correct.  I'd suggest doing one of the following:

- Remove LFTR and deal with the fallout somehow
- Change LFTR to strip out all no-wrap flags

We can lower the cost of the second option by moving LFTR to later in the optimization pipeline right before codegen since mostly mid level passes care about nowrap flags.



================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2382
+    // whatever it computed here.
+    if (auto *BO = dyn_cast<BinaryOperator>(CmpIndVar)) {
+      const SCEVAddRecExpr *AR = cast<SCEVAddRecExpr>(SE->getSCEV(CmpIndVar));
----------------
I don't think the no-wrap flags on the SCEV expression can be propagated to the increment operation like this. I wrote up some background here: https://www.playingwithpointers.com/blog/scev-integer-overflow.html but in short, say your pre-increment SCEV expression is `{S,+,X}` then the post-inc SCEV expression, `SE->getSCEV(CmpIndVar)` down below, is `{S+X,+,X}`.  Whether this is nsw/nuw has nothing to do with whether `S+X` can overflow -- all it says is that on all but the last iteration of the loop `Add(CmpIndVar,X)` will not overflow where `CmpIndVar` starts from `S+X` and is incremented by `X` on every iteration.

As a concrete example, say the pre-increment expression is `{-1,+,1}` and let's say the loop body executes 10 times.  Then the post-inc IV is `{0,+,1}`, which is both nuw and nsw (the value of the IV is < 10 and adding 1 to it does not overflow).  But you can't mark the increment operation as nuw because on the very first iteration it computes `Add(-1,1)` which unsigned overflows.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60935