[llvm] ab4253f - [Analysis] Remove global state from `PluginInline{Advisor,Order}Analysis`. (#114615)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 18 10:24:13 PST 2024
Author: Michele Scandale
Date: 2024-11-18T10:24:09-08:00
New Revision: ab4253f6dff194a1e09448c8628809d21f148df9
URL: https://github.com/llvm/llvm-project/commit/ab4253f6dff194a1e09448c8628809d21f148df9
DIFF: https://github.com/llvm/llvm-project/commit/ab4253f6dff194a1e09448c8628809d21f148df9.diff
LOG: [Analysis] Remove global state from `PluginInline{Advisor,Order}Analysis`. (#114615)
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.
Added:
Modified:
llvm/include/llvm/Analysis/InlineAdvisor.h
llvm/include/llvm/Analysis/InlineOrder.h
llvm/include/llvm/IR/PassManager.h
llvm/lib/Analysis/InlineAdvisor.cpp
llvm/lib/Analysis/InlineOrder.cpp
llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp
llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp
Removed:
################################################################################
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..5dab9d0d0a7979 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.
@@ -458,10 +463,9 @@ template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager {
/// and this function returns true.
///
/// (Note: Although the return value of this function indicates whether or not
- /// an analysis was previously registered, there intentionally isn't a way to
- /// query this directly. Instead, you should just register all the analyses
- /// you might want and let this class run them lazily. This idiom lets us
- /// minimize the number of times we have to look up analyses in our
+ /// an analysis was previously registered, you should just register all the
+ /// analyses you might want and let this class run them lazily. This idiom
+ /// lets us minimize the number of times we have to look up analyses in our
/// hashtable.)
template <typename PassBuilderT>
bool registerPass(PassBuilderT &&PassBuilder) {
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;
More information about the llvm-commits
mailing list