[PATCH] D28627: [PM] Introduce an analysis set used to preserve all analyses over a function's CFG when that CFG is unchanged.
Davide Italiano via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 13 10:37:11 PST 2017
davide added inline comments.
================
Comment at: lib/Analysis/BlockFrequencyInfo.cpp:135-143
+bool BlockFrequencyInfo::invalidate(Function &F, const PreservedAnalyses &PA,
+ FunctionAnalysisManager::Invalidator &) {
+ // Check whether the analysis, all analyses on functions, or the function's
+ // CFG have been preserved.
+ auto PAC = PA.getChecker<BlockFrequencyAnalysis>();
+ return !(PAC.preserved() || PAC.preservedSet<AllAnalysesOn<Function>>() ||
+ PAC.preservedSet<AnalysesOnFunctionCFG>());
----------------
jlebar wrote:
> davide wrote:
> > The class of `invalidate` functions share a decent amount of code, maybe we can factor it out somehow?
> The way I think of it, this here is how we list the things that preserve BlockFrequencyInfo. "Factoring out" the code somehow would mean we changed the invalidator API somehow.
>
> When I did an earlier review, I suggested not even calling invalidate() if we know a priori that the pass is preserved (I guess that would correspond to `PAC.preserved() || PAC.preservedSet<AllAnalysesOn<IRUnitT>>())`). Chandler explained at the time why this wouldn't work. I don't quite remember, but IIRC the problem isn't with analyses but with transformation passes, which want to do work inside `invalidate()`. I think it's a decent simplification if we keep the analysis and transformation invalidate functions close together, even if it means we have to write a bit more boilerplate.
Fair enough.
https://reviews.llvm.org/D28627
More information about the llvm-commits
mailing list