[PATCH] D39954: [IRCE] Smart range intersection

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1624
   //
   //   0 <= M + 1 * IndVar < L given L >= 0  (i.e. N == 1)
   //
----------------
anna wrote:
> 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".
Yes, I will rename it to make it more clear.


================
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
----------------
anna wrote:
> "if IndVar is unsigned, (0 - M) overflows, when M > 0". 
> The inequality is ` 0 <= M + IndVar`, so M can be 0 right..
Right, thanks for pointing out.


================
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.
----------------
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.


https://reviews.llvm.org/D39954