From: Chandler Carruth Date: Thu, 21 Nov 2013 10:53:05 +0000 (+0000) Subject: [PM] Widen the interface for invalidate on an analysis result now that X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=edd2b4913433d92fa6b6b89c31a2837d1bba32a5 [PM] Widen the interface for invalidate on an analysis result now that it is completely optional, and sink the logic for handling the preserved analysis set into it. This allows us to implement the delegation logic desired in the proxy module analysis for the function analysis manager where if the proxy itself is preserved we assume the set of functions hasn't changed and we do a fine grained invalidation by walking the functions in the module and running the invalidate for them all at the manager level and letting it try to invalidate any passes. This in turn makes it blindingly obvious why we should hoist the invalidate trait and have two collections of results. That allows handling invalidation for almost all analyses without indirect calls and it allows short circuiting when the preserved set is all. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@195338 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/IR/PassManager.h b/include/llvm/IR/PassManager.h index a37fc8f2f93..c930f7b9252 100644 --- a/include/llvm/IR/PassManager.h +++ b/include/llvm/IR/PassManager.h @@ -185,11 +185,15 @@ template struct AnalysisResultConcept { /// \brief Method to try and mark a result as invalid. /// - /// When the outer \c AnalysisManager detects a change in some underlying + /// When the outer analysis manager detects a change in some underlying /// unit of the IR, it will call this method on all of the results cached. /// - /// \returns true if the result should indeed be invalidated (the default). - virtual bool invalidate(IRUnitT *IR) = 0; + /// This method also receives a set of preserved analyses which can be used + /// to avoid invalidation because the pass which changed the underlying IR + /// took care to update or preserve the analysis result in some way. + /// + /// \returns true if the result is indeed invalid (the default). + virtual bool invalidate(IRUnitT *IR, const PreservedAnalyses &PA) = 0; }; /// \brief Wrapper to model the analysis result concept. @@ -198,19 +202,22 @@ template struct AnalysisResultConcept { /// implementation so that the actual analysis result doesn't need to provide /// an invalidation handler. It is only selected when the invalidation handler /// is not part of the ResultT's interface. -template +template struct AnalysisResultModel : AnalysisResultConcept { AnalysisResultModel(ResultT Result) : Result(llvm_move(Result)) {} virtual AnalysisResultModel *clone() { return new AnalysisResultModel(Result); } - /// \brief The model returns true to allow the invalidation. + /// \brief The model bases invalidation soley on being in the preserved set. // // FIXME: We should actually use two different concepts for analysis results // rather than two different models, and avoid the indirect function call for // ones that use the trivial behavior. - virtual bool invalidate(IRUnitT *) { return true; } + virtual bool invalidate(IRUnitT *, const PreservedAnalyses &PA) { + return !PA.preserved(PassT::ID()); + } ResultT Result; }; @@ -219,15 +226,18 @@ struct AnalysisResultModel : AnalysisResultConcept { /// /// Can wrap any type which implements a suitable invalidate member and model /// the AnalysisResultConcept for the AnalysisManager. -template -struct AnalysisResultModel : AnalysisResultConcept { +template +struct AnalysisResultModel : AnalysisResultConcept { AnalysisResultModel(ResultT Result) : Result(llvm_move(Result)) {} virtual AnalysisResultModel *clone() { return new AnalysisResultModel(Result); } /// \brief The model delegates to the \c ResultT method. - virtual bool invalidate(IRUnitT *IR) { return Result.invalidate(IR); } + virtual bool invalidate(IRUnitT *IR, const PreservedAnalyses &PA) { + return Result.invalidate(IR, PA); + } ResultT Result; }; @@ -237,7 +247,10 @@ struct AnalysisResultModel : AnalysisResultConcept class ResultHasInvalidateMethod { typedef char SmallType; struct BigType { char a, b; }; - template struct Checker; + + template + struct Checker; + template static SmallType f(Checker *); template static BigType f(...); @@ -274,7 +287,7 @@ struct AnalysisPassModel : AnalysisPassConcept { // FIXME: Replace PassT::Result with type traits when we use C++11. typedef AnalysisResultModel< - IRUnitT, typename PassT::Result, + IRUnitT, PassT, typename PassT::Result, ResultHasInvalidateMethod::Value> ResultModelT; @@ -363,7 +376,7 @@ public: const detail::AnalysisResultConcept &ResultConcept = getResultImpl(PassT::ID(), M); typedef detail::AnalysisResultModel< - Module, typename PassT::Result, + Module, PassT, typename PassT::Result, detail::ResultHasInvalidateMethod< Module, typename PassT::Result>::Value> ResultModelT; return static_cast(ResultConcept).Result; @@ -447,7 +460,7 @@ public: const detail::AnalysisResultConcept &ResultConcept = getResultImpl(PassT::ID(), F); typedef detail::AnalysisResultModel< - Function, typename PassT::Result, + Function, PassT, typename PassT::Result, detail::ResultHasInvalidateMethod< Function, typename PassT::Result>::Value> ResultModelT; return static_cast(ResultConcept).Result; @@ -591,7 +604,16 @@ public: ~Result(); /// \brief Handler for invalidation of the module. - bool invalidate(Module *M); + /// + /// If this analysis itself is preserved, then we assume that the set of \c + /// Function objects in the \c Module hasn't changed and thus we don't need + /// to invalidate *all* cached data associated with a \c Function* in the \c + /// FunctionAnalysisManager. + /// + /// Regardless of whether this analysis is marked as preserved, all of the + /// analyses in the \c FunctionAnalysisManager are potentially invalidated + /// based on the set of preserved analyses. + bool invalidate(Module *M, const PreservedAnalyses &PA); private: FunctionAnalysisManager &FAM; @@ -623,6 +645,9 @@ public: PreservedAnalyses PassPA = Pass.run(I); PA.intersect(llvm_move(PassPA)); } + + // By definition we preserve the proxy. + PA.preserve(); return PA; } diff --git a/lib/IR/PassManager.cpp b/lib/IR/PassManager.cpp index 50ace5880a5..18388dc0c5a 100644 --- a/lib/IR/PassManager.cpp +++ b/lib/IR/PassManager.cpp @@ -29,7 +29,7 @@ void ModuleAnalysisManager::invalidate(Module *M, const PreservedAnalyses &PA) { for (ModuleAnalysisResultMapT::iterator I = ModuleAnalysisResults.begin(), E = ModuleAnalysisResults.end(); I != E; ++I) - if (!PA.preserved(I->first) && I->second->invalidate(M)) + if (I->second->invalidate(M, PA)) ModuleAnalysisResults.erase(I); } @@ -76,7 +76,7 @@ void FunctionAnalysisManager::invalidate(Function *F, const PreservedAnalyses &P for (FunctionAnalysisResultListT::iterator I = ResultsList.begin(), E = ResultsList.end(); I != E;) - if (!PA.preserved(I->first) && I->second->invalidate(F)) { + if (I->second->invalidate(F, PA)) { InvalidatedPassIDs.push_back(I->first); I = ResultsList.erase(I); } else { @@ -145,11 +145,19 @@ FunctionAnalysisModuleProxy::Result::~Result() { FAM.clear(); } -bool FunctionAnalysisModuleProxy::Result::invalidate(Module *M) { - // FIXME: We should pull the preserved analysis set into the invalidation - // handler so that we can detect when there is no need to clear the entire - // function analysis manager. - FAM.clear(); +bool FunctionAnalysisModuleProxy::Result::invalidate(Module *M, const PreservedAnalyses &PA) { + // If this proxy isn't marked as preserved, then it is has an invalid set of + // Function objects in the cache making it impossible to incrementally + // preserve them. Just clear the entire manager. + if (!PA.preserved(ID())) { + FAM.clear(); + return false; + } + + // The set of functions was preserved some how, so just directly invalidate + // any analysis results not preserved. + for (Module::iterator I = M->begin(), E = M->end(); I != E; ++I) + FAM.invalidate(I, PA); // Return false to indicate that this result is still a valid proxy. return false; diff --git a/unittests/IR/PassManagerTest.cpp b/unittests/IR/PassManagerTest.cpp index 38ef1a43290..d23d85b8828 100644 --- a/unittests/IR/PassManagerTest.cpp +++ b/unittests/IR/PassManagerTest.cpp @@ -64,6 +64,20 @@ struct TestModulePass { int &RunCount; }; +struct TestPreservingModulePass { + PreservedAnalyses run(Module *M) { + return PreservedAnalyses::all(); + } +}; + +struct TestMinPreservingModulePass { + PreservedAnalyses run(Module *M) { + PreservedAnalyses PA; + PA.preserve(); + return PA; + } +}; + struct TestFunctionPass { TestFunctionPass(FunctionAnalysisManager &AM, int &RunCount, int &AnalyzedInstrCount) @@ -137,6 +151,22 @@ TEST_F(PassManagerTest, Basic) { FPM2.addPass(TestFunctionPass(FAM, FunctionPassRunCount2, AnalyzedInstrCount2)); MPM.addPass(createModuleToFunctionPassAdaptor(FPM2, &MAM)); + // A third function pass manager but with only preserving intervening passes. + MPM.addPass(TestPreservingModulePass()); + FunctionPassManager FPM3(&FAM); + int FunctionPassRunCount3 = 0; + int AnalyzedInstrCount3 = 0; + FPM3.addPass(TestFunctionPass(FAM, FunctionPassRunCount3, AnalyzedInstrCount3)); + MPM.addPass(createModuleToFunctionPassAdaptor(FPM3, &MAM)); + + // A fourth function pass manager but with a minimal intervening passes. + MPM.addPass(TestMinPreservingModulePass()); + FunctionPassManager FPM4(&FAM); + int FunctionPassRunCount4 = 0; + int AnalyzedInstrCount4 = 0; + FPM4.addPass(TestFunctionPass(FAM, FunctionPassRunCount4, AnalyzedInstrCount4)); + MPM.addPass(createModuleToFunctionPassAdaptor(FPM4, &MAM)); + MPM.run(M.get()); // Validate module pass counters. @@ -147,8 +177,12 @@ TEST_F(PassManagerTest, Basic) { EXPECT_EQ(5, AnalyzedInstrCount1); EXPECT_EQ(3, FunctionPassRunCount2); EXPECT_EQ(5, AnalyzedInstrCount2); + EXPECT_EQ(3, FunctionPassRunCount3); + EXPECT_EQ(5, AnalyzedInstrCount3); + EXPECT_EQ(3, FunctionPassRunCount4); + EXPECT_EQ(5, AnalyzedInstrCount4); // Validate the analysis counters. - EXPECT_EQ(6, AnalysisRuns); + EXPECT_EQ(9, AnalysisRuns); } }