[PATCH] D17477: [Polly] [RFC] Allow the client of DependenceInfo to obtain dependences at different granularities.

[Hongbin Zheng via llvm-commits]

etherzhhb added inline comments.

================
Comment at: include/polly/DependenceInfo.h:58
@@ +57,3 @@
+
+    NUM_ANALYSIS_LEVELS
+  };
----------------
Meinersbur wrote:
> For consistent naming, please refer to the [[ http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | LLVM style guide ]]
will do

================
Comment at: lib/Analysis/DependenceInfo.cpp:797
@@ -775,3 +796,3 @@
 void DependenceInfo::getAnalysisUsage(AnalysisUsage &AU) const {
   ScopPass::getAnalysisUsage(AU);
 }
----------------
Meinersbur wrote:
> ScopPass::getAnalysisUsage(AU) 
> adds 
> addRequired<ScopInfo>();
> what you'd need with this change is
> addRequiredTransitive<ScopInfo>();
> 
> To avoid transitive requirements, I suggest per-granularity DependenceInfo passes:
> DependenceInfo(Dependences::AnalysisLevel) {} 
> ScopPass::getAnalysisUsage(AU) 
> adds 
> addRequired<ScopInfo>();
> what you'd need with this change is
> addRequiredTransitive<ScopInfo>();

I am confused, could you explain a little bit?

I guess even with the previous DependenceInfo, we also need 'addRequiredTransitive<ScopInfo>();'

Otherwise, why the previous DependenceInfo do not need this, but this change need 'addRequiredTransitive<ScopInfo>();'?

================
Comment at: lib/Analysis/DependenceInfo.cpp:797
@@ -775,3 +796,3 @@
 void DependenceInfo::getAnalysisUsage(AnalysisUsage &AU) const {
   ScopPass::getAnalysisUsage(AU);
 }
----------------
etherzhhb wrote:
> Meinersbur wrote:
> > ScopPass::getAnalysisUsage(AU) 
> > adds 
> > addRequired<ScopInfo>();
> > what you'd need with this change is
> > addRequiredTransitive<ScopInfo>();
> > 
> > To avoid transitive requirements, I suggest per-granularity DependenceInfo passes:
> > DependenceInfo(Dependences::AnalysisLevel) {} 
> > ScopPass::getAnalysisUsage(AU) 
> > adds 
> > addRequired<ScopInfo>();
> > what you'd need with this change is
> > addRequiredTransitive<ScopInfo>();
> 
> I am confused, could you explain a little bit?
> 
> I guess even with the previous DependenceInfo, we also need 'addRequiredTransitive<ScopInfo>();'
> 
> Otherwise, why the previous DependenceInfo do not need this, but this change need 'addRequiredTransitive<ScopInfo>();'?
> To avoid transitive requirements, I suggest per-granularity DependenceInfo passes:
> DependenceInfo(Dependences::AnalysisLevel) {}

This can also be done, but I need to figure out some implementation details.


Repository:
  rL LLVM

http://reviews.llvm.org/D17477