[PATCH] D42417: Re-apply [SCEV] Fix isLoopEntryGuardedByCond usage

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

In https://reviews.llvm.org/D42417#998618, @mkazantsev wrote:

> I agree in general, just few notions.
>
> > It only look at the outermost operation of LHS and RHS. It should instead by checking if the LHS and RHS are *functions* of add recurrences.
>
> Even more, I think it's useful to have a helper function that returns the set of loops on which the current SCEV may depend. These are loops on which its SCEVAddRecs depend on.


There already is `SCEVInitRewriter`.  There may a `SCEVPostIncRewriter` too, but I'm not sure.  In any case, it should be easy to write.

>> In isKnownPredicate, find the deepest loops in LHS and RHS. Call then L_L and L_R. The invariant is that one's header has to dominate the other, or L_R == L_L.
> 
> It should not be deepest loops, but lowest by domination. For example:
> 
>   for (i1 = 0; i1 != e1; i1++) // L1
>     for (i2 = 0; i2 != e2; i2++) // L2
>       ...
>   for (i3 = 0; i3 != e3; i3++) // L3
>     ...
>    
> 
> If `LHS`  depends on `i1`, `i2` and `i3` then `L_L` should be the loop of `i3` (while the deepest is the loop of `i2`).

Yes, should have been "most dominated".  And we can assert that the dominates relation forms a total order on the loops.

> I think this is how implementation of this method should look like:
> 
>   isKnownPredicate(Pred, LHS, RHS) {
>     1. Collect set S all loops on which either LHS or RHS depend.
>     2. If S is non-empty
>       a. Let PD be the element of S which is dominated by all other elements of S
>       b. Let E(LHS) be value of LHS on entry of PD.
>          To get E(LHS), we should just take LHS and replace all AddRecs that are attached to PD on with their entry values.
>          Define E(RHS) in the same way.
>       c. Let B(LHS) be value of L on backedge of PD.
>          To get B(LHS), we should just take LHS and replace all AddRecs that are attached to PD on with their backedge values.
>          Define B(RHS) in the same way.
>       d. Note that E(LHS) and E(RHS) are automatically available on entry of PD, so we can assert on that.
>       e. Return true if isLoopEntryGuardedByCond(Pred, E(LHS), E(RHS)) && isLoopBackedgeGuardedByCond(Pred, B(LHS), B(RHS))
>     3. Return true if Pred, L, R is known from ranges, splitting etc.
>   }

SGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D42417