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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 6 23:59:41 PDT 2016


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

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

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


http://reviews.llvm.org/D20560





More information about the llvm-commits mailing list