[PATCH] D20560: [new PM] port LoopAccessAnalysis to new pass manager (part-1)

[Xinliang David Li via llvm-commits]

On Mon, Jun 6, 2016 at 11:59 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> chandlerc added a comment.
>
> Thanks, this definitely helps.
>
> In http://reviews.llvm.org/D20560#450661, @davidxl wrote:
>
> > New summary of the change:
> >
> > 1. split the legacy LoopAccess Analysis pass out of the header file
> LoopAccessAnalysis.h
>
>

>
> Any particular reason to split these apart? It would seem simpler to just
> do the refactoring in place in the header file...
>
>
The file LoopAccessAnalysis.h is pretty big so I split out parts that are
relevant to pass manager to a new file (where the new pass  will also
reside) to reduce dependency.  I don't have strong preference. If you think
it is better to keep them together, I can make the change.



> > 2. introduced a new class LoopAccessFuncInfo that represents the
> function level analysis results that is cacheble.
>
>
> Makes sense. I'd probably call it LoopAccessInfo to fit with the common
> pattern, but I'm happy if there are reasons to name this differently.
>
>
LoopAccessInfo name is already taken for the loop level data structure. I
can change it to LoopAccessLoopInfo if you think it is better.



> > 3. move the interfaces that access the loop level LoopAccessInfo from
> legacy pass class to the new LoopAccessFuncInfo class
>
> > 4. change clients to use the new interfaces
>
> >
>
> >   Does it make sense?
>
>
> Yep. See one question on the code I have initially below...
>
>
> ================
> Comment at: include/llvm/Analysis/LoopAccessInfoAnalysis.h:36-45
> @@ +35,12 @@
> +
> +// Analysis manager base
> +class LoopAccessAMBase {
> +public:
> +  virtual TargetLibraryInfo *getTLI() = 0;
> +  virtual AliasAnalysis *getAAResults() = 0;
> +  virtual DominatorTree *getDominatorTree() = 0;
> +  virtual LoopInfo *getLoopInfo() = 0;
> +  virtual ScalarEvolution *getSCEV() = 0;
> +  virtual ~LoopAccessAMBase() {}
> +};
> +
> ----------------
> Why this pattern?
>
> Specifically, why deviate from the pattern that other analyses that have
> been ported use where you just construct the result class with pointers to
> the various other analysis results they reference?
>
> Introducing a complete abstract interface seems like a much heavier weight
> abstraction; unless there is a specific problem solved, I'd rather the
> lighter weight abstraction.
>

The problem is that this info depends on 5 different analyses, so I thought
encapsulate them into one place make it cleaner
1) one centralized place for finding the informatin
2) make the creator interface slightly cleaner.

I don't have strong preference one way or the other.

thanks,

David


>
>
> http://reviews.llvm.org/D20560
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160607/8f2c6ebc/attachment.html>