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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

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.

> 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`).

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

For my code example above, we have three loops `L1`, `L2`, `L3` with AddRecs `{0,+,1}<L1>`, `{0,+,1}<L2>`, `{0,+,1}<L3>`. Let for example `LHS = {0,+,1}<L1> + {0,+,1}<L3>` and `RHS = {0,+,1}<L2> + {0,+,1}<L3>`. Accodring to the algorithm:

  isKnownPredicate(Pred, LHS, RHS) {
    1. Set S consists of loops L1, L2, L3.
    2. S is non-empty.
      a. PD is a loop of S which is dominated by all other loops, which happens to be L3.
      b. E(LHS) = E({0,+1}<L1> + {0,+,1}<L3>) = ({0,+,1}<L1> + 0. E(RHS) = E({0,+,1}<L2> + {0,+,1}<L3>) = {0,+,1}<L2> + 0.
          Hint: We've taken entry value of {0,+1}<L3> because it is depends on L3, and we took all other AddRecs as is because they do not.
  
      c.  B(LHS) = B({0,+1}<L1> + {0,+,1}<L3>) = {0,+,1}<L1> + {1,+,1}<L3>. B(RHS) = B({0,+,1}<L2> + {0,+,1}<L3>) = {0,+1}<L2>+ {1,+,1}<L3>.
         Hint: We've taken backedge value of {0,+,1}<L3> because it is depends on L3, and we took all other AddRecs as is because they do not.
  
      d. Assert that E(LHS) and E(RHS) are available at entry of L3.
      e. Return true if isLoopEntryGuardedByCond(Pred, E(LHS), E(RHS)) && isLoopBackedgeGuardedByCond(Pred, B(LHS), B(RHS))
    e. Check ranges etc.
    

How does it sound to you?


Repository:
  rL LLVM

https://reviews.llvm.org/D42417