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

[Sanjoy Das via llvm-commits]

If you won't be able to get to this soon do you mind adding a TODO or
FIXME to the isAvailableAtLoopEntry bailout (perhaps linking to a llvm
bugzilla bug containing the contents of this thread)? That way the
current state of the code will be a little less confusing.

-- Sanjoy

On Tue, Feb 6, 2018 at 1:50 PM, Sanjoy Das via Phabricator
<reviews at reviews.llvm.org> wrote:
> 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
>
>
>