[PATCH] D38581: [IRCE] Fix intersection between signed and unsigned ranges

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev planned changes to this revision.
mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:900
-    // We are going to turn it on once the problems are fixed.
-    if (!IsSignedPredicate && !AllowUnsignedLatchCondition) {
-      FailureReason = "unsigned latch conditions are explicitly prohibited";
----------------
anna wrote:
> How about leaving this here to quickly find any problems in the future? 
Ok, I'll update the patch preserving the option (and set it to false by default).


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1753
+  auto RangeIsNonNegative = [&](InductiveRangeCheck::Range &R) {
+    return SE.isKnownNonNegative(R.getBegin()) &&
+           SE.isKnownNonNegative(R.getEnd());
----------------
anna wrote:
> Is this enough for proving range is non-negative?
> 
> Looking at this lambda alone with no context here, we can have [5, 2] and still have range wrap around.
> Is there a way to add an assert that getBegin < getEnd?
In `parseLoopStructure` we assert that the IV `HasNoSignedWrap`. Interesting detail is that currently it only checks for `nsw`. My guess is that we should check `nuw` for unsigned IVs instead. I will check with that and possibly add some tests.

But generally, we assume that the IV does not have wraparound, and ranges for range checks are also in canonical form from a smaller value to a greater value.


https://reviews.llvm.org/D38581