Skip to content

Commit ab4253f

Browse files
[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.
1 parent a7b2e73 commit ab4253f

File tree

7 files changed

+20
-52
lines changed

7 files changed

+20
-52
lines changed

llvm/include/llvm/Analysis/InlineAdvisor.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,15 +287,13 @@ class PluginInlineAdvisorAnalysis
287287
: public AnalysisInfoMixin<PluginInlineAdvisorAnalysis> {
288288
public:
289289
static AnalysisKey Key;
290-
static bool HasBeenRegistered;
291290

292291
typedef InlineAdvisor *(*AdvisorFactory)(Module &M,
293292
FunctionAnalysisManager &FAM,
294293
InlineParams Params,
295294
InlineContext IC);
296295

297296
PluginInlineAdvisorAnalysis(AdvisorFactory Factory) : Factory(Factory) {
298-
HasBeenRegistered = true;
299297
assert(Factory != nullptr &&
300298
"The plugin advisor factory should not be a null pointer.");
301299
}

llvm/include/llvm/Analysis/InlineOrder.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ class PluginInlineOrderAnalysis
5959
ModuleAnalysisManager &MAM, Module &M);
6060

6161
PluginInlineOrderAnalysis(InlineOrderFactory Factory) : Factory(Factory) {
62-
HasBeenRegistered = true;
6362
assert(Factory != nullptr &&
6463
"The plugin inline order factory should not be a null pointer.");
6564
}
@@ -71,11 +70,7 @@ class PluginInlineOrderAnalysis
7170
Result run(Module &, ModuleAnalysisManager &) { return {Factory}; }
7271
Result getResult() { return {Factory}; }
7372

74-
static bool isRegistered() { return HasBeenRegistered; }
75-
static void unregister() { HasBeenRegistered = false; }
76-
7773
private:
78-
static bool HasBeenRegistered;
7974
InlineOrderFactory Factory;
8075
};
8176

llvm/include/llvm/IR/PassManager.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,11 @@ template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager {
398398
AnalysisResultLists.clear();
399399
}
400400

