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

[Serguei Katkov via llvm-commits]

Hi Sanjoy, this task is in my TODO list.

My plan was to start this work this week. You know, plan is subject to change :) but I still plan to start it this week :)

Thank you,
Serguei.

> -----Original Message-----
> From: Sanjoy Das [mailto:sanjoy at playingwithpointers.com]
> Sent: Tuesday, February 20, 2018 7:49 AM
> To: reviews+D42417+public+54a7b9b7b0be118c at reviews.llvm.org
> Cc: Serguei Katkov <serguei.katkov at azul.com>; Maxim Kazantsev
> <max.kazantsev at azul.com>; Anna Thomas <anna at azul.com>;
> dorit.nuzman at intel.com; Philip Reames <listmail at philipreames.com>; llvm-
> commits <llvm-commits at lists.llvm.org>; Junbum Lim
> <junbuml at codeaurora.org>; Wei Mi <wmi at google.com>;
> florian.hahn at arm.com
> Subject: Re: [PATCH] D42417: Re-apply [SCEV] Fix
> isLoopEntryGuardedByCond usage
> 
> 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
> >
> >
> >