[PATCH] D39954: [IRCE] Smart range intersection

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1624
   //
   //   0 <= M + 1 * IndVar < L given L >= 0  (i.e. N == 1)
   //
----------------
mkazantsev wrote:
> anna wrote:
> > Could you please state here what exactly L stands for?
> Here L denotes right border of the safe iteration space of the given range check. If there is no such border (e.g. the check was `iv > low`), it is in fact `SINT_MAX + 1`, but we strengthen it to `SINT_MAX`.
> 
So, in the code below, L is the "EndLimit".


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1629
+  // IV's iteration space, limit the calculations by borders of the iteration
+  // space. For example, if IndVar is unsigned, (0 - M) overflows. If we figured
+  // out that "anything greater than (-M) is safe", we strengthen this to
----------------
"if IndVar is unsigned, (0 - M) overflows, when M > 0". 
The inequality is ` 0 <= M + IndVar`, so M can be 0 right..


================
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:
> 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?



================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1668
+      //   Rule 3: Y <s (X - SINT_MAX) ---> (X - SINT_MAX).
+      // It gives us smax(Y, X - SINT_MAX) to substract in either.
+      const SCEV *XMinusSIntMax = SE.getMinusSCEV(X, SIntMax);
----------------
"to subtract in all cases"


https://reviews.llvm.org/D39954