401+
/// Returns true if the specified analysis pass is registered.
402+
template <typename PassT> bool isPassRegistered() const {
403+
return AnalysisPasses.count(PassT::ID());
404+
}
405+
401406
/// Get the result of an analysis pass for a given IR unit.
402407
///
403408
/// Runs the analysis if a cached result is not available.
@@ -458,10 +463,9 @@ template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager {
458463
/// and this function returns true.
459464
///
460465
/// (Note: Although the return value of this function indicates whether or not
461-
/// an analysis was previously registered, there intentionally isn't a way to
462-
/// query this directly. Instead, you should just register all the analyses
463-
/// you might want and let this class run them lazily. This idiom lets us
464-
/// minimize the number of times we have to look up analyses in our
466+
/// an analysis was previously registered, you should just register all the
467+
/// analyses you might want and let this class run them lazily. This idiom
468+
/// lets us minimize the number of times we have to look up analyses in our
465469
/// hashtable.)
466470
template <typename PassBuilderT>
467471
bool registerPass(PassBuilderT &&PassBuilder) {

llvm/lib/Analysis/InlineAdvisor.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,12 @@ void InlineAdvice::recordInliningWithCalleeDeleted() {
199199

200200
AnalysisKey InlineAdvisorAnalysis::Key;
201201
AnalysisKey PluginInlineAdvisorAnalysis::Key;
202-
bool PluginInlineAdvisorAnalysis::HasBeenRegistered = false;
203202

204203
bool InlineAdvisorAnalysis::Result::tryCreate(
205204
InlineParams Params, InliningAdvisorMode Mode,
206205
const ReplayInlinerSettings &ReplaySettings, InlineContext IC) {
207206
auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
208-
if (PluginInlineAdvisorAnalysis::HasBeenRegistered) {
207+
if (MAM.isPassRegistered<PluginInlineAdvisorAnalysis>()) {
209208
auto &DA = MAM.getResult<PluginInlineAdvisorAnalysis>(M);
210209
Advisor.reset(DA.Factory(M, FAM, Params, IC));
211210
return !!Advisor;

llvm/lib/Analysis/InlineOrder.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ class PriorityInlineOrder : public InlineOrder<std::pair<CallBase *, int>> {
283283
} // namespace
284284

285285
AnalysisKey llvm::PluginInlineOrderAnalysis::Key;
286-
bool llvm::PluginInlineOrderAnalysis::HasBeenRegistered;
287286

288287
std::unique_ptr<InlineOrder<std::pair<CallBase *, int>>>
289288
llvm::getDefaultInlineOrder(FunctionAnalysisManager &FAM,
@@ -313,7 +312,7 @@ llvm::getDefaultInlineOrder(FunctionAnalysisManager &FAM,
313312
std::unique_ptr<InlineOrder<std::pair<CallBase *, int>>>
314313
llvm::getInlineOrder(FunctionAnalysisManager &FAM, const InlineParams &Params,
315314
ModuleAnalysisManager &MAM, Module &M) {
316-
if (llvm::PluginInlineOrderAnalysis::isRegistered()) {
315+
if (MAM.isPassRegistered<PluginInlineOrderAnalysis>()) {
317316
LLVM_DEBUG(dbgs() << " Current used priority: plugin ---- \n");
318317
return MAM.getResult<PluginInlineOrderAnalysis>(M).Factory(FAM, Params, MAM,
319318
M);

llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -87,33 +87,10 @@ struct CompilerInstance {
8787
ThinOrFullLTOPhase::None));
8888
}
8989

90-
~CompilerInstance() {
91-
// Reset the static variable that tracks if the plugin has been registered.
92-
// This is needed to allow the test to run multiple times.
93-
PluginInlineAdvisorAnalysis::HasBeenRegistered = false;
94-
}
95-
9690
std::string output;
9791
std::unique_ptr<Module> outputM;
9892

99-
// run with the default inliner
100-
auto run_default(StringRef IR) {
101-
PluginInlineAdvisorAnalysis::HasBeenRegistered = false;
102-
outputM = parseAssemblyString(IR, Error, Ctx);
103-
MPM.run(*outputM, MAM);
104-
ASSERT_TRUE(outputM);
105-
output.clear();
106-
raw_string_ostream o_stream{output};
107-
outputM->print(o_stream, nullptr);
108-
ASSERT_TRUE(true);
109-
}
110-
111-
// run with the dnamic inliner
112-
auto run_dynamic(StringRef IR) {
113-
// note typically the constructor for the DynamicInlineAdvisorAnalysis
114-
// will automatically set this to true, we controll it here only to
115-
// altenate between the default and dynamic inliner in our test
116-
PluginInlineAdvisorAnalysis::HasBeenRegistered = true;
93+
auto run(StringRef IR) {
11794
outputM = parseAssemblyString(IR, Error, Ctx);
11895
MPM.run(*outputM, MAM);
11996
ASSERT_TRUE(outputM);
@@ -274,14 +251,16 @@ TEST(PluginInlineAdvisorTest, PluginLoad) {
274251
// Skip the test if plugins are disabled.
275252
GTEST_SKIP();
276253
#endif
277-
CompilerInstance CI{};
278-
CI.setupPlugin();
254+
CompilerInstance DefaultCI{};
255+
256+
CompilerInstance PluginCI{};
257+
PluginCI.setupPlugin();
279258

280259
for (StringRef IR : TestIRS) {
281-
CI.run_default(IR);
282-
std::string default_output = CI.output;
283-
CI.run_dynamic(IR);
284-
std::string dynamic_output = CI.output;
260+
DefaultCI.run(IR);
261+
std::string default_output = DefaultCI.output;
262+
PluginCI.run(IR);
263+
std::string dynamic_output = PluginCI.output;
285264
ASSERT_EQ(default_output, dynamic_output);
286265
}
287266
}
@@ -294,7 +273,7 @@ TEST(PluginInlineAdvisorTest, CustomAdvisor) {
294273
CI.setupFooOnly();
295274

296275
for (StringRef IR : TestIRS) {
297-
CI.run_dynamic(IR);
276+
CI.run(IR);
298277
CallGraph CGraph = CallGraph(*CI.outputM);
299278
for (auto &node : CGraph) {
300279
for (auto &edge : *node.second) {

llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,6 @@ struct CompilerInstance {
6161
ThinOrFullLTOPhase::None));
6262
}
6363

64-
~CompilerInstance() {
65-
// Reset the static variable that tracks if the plugin has been registered.
66-
// This is needed to allow the test to run multiple times.
67-
PluginInlineOrderAnalysis::unregister();
68-
}
69-
7064
std::string Output;
7165
std::unique_ptr<Module> OutputM;
7266

0 commit comments

Comments
 (0)