[PATCH] D44515: [IRCE] Change min value safety check

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:705
 
 static bool CanBeMin(ScalarEvolution &SE, const SCEV *S, bool Signed) {
   APInt Min = Signed ?
----------------
mkazantsev wrote:
> Can we now remove this function and replace its usages with `LoopGuardedAgainstMin`? I don't mind if it goes as a follow-up change.
I will have a look at the other uses.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:908
+        // IndVarStart is non negative and we're increasing with no wrap.
+        if (SE.isKnownNonNegative(IndVarStart) &&
+            IndVarBase->getNoWrapFlags(SCEV::FlagNUW) &&
----------------
mkazantsev wrote:
> Why do you need non-negativity here? In unsigned, "negative" only means that the first bit is 1 and the value is still seen as big positive.
Ah, guess I was trying to be overly cautious, FlagNUW will do.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:991
         Pred = ICmpInst::ICMP_SGT;
       else if (Pred == ICmpInst::ICMP_EQ && LatchBrExitIdx == 0 &&
                !CanBeMax(SE, RightSCEV, /* IsSignedPredicate */ true)) {
----------------
mkazantsev wrote:
> Do you plan to do the same for max value? I'm OK if it will be done in a separate patch.
Yes, I can do that.


================
Comment at: test/Transforms/IRCE/eq_ne.ll:147
 entry:
-  %len = load i32, i32* %a_len_ptr, !range !0
   br label %loop
----------------
mkazantsev wrote:
> What is the reason of this change? If it demonstrates something new you can just create a new test identical to this one but without the range.
>From the description, the existing test didn't seem like it was testing what it should have been. Given the range values, IRCE should be able to be performed and with this patch it does. So I wanted to change the test to match up with the description with IRCE being prevented.


https://reviews.llvm.org/D44515