[llvm] [Analysis] Remove global state from `PluginInline{Advisor,Order}Analysis`. (PR #114615)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 1 15:17:28 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Michele Scandale (michele-scandale)
<details>
<summary>Changes</summary>
The plugin analysis for `InlineAdvisor` and `InlineOrder` currently relies on shared global state to keep track if the analysis is available.
This causes issues when pipelines using plugins and pipelines not using plugins are run in the same process.
The shared global state can be easily replaced by checking in the given instance of `ModuleAnalysisManager` if the plugin analysis has been registered.
---
Full diff: https://github.com/llvm/llvm-project/pull/114615.diff
7 Files Affected:
- (modified) llvm/include/llvm/Analysis/InlineAdvisor.h (-2)
- (modified) llvm/include/llvm/Analysis/InlineOrder.h (-5)
- (modified) llvm/include/llvm/IR/PassManager.h (+5)
- (modified) llvm/lib/Analysis/InlineAdvisor.cpp (+1-2)
- (modified) llvm/lib/Analysis/InlineOrder.cpp (+1-2)
- (modified) llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp (+10-31)
- (modified) llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp (-6)
``````````diff
diff --git a/llvm/include/llvm/Analysis/InlineAdvisor.h b/llvm/include/llvm/Analysis/InlineAdvisor.h
index 700c3b0f18b8d5..b002bec2c91836 100644
--- a/llvm/include/llvm/Analysis/InlineAdvisor.h
+++ b/llvm/include/llvm/Analysis/InlineAdvisor.h
@@ -287,7 +287,6 @@ class PluginInlineAdvisorAnalysis
: public AnalysisInfoMixin<PluginInlineAdvisorAnalysis> {
public:
static AnalysisKey Key;
- static bool HasBeenRegistered;
typedef InlineAdvisor *(*AdvisorFactory)(Module &M,
FunctionAnalysisManager &FAM,
@@ -295,7 +294,6 @@ class PluginInlineAdvisorAnalysis
InlineContext IC);
PluginInlineAdvisorAnalysis(AdvisorFactory Factory) : Factory(Factory) {
- HasBeenRegistered = true;
assert(Factory != nullptr &&
"The plugin advisor factory should not be a null pointer.");
}
diff --git a/llvm/include/llvm/Analysis/InlineOrder.h b/llvm/include/llvm/Analysis/InlineOrder.h
index 2fa2d6091303ad..498cef314b5c31 100644
--- a/llvm/include/llvm/Analysis/InlineOrder.h
+++ b/llvm/include/llvm/Analysis/InlineOrder.h
@@ -59,7 +59,6 @@ class PluginInlineOrderAnalysis
ModuleAnalysisManager &MAM, Module &M);
PluginInlineOrderAnalysis(InlineOrderFactory Factory) : Factory(Factory) {
- HasBeenRegistered = true;
assert(Factory != nullptr &&
"The plugin inline order factory should not be a null pointer.");
}
@@ -71,11 +70,7 @@ class PluginInlineOrderAnalysis
Result run(Module &, ModuleAnalysisManager &) { return {Factory}; }
Result getResult() { return {Factory}; }
- static bool isRegistered() { return HasBeenRegistered; }
- static void unregister() { HasBeenRegistered = false; }
-
private:
- static bool HasBeenRegistered;
InlineOrderFactory Factory;
};
diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h
index d269221fac0701..cd090384c353ec 100644
--- a/llvm/include/llvm/IR/PassManager.h
+++ b/llvm/include/llvm/IR/PassManager.h
@@ -398,6 +398,11 @@ template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager {
AnalysisResultLists.clear();
}
+ /// Returns true if the specified analysis pass is registered.
+ template <typename PassT> bool isPassRegistered() const {
+ return AnalysisPasses.count(PassT::ID());
+ }
+
/// Get the result of an analysis pass for a given IR unit.
///
/// Runs the analysis if a cached result is not available.
diff --git a/llvm/lib/Analysis/InlineAdvisor.cpp b/llvm/lib/Analysis/InlineAdvisor.cpp
index 45702fa25d8b14..12553dd446a616 100644
--- a/llvm/lib/Analysis/InlineAdvisor.cpp
+++ b/llvm/lib/Analysis/InlineAdvisor.cpp
@@ -199,13 +199,12 @@ void InlineAdvice::recordInliningWithCalleeDeleted() {
AnalysisKey InlineAdvisorAnalysis::Key;
AnalysisKey PluginInlineAdvisorAnalysis::Key;
-bool PluginInlineAdvisorAnalysis::HasBeenRegistered = false;
bool InlineAdvisorAnalysis::Result::tryCreate(
InlineParams Params, InliningAdvisorMode Mode,
const ReplayInlinerSettings &ReplaySettings, InlineContext IC) {
auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
- if (PluginInlineAdvisorAnalysis::HasBeenRegistered) {
+ if (MAM.isPassRegistered<PluginInlineAdvisorAnalysis>()) {
auto &DA = MAM.getResult<PluginInlineAdvisorAnalysis>(M);
Advisor.reset(DA.Factory(M, FAM, Params, IC));
return !!Advisor;
diff --git a/llvm/lib/Analysis/InlineOrder.cpp b/llvm/lib/Analysis/InlineOrder.cpp
index f156daa2f126fb..8d920153f250df 100644
--- a/llvm/lib/Analysis/InlineOrder.cpp
+++ b/llvm/lib/Analysis/InlineOrder.cpp
@@ -283,7 +283,6 @@ class PriorityInlineOrder : public InlineOrder<std::pair<CallBase *, int>> {
} // namespace
AnalysisKey llvm::PluginInlineOrderAnalysis::Key;
-bool llvm::PluginInlineOrderAnalysis::HasBeenRegistered;
std::unique_ptr<InlineOrder<std::pair<CallBase *, int>>>
llvm::getDefaultInlineOrder(FunctionAnalysisManager &FAM,
@@ -313,7 +312,7 @@ llvm::getDefaultInlineOrder(FunctionAnalysisManager &FAM,
std::unique_ptr<InlineOrder<std::pair<CallBase *, int>>>
llvm::getInlineOrder(FunctionAnalysisManager &FAM, const InlineParams &Params,
ModuleAnalysisManager &MAM, Module &M) {
- if (llvm::PluginInlineOrderAnalysis::isRegistered()) {
+ if (MAM.isPassRegistered<PluginInlineOrderAnalysis>()) {
LLVM_DEBUG(dbgs() << " Current used priority: plugin ---- \n");
return MAM.getResult<PluginInlineOrderAnalysis>(M).Factory(FAM, Params, MAM,
M);
diff --git a/llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp b/llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp
index 3330751120e6c8..92c0b1bcacb12b 100644
--- a/llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp
+++ b/llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp
@@ -87,33 +87,10 @@ struct CompilerInstance {
ThinOrFullLTOPhase::None));
}
- ~CompilerInstance() {
- // Reset the static variable that tracks if the plugin has been registered.
- // This is needed to allow the test to run multiple times.
- PluginInlineAdvisorAnalysis::HasBeenRegistered = false;
- }
-
std::string output;
std::unique_ptr<Module> outputM;
- // run with the default inliner
- auto run_default(StringRef IR) {
- PluginInlineAdvisorAnalysis::HasBeenRegistered = false;
- outputM = parseAssemblyString(IR, Error, Ctx);
- MPM.run(*outputM, MAM);
- ASSERT_TRUE(outputM);
- output.clear();
- raw_string_ostream o_stream{output};
- outputM->print(o_stream, nullptr);
- ASSERT_TRUE(true);
- }
-
- // run with the dnamic inliner
- auto run_dynamic(StringRef IR) {
- // note typically the constructor for the DynamicInlineAdvisorAnalysis
- // will automatically set this to true, we controll it here only to
- // altenate between the default and dynamic inliner in our test
- PluginInlineAdvisorAnalysis::HasBeenRegistered = true;
+ auto run(StringRef IR) {
outputM = parseAssemblyString(IR, Error, Ctx);
MPM.run(*outputM, MAM);
ASSERT_TRUE(outputM);
@@ -274,14 +251,16 @@ TEST(PluginInlineAdvisorTest, PluginLoad) {
// Skip the test if plugins are disabled.
GTEST_SKIP();
#endif
- CompilerInstance CI{};
- CI.setupPlugin();
+ CompilerInstance DefaultCI{};
+
+ CompilerInstance PluginCI{};
+ PluginCI.setupPlugin();
for (StringRef IR : TestIRS) {
- CI.run_default(IR);
- std::string default_output = CI.output;
- CI.run_dynamic(IR);
- std::string dynamic_output = CI.output;
+ DefaultCI.run(IR);
+ std::string default_output = DefaultCI.output;
+ PluginCI.run(IR);
+ std::string dynamic_output = PluginCI.output;
ASSERT_EQ(default_output, dynamic_output);
}
}
@@ -294,7 +273,7 @@ TEST(PluginInlineAdvisorTest, CustomAdvisor) {
CI.setupFooOnly();
for (StringRef IR : TestIRS) {
- CI.run_dynamic(IR);
+ CI.run(IR);
CallGraph CGraph = CallGraph(*CI.outputM);
for (auto &node : CGraph) {
for (auto &edge : *node.second) {
diff --git a/llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp b/llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp
index ca860a0dd55843..0b31b0892d75ae 100644
--- a/llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp
+++ b/llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp
@@ -61,12 +61,6 @@ struct CompilerInstance {
ThinOrFullLTOPhase::None));
}
- ~CompilerInstance() {
- // Reset the static variable that tracks if the plugin has been registered.
- // This is needed to allow the test to run multiple times.
- PluginInlineOrderAnalysis::unregister();
- }
-
std::string Output;
std::unique_ptr<Module> OutputM;
``````````
</details>
https://github.com/llvm/llvm-project/pull/114615
More information about the llvm-commits
mailing list