diff --git a/llvm/include/llvm/Analysis/CGSCCPassManager.h b/llvm/include/llvm/Analysis/CGSCCPassManager.h --- a/llvm/include/llvm/Analysis/CGSCCPassManager.h +++ b/llvm/include/llvm/Analysis/CGSCCPassManager.h @@ -327,7 +327,8 @@ /// within this run safely. template class ModuleToPostOrderCGSCCPassAdaptor - : public PassInfoMixin> { + : public RequiredPassInfoMixin< + ModuleToPostOrderCGSCCPassAdaptor> { public: explicit ModuleToPostOrderCGSCCPassAdaptor(CGSCCPassT Pass) : Pass(std::move(Pass)) {} @@ -444,7 +445,7 @@ /// within this run safely. template class CGSCCToFunctionPassAdaptor - : public PassInfoMixin> { + : public RequiredPassInfoMixin> { public: explicit CGSCCToFunctionPassAdaptor(FunctionPassT Pass) : Pass(std::move(Pass)) {} diff --git a/llvm/include/llvm/IR/PassInstrumentation.h b/llvm/include/llvm/IR/PassInstrumentation.h --- a/llvm/include/llvm/IR/PassInstrumentation.h +++ b/llvm/include/llvm/IR/PassInstrumentation.h @@ -44,10 +44,6 @@ /// of a pass. For those callbacks returning false means pass will not be /// executed. /// -/// TODO: currently there is no way for a pass to opt-out of execution control -/// (e.g. become unskippable). PassManager is the only entity that determines -/// how pass instrumentation affects pass execution. -/// //===----------------------------------------------------------------------===// #ifndef LLVM_IR_PASSINSTRUMENTATION_H @@ -148,6 +144,8 @@ bool ShouldRun = true; for (auto &C : Callbacks->BeforePassCallbacks) ShouldRun &= C(Pass.name(), llvm::Any(&IR)); + // Required passes should always be run. + ShouldRun |= Pass.isRequired(); return ShouldRun; } diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h --- a/llvm/include/llvm/IR/PassManager.h +++ b/llvm/include/llvm/IR/PassManager.h @@ -370,6 +370,7 @@ /// passes. /// /// This provides some boilerplate for types that are passes. +/// Use this for optional (usually optimization) passes. template struct PassInfoMixin { /// Gets the name of the pass we are mixed into. static StringRef name() { @@ -380,6 +381,20 @@ Name = Name.drop_front(strlen("llvm::")); return Name; } + + /// This pass is not required to produce a valid executable. + /// Use RequiredPassInfoMixin for required passes. + static bool isRequired() { return false; } +}; + +/// A CRTP mix-in to automatically provide informational APIs needed for +/// passes. +/// +/// Use this for passes required to produce a valid executable. +template +struct RequiredPassInfoMixin : PassInfoMixin { + /// This pass is required to produce a valid executable. + static bool isRequired() { return true; } }; /// A CRTP mix-in that provides informational APIs needed for analysis passes. @@ -462,7 +477,7 @@ template , typename... ExtraArgTs> -class PassManager : public PassInfoMixin< +class PassManager : public RequiredPassInfoMixin< PassManager> { public: /// Construct a pass manager. @@ -1207,7 +1222,7 @@ /// may be stale. Function analyses will not be stale. template class ModuleToFunctionPassAdaptor - : public PassInfoMixin> { + : public RequiredPassInfoMixin> { public: explicit ModuleToFunctionPassAdaptor(FunctionPassT Pass) : Pass(std::move(Pass)) {} diff --git a/llvm/include/llvm/IR/PassManagerInternal.h b/llvm/include/llvm/IR/PassManagerInternal.h --- a/llvm/include/llvm/IR/PassManagerInternal.h +++ b/llvm/include/llvm/IR/PassManagerInternal.h @@ -48,6 +48,11 @@ /// Polymorphic method to access the name of a pass. virtual StringRef name() const = 0; + + /// Whether or not this pass is required to create executable code. + /// + /// Used for optnone and opt-bisect. + virtual bool isRequired() const = 0; }; /// A template wrapper used to implement the polymorphic API. @@ -81,6 +86,8 @@ StringRef name() const override { return PassT::name(); } + bool isRequired() const override { return PassT::isRequired(); } + PassT Pass; }; diff --git a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h --- a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h +++ b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h @@ -229,7 +229,7 @@ /// LoopAnalysisManager to be used within this run safely. template class FunctionToLoopPassAdaptor - : public PassInfoMixin> { + : public RequiredPassInfoMixin> { public: explicit FunctionToLoopPassAdaptor(LoopPassT Pass, bool UseMemorySSA = false, bool DebugLogging = false) diff --git a/llvm/unittests/IR/PassBuilderCallbacksTest.cpp b/llvm/unittests/IR/PassBuilderCallbacksTest.cpp --- a/llvm/unittests/IR/PassBuilderCallbacksTest.cpp +++ b/llvm/unittests/IR/PassBuilderCallbacksTest.cpp @@ -524,10 +524,10 @@ // Non-mock instrumentation run here can safely be ignored. CallbacksHandle.ignoreNonMockPassInstrumentation(""); - // Skip the pass by returning false. - EXPECT_CALL(CallbacksHandle, runBeforePass(HasNameRegex("MockPassHandle"), - HasName(""))) - .WillOnce(Return(false)); + // Skip all passes by returning false. Pass managers and adaptor passes are + // also passes that observed by the callbacks. + EXPECT_CALL(CallbacksHandle, runBeforePass(_, _)) + .WillRepeatedly(Return(false)); EXPECT_CALL(AnalysisHandle, run(HasName(""), _)).Times(0); EXPECT_CALL(PassHandle, run(HasName(""), _)).Times(0); @@ -543,7 +543,60 @@ runAfterAnalysis(HasNameRegex("MockAnalysisHandle"), _)) .Times(0); - StringRef PipelineText = "test-transform"; + // Order is important here. `Adaptor` expectations should be checked first + // because the its argument contains 'PassManager' (for example: + // ModuleToFunctionPassAdaptor{{.*}}PassManager{{.*}}). Here only check + // `runAfterPass` to show that they are not skipped. + + // Pass managers are not ignored. + // 5 = (1) ModulePassManager + (2) FunctionPassMangers + (1) LoopPassManager + + // (1) CGSCCPassManager + EXPECT_CALL(CallbacksHandle, runAfterPass(HasNameRegex("PassManager"), _)) + .Times(5); + EXPECT_CALL(CallbacksHandle, + runAfterPass(HasNameRegex("ModuleToFunctionPassAdaptor"), _)) + .Times(1); + EXPECT_CALL( + CallbacksHandle, + runAfterPass(HasNameRegex("ModuleToPostOrderCGSCCPassAdaptor"), _)) + .Times(1); + EXPECT_CALL(CallbacksHandle, + runAfterPass(HasNameRegex("CGSCCToFunctionPassAdaptor"), _)) + .Times(1); + EXPECT_CALL(CallbacksHandle, + runAfterPass(HasNameRegex("FunctionToLoopPassAdaptor"), _)) + .Times(1); + + // Ignore analyses introduced by adaptor passes. + EXPECT_CALL(CallbacksHandle, + runBeforeAnalysis(Not(HasNameRegex("MockAnalysisHandle")), _)) + .Times(AnyNumber()); + EXPECT_CALL(CallbacksHandle, + runAfterAnalysis(Not(HasNameRegex("MockAnalysisHandle")), _)) + .Times(AnyNumber()); + + // Register Funtion and Loop version of "test-transform" for testing + PB.registerPipelineParsingCallback( + [](StringRef Name, FunctionPassManager &FPM, + ArrayRef) { + if (Name == "test-transform") { + FPM.addPass(MockPassHandle().getPass()); + return true; + } + return false; + }); + PB.registerPipelineParsingCallback( + [](StringRef Name, LoopPassManager &LPM, + ArrayRef) { + if (Name == "test-transform") { + LPM.addPass(MockPassHandle().getPass()); + return true; + } + return false; + }); + + StringRef PipelineText = "test-transform,function(test-transform),cgscc(" + "function(loop(test-transform)))"; ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText, true), Succeeded()) << "Pipeline was: " << PipelineText; diff --git a/polly/include/polly/ScopPass.h b/polly/include/polly/ScopPass.h --- a/polly/include/polly/ScopPass.h +++ b/polly/include/polly/ScopPass.h @@ -204,7 +204,7 @@ template class FunctionToScopPassAdaptor - : public PassInfoMixin> { + : public RequiredPassInfoMixin> { public: explicit FunctionToScopPassAdaptor(ScopPassT Pass) : Pass(std::move(Pass)) {}