[PATCH] D13177: [Bug 24848] Use range metadata to constant fold comparisons between two values

[Chen Li via llvm-commits]

chenli added inline comments.

================
Comment at: lib/Analysis/InstructionSimplify.cpp:2427
@@ +2426,3 @@
+        return ConstantInt::getTrue(RHS->getContext());
+      if (InversedSatisfied_CR.contains(LHS_CR))
+        return ConstantInt::getFalse(RHS->getContext());
----------------
sanjoy wrote:
> hfinkel wrote:
> > sanjoy wrote:
> > > hfinkel wrote:
> > > > sanjoy wrote:
> > > > > I don't think this second is correct.  `makeSatisfiedRegion` returns the range of LHS values that is *guaranteed* to satisfy the predicate "Pred RHS".  So if `RHS`'s range is `[2, 5)` and `Pred` is `ULT`, then `Satisfied_CR` is `[0, 2)`.  `InversedSatisfied_CR` will then be `[2, 0)` (i.e. all integers unsigned greater than or equal to `2`) and so you'll fold the equality to false if `LHS_CR` is `[3,4)`, even though in that case there is a value of `LHS` = `3` for which the equality is `true`.
> > > > > 
> > > > > I think for the negative check you need to do `InverseAllowed_CR.contains(LHS_CR)` where `InverseAllowed_CR` is the inverse of what is returned by `makeAllowedRegion`.
> > > > I don't understand your example. If we have:
> > > > 
> > > >   x <u 2
> > > > 
> > > > then Satisfied_CR is indeed [0, 2), and the inverse (x >=u 2) should indeed be [2, 0). But if LHS_CR is [3, 4), then the original predicate is false, because 3 is not less than 2.
> > > > 
> > > I'm talking about a case where `X u< Y` and `X`'s range is `[3, 4)`, and `Y`'s range is `[2, 5)`.  Then the code as it is now will fold the condition to `false` (because `[3,4)` is contained in `[2, 0)`) when it //could// be `true` if `X` is `3` and `Y` is `4`.
> > > 
> > > This code is basically checking no `X` is *guaranteed* to satisfy `Pred Y`, i.e. there is no such `X` such that `X Pred Y` is true for *all* Y, when it should be trying to prove that there is no such `X` such that `X Pred Y` for *some* Y.
> > My thought had been that in this case, we have X <u Y, so the Satisfied_CR is [0, 2), and the inverse X >=u Y, has InversedSatisfied_CR of [4, 0), because only if X is >= 4 do we know it satisfies the inequality for all possible values of Y. If X is [3, 4), then it is not contained in either set.
> > 
> I was misreading the code. :/  Sorry for the noise.  What's here is fine.
@hfinkel - Thanks for explaining that for me :)


http://reviews.llvm.org/D13177