[PATCH] D39954: [IRCE] Smart range intersection

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1658
+           "We can only substract from values in [0; SINT_MAX]!");
+    if (IsLatchSigned) {
+      // X is a number from signed range, Y is interpreted as signed.
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > anna wrote:
> > > mkazantsev wrote:
> > > > anna wrote:
> > > > > To clarify: In both cases - signed and unsigned latch, the SCEV subtraction and rules you have below will always give us a value in range [0, SINT_MAX], i.e. we will never cross the signed border.
> > > > > Here we are just "strengthening" the range and using unambiguous values for range intersection later on IRCE. 
> > > > It is not true. If we substract `-5` from `SINT_MAX` in unsigned case, we have a value greater than `SINT_MAX`.
> > > err.. I'm not sure what I'm missing. In the case you state, X = SINT_MAX, and Y = -5, latch is unsigned.
> > > XMinusSIntMax = 0.
> > > Y s< XMinusSIntMax ( i.e. -5 s< 0), i.e. it satisfies Rule 3.
> > > So, what we are returning here is X - (XMinusSintMax) = X = SINT_MAX. 
> > > 
> > > 
> > > In other words, I think, if M = -5, and upperLimit = SINT_MAX, latch is unsigned, what we want the safe iteration space to be is [0, SINT_MAX + 5]. 
> > > So what am I missing?
> > > 
> > That `Y s< XMinusSIntMax ( i.e. -5 s< 0)` is a rule for signed latch.
> > 
> > Rules for unsigned latch are in `else` branch of this `if` :) `Y = -5` goes under `Rule 1: Y <s 0 ---> Y`. `SINT_MAX - (-5) = SINT_MAX + 5` which is still a positive value in unsigned iteration space.
> In other words, for `Y = -5`, Range Check range `[0, SINT_MAX)`, unsigned latch we have result `[5, SINT_MAX + 5)` according to `Rule 1` for unsigned latches.
ah I was only going by the signed rules :)
Now it makes sense. 
Could you please spell it out in the comment for the lambda?
"the subtract stays within the iteration space"  actually implies:
if latch is Signed, then X is treated as signed, and the subtract returns:
[0, SINT_MAX]
If latch is unsigned, X is treated as unsigned and subtract returns:
[0, UINT_MAX]
In other words, they will never overflow from the iteration space where X is proven to be. 
Or you could have this stated separately for the `isLatchSigned` and else case, like a summary for the rules.

Thanks.


https://reviews.llvm.org/D39954