[llvm] r268452 - PM: Port LoopRotation to the new loop pass manager

[Sean Silva via llvm-commits]

On Tue, May 10, 2016 at 11:27 AM, Justin Bogner <mail at justinbogner.com>
wrote:

> Sean Silva <chisophugis at gmail.com> writes:
> > On Tue, May 3, 2016 at 3:02 PM, Justin Bogner via llvm-commits <
> > llvm-commits at lists.llvm.org> wrote:
> >
> >> Author: bogner
> >> Date: Tue May  3 17:02:31 2016
> >> New Revision: 268452
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=268452&view=rev
> >> Log:
> >> PM: Port LoopRotation to the new loop pass manager
>  ...
> >> --- llvm/trunk/test/Transforms/LoopRotate/basic.ll (original)
> >> +++ llvm/trunk/test/Transforms/LoopRotate/basic.ll Tue May  3 17:02:31
> 2016
> >> @@ -1,4 +1,6 @@
> >>  ; RUN: opt -S -loop-rotate < %s | FileCheck %s
> >> +; RUN: opt -S
> >>
> -passes='require<loops>,require<targetir>,require<assumptions>,loop(rotate)'
> >> < %s | FileCheck %s
> >>
> >
> > Sorry if this is a stupid question, but why do we need to explicitly
> > "require" the passes when the loop-rotate already declares the
> dependency?
> > (I feel like there's some part of the bigger picture that I'm missing
> here)
>
> To be clear, there are no dependencies in the new pass manager - passes
> request analysis results from an analysis manager and they're calculated
> or returned from the cache as appropriate. Naturally, the next question
> one would ask is "okay then, why aren't these calculated on demand when
> loop-rotate asks for them?".
>

Ah, yeah. The whole "schedule vs. cache" is one of the main differences of
the new PM.


>
> The mechanical answer is that loop-rotate uses the `getCachedResult`
> API, not the `getResult` one - the difference being that
> `getCachedResult` only returns results if they're already calculated,
> and null otherwise. Hence, without the require<> in the test, there are
> no results.


Ah, makes perfect sense!


> This of course leads to "why does loop-rotate use
> getCachedResult?"
>
> It has to. These analyses are function analyses, and a loop pass isn't
> allowed to cause a function analysis to run, much like a function pass
> isn't allowed to cause a module analysis to be run. They have to stick
> to their own level. This enforces correct layering and acts as a
> safeguard against accidentally doing extra work.
>
> That said, the "require<loops>" is redundant, since a LoopPassManager
> can't operate without having calculated that. There may also be an
> argument to be made that the LPM should implicitly calculate some other
> analyses, but we'd have to be careful not to do too much.
>

Makes sense. But then `+  assert((LI && TTI && AC) && "Analyses for loop
rotation not available");` is not really appropriate, right? Can't we hit
that assert just by passing the right arguments to opt? (by dropping the
`require<*>` analyses from the passes= specification?)

(sorry, I'm away from the office and don't have an easy way to test locally)

-- Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160510/2f5f3f37/attachment.html>