[llvm] [Analysis] Remove global state from `PluginInline{Advisor,Order}Analysis`. (PR #114615)
Michele Scandale via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 10:03:18 PST 2024
michele-scandale wrote:
> > This causes issues when pipelines using plugins and pipelines not using plugins are run in the same process.
>
> I think I'm not understanding how
>
> ```
> llvmGetPassPluginInfo() {
> return {
> [](PassBuilder &PB) {
> PB.registerPipelineStartEPCallback(
> [](ModulePassManager &MPM, OptimizationLevel Level) {
> MPM.addPass(DefaultDynamicAdvisor());
> });
> }};
> }
> ```
>
> with this patch applied is any different from just
>
> ```
> llvmGetPassPluginInfo() {
> return {
> [](PassBuilder &PB) {
> PB.registerAnalysisRegistrationCallback(
> [](ModuleAnalysisManager & MAM) {
> PluginInlineAdvisorAnalysis PA(defaultAdvisorFactory);
> MAM.registerPass([&] { return PA; });
> });
> }};
> }
> ```
>
> with the `DefaultDynamicAdvisor` pass removed. `registerAnalysisRegistrationCallback` seems like a cleaner way of registering the inline advisor unless I'm not understanding some other detail of `DefaultDynamicAdvisor`
The problem is on the `InlineAdvisor` side when the `PluginInlineAdvisorAnalysis ` is being queried:
```
bool InlineAdvisorAnalysis::Result::tryCreate(
InlineParams Params, InliningAdvisorMode Mode,
const ReplayInlinerSettings &ReplaySettings, InlineContext IC) {
auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
if (PluginInlineAdvisorAnalysis::HasBeenRegistered) { // <---- this is a shared global variable!
auto &DA = MAM.getResult<PluginInlineAdvisorAnalysis>(M);
Advisor.reset(DA.Factory(M, FAM, Params, IC));
return !!Advisor;
}
```
the `HasBeenRegistered` global variable is being set to `true` in the constructor of `PluginInlineAdvisorAnalysis` -- which currently happen when the `DefaultDynamicAdvisor::run` is executed, and with your change it will happen when the analysis are registered.
In both cases once someone somewhere does that the `HasBeenRegistered` applies globally, and that will affect whatever other pipelines where plugins are not being loaded.
https://github.com/llvm/llvm-project/pull/114615
More information about the llvm-commits
mailing list