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

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:929
 
-    if (!HasNoSignedWrap(AR))
-      return false;
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > Actually I look into it and the old implementation seems suspicious to me. We only check `nsw`, but when we don't have `nuw` and actually do have an unsigned wrap, why don't we have a bug on unsigned latch predicates? Or we do, but not aware about?.. :)
> If you look at `HasNoSignedWrap`, it has a bit more complex check than just looking into the immediate flag, it also tries to go through `sext` to find this flag. Can we do the same for `nuw` and `zext` to keep things consistent?
Is this case handled on line 915?


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:929
+    if (HasNoSignedWrap(AR) ||
+        (!IsSigned && AR->hasNoUnsignedWrap())) {
 
----------------
samparker wrote:
> mkazantsev wrote:
> > mkazantsev wrote:
> > > Actually I look into it and the old implementation seems suspicious to me. We only check `nsw`, but when we don't have `nuw` and actually do have an unsigned wrap, why don't we have a bug on unsigned latch predicates? Or we do, but not aware about?.. :)
> > If you look at `HasNoSignedWrap`, it has a bit more complex check than just looking into the immediate flag, it also tries to go through `sext` to find this flag. Can we do the same for `nuw` and `zext` to keep things consistent?
> Is this case handled on line 915?
Ok, I'll make some changes. I also need to handle equality differently, because I believe its classed as unsigned which messes up the NoSignedWrap check.


https://reviews.llvm.org/D45439