[PATCH] D45439: [IRCE] Use NUW flag for indvar

Sam Parker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 16 02:53:00 PDT 2018


samparker added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:894
+  auto HasNoWrap = [&](const SCEVAddRecExpr *AR) {
+    if (AR->hasNoSelfWrap())
       return true;
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > This is incorrect. If counter value goes from `-1` to `(SINT_MAX + 1)` (both included), it has both signed and unsigned wrap and has no self-wrap.
> Just to clarify the example: we start with `i = -1` and go up with step of `1` and exit the loop when `i = SINT_MAX + 1`. In this case when we pass from `-1` to `0` we are making an unsigned wrap and when we pass from `SINT_MAX` to `SINT_MAX + 1` we are making a signed wrap. There is no self-wrap because this variable will never reach `-1` again.
>From the SCEV header comments, I understand that NoSelfWrap can be used to query whether either nsw or nuw has been set.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:907
 
-      if (NoSignedWrap)
-        return true;
-    }
+    const SCEV *ExtendedStart = SE.getSignExtendExpr(AR->getStart(), WideTy);
+    const SCEV *ExtendedStep =
----------------
mkazantsev wrote:
> Do you have a plan to do the same for `zext`? I'm OK if it's in a follow-up patch.
Yes, happy to do that in a follow-up.


https://reviews.llvm.org/D45439





More information about the llvm-commits mailing list