[llvm] af4c873 - [NewPM] Allow passes to never be skipped
Yuanfang Chen via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 18 22:57:04 PDT 2020
Author: Yuanfang Chen
Date: 2020-07-18T22:28:46-07:00
New Revision: af4c8730924fb6617494c223dac62d6c72c97c6f
URL: https://github.com/llvm/llvm-project/commit/af4c8730924fb6617494c223dac62d6c72c97c6f
DIFF: https://github.com/llvm/llvm-project/commit/af4c8730924fb6617494c223dac62d6c72c97c6f.diff
LOG: [NewPM] Allow passes to never be skipped
A pass declares itself unskippable by defining a method `static bool isRequired()`.
Also, this patch makes pass managers and adaptor passes required (unskippable).
PassInstrumentation before-pass-callbacks could be used to skip passes by returning false.
However, some passes should not be skipped at all. Especially so for special-purpose passes such as pass managers and adaptor passes since if they are skipped for any reason, the passes contained by them would also be skipped ignoring contained passes's return value of `isRequired()`.
Reviewed By: aeubanks
Differential Revision: https://reviews.llvm.org/D82344
Added:
Modified:
llvm/include/llvm/Analysis/CGSCCPassManager.h
llvm/include/llvm/IR/PassInstrumentation.h
llvm/include/llvm/IR/PassManager.h
llvm/include/llvm/IR/PassManagerInternal.h
llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
llvm/unittests/IR/PassBuilderCallbacksTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/CGSCCPassManager.h b/llvm/include/llvm/Analysis/CGSCCPassManager.h
index eb0d3ae8fedf..e70af71b3da6 100644
--- a/llvm/include/llvm/Analysis/CGSCCPassManager.h
+++ b/llvm/include/llvm/Analysis/CGSCCPassManager.h
@@ -355,6 +355,8 @@ class ModuleToPostOrderCGSCCPassAdaptor
/// Runs the CGSCC pass across every SCC in the module.
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
+ static bool isRequired() { return true; }
+
private:
CGSCCPassT Pass;
};
@@ -543,6 +545,8 @@ class CGSCCToFunctionPassAdaptor
return PA;
}
+ static bool isRequired() { return true; }
+
private:
FunctionPassT Pass;
};
diff --git a/llvm/include/llvm/IR/PassInstrumentation.h b/llvm/include/llvm/IR/PassInstrumentation.h
index bcc434548e67..37390e4e682b 100644
--- a/llvm/include/llvm/IR/PassInstrumentation.h
+++ b/llvm/include/llvm/IR/PassInstrumentation.h
@@ -129,6 +129,26 @@ class PassInstrumentationCallbacks {
class PassInstrumentation {
PassInstrumentationCallbacks *Callbacks;
+ // Template argument PassT of PassInstrumentation::runBeforePass could be two
+ // kinds: (1) a regular pass inherited from PassInfoMixin (happen when
+ // creating a adaptor pass for a regular pass); (2) a type-erased PassConcept
+ // created from (1). Here we want to make case (1) skippable unconditionally
+ // since they are regular passes. We call PassConcept::isRequired to decide
+ // for case (2).
+ template <typename PassT>
+ using has_required_t = decltype(std::declval<PassT &>().isRequired());
+
+ template <typename PassT>
+ static std::enable_if_t<is_detected<has_required_t, PassT>::value, bool>
+ isRequired(const PassT &Pass) {
+ return Pass.isRequired();
+ }
+ template <typename PassT>
+ static std::enable_if_t<!is_detected<has_required_t, PassT>::value, bool>
+ isRequired(const PassT &Pass) {
+ return false;
+ }
+
public:
/// Callbacks object is not owned by PassInstrumentation, its life-time
/// should at least match the life-time of corresponding
@@ -148,6 +168,7 @@ class PassInstrumentation {
bool ShouldRun = true;
for (auto &C : Callbacks->BeforePassCallbacks)
ShouldRun &= C(Pass.name(), llvm::Any(&IR));
+ ShouldRun = ShouldRun || isRequired(Pass);
return ShouldRun;
}
diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h
index 4d5f292ba9a1..f503871e2360 100644
--- a/llvm/include/llvm/IR/PassManager.h
+++ b/llvm/include/llvm/IR/PassManager.h
@@ -559,6 +559,8 @@ class PassManager : public PassInfoMixin<
Passes.emplace_back(new PassModelT(std::move(Pass)));
}
+ static bool isRequired() { return true; }
+
private:
using PassConceptT =
detail::PassConcept<IRUnitT, AnalysisManagerT, ExtraArgTs...>;
@@ -1260,6 +1262,8 @@ class ModuleToFunctionPassAdaptor
return PA;
}
+ static bool isRequired() { return true; }
+
private:
FunctionPassT Pass;
};
diff --git a/llvm/include/llvm/IR/PassManagerInternal.h b/llvm/include/llvm/IR/PassManagerInternal.h
index c602c0b5cc20..986ed0b5a7ac 100644
--- a/llvm/include/llvm/IR/PassManagerInternal.h
+++ b/llvm/include/llvm/IR/PassManagerInternal.h
@@ -48,6 +48,12 @@ struct PassConcept {
/// Polymorphic method to access the name of a pass.
virtual StringRef name() const = 0;
+
+ /// Polymorphic method to to let a pass optionally exempted from skipping by
+ /// PassInstrumentation.
+ /// To opt-in, pass should implement `static bool isRequired()`. It's no-op
+ /// to have `isRequired` always return false since that is the default.
+ virtual bool isRequired() const = 0;
};
/// A template wrapper used to implement the polymorphic API.
@@ -81,6 +87,22 @@ struct PassModel : PassConcept<IRUnitT, AnalysisManagerT, ExtraArgTs...> {
StringRef name() const override { return PassT::name(); }
+ template <typename T>
+ using has_required_t = decltype(std::declval<T &>().isRequired());
+
+ template <typename T>
+ static std::enable_if_t<is_detected<has_required_t, T>::value, bool>
+ passIsRequiredImpl() {
+ return T::isRequired();
+ }
+ template <typename T>
+ static std::enable_if_t<!is_detected<has_required_t, T>::value, bool>
+ passIsRequiredImpl() {
+ return false;
+ }
+
+ bool isRequired() const override { return passIsRequiredImpl<PassT>(); }
+
PassT Pass;
};
diff --git a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
index 9b2f0fcab95b..aff80ef1dcfa 100644
--- a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
+++ b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
@@ -366,6 +366,8 @@ class FunctionToLoopPassAdaptor
return PA;
}
+ static bool isRequired() { return true; }
+
private:
LoopPassT Pass;
diff --git a/llvm/unittests/IR/PassBuilderCallbacksTest.cpp b/llvm/unittests/IR/PassBuilderCallbacksTest.cpp
index 1f17a4f34b18..c2c15069f413 100644
--- a/llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ b/llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -524,10 +524,10 @@ TEST_F(ModuleCallbacksTest, InstrumentedSkippedPasses) {
// Non-mock instrumentation run here can safely be ignored.
CallbacksHandle.ignoreNonMockPassInstrumentation("<string>");
- // Skip the pass by returning false.
- EXPECT_CALL(CallbacksHandle, runBeforePass(HasNameRegex("MockPassHandle"),
- HasName("<string>")))
- .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("<string>"), _)).Times(0);
EXPECT_CALL(PassHandle, run(HasName("<string>"), _)).Times(0);
@@ -543,7 +543,60 @@ TEST_F(ModuleCallbacksTest, InstrumentedSkippedPasses) {
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<PassBuilder::PipelineElement>) {
+ if (Name == "test-transform") {
+ FPM.addPass(MockPassHandle<Function>().getPass());
+ return true;
+ }
+ return false;
+ });
+ PB.registerPipelineParsingCallback(
+ [](StringRef Name, LoopPassManager &LPM,
+ ArrayRef<PassBuilder::PipelineElement>) {
+ if (Name == "test-transform") {
+ LPM.addPass(MockPassHandle<Loop>().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;
More information about the llvm-commits
mailing list