Page MenuHomePhabricator

[llvm] Introduce a pass plugin registry and loader
Needs ReviewPublic

Authored by viroulep on Aug 1 2022, 2:19 AM.

Details

Summary

This basically extends what has been done for opt in https://reviews.llvm.org/D121566.

The general issue is that, at the moment and except for opt, you can't use command line options defined by plugins when using only -load-pass-plugin (or -fpass-plugin), and you have to also load the library with the "old" entry point through -load to have them recognized.

This is due to the fact that parsing command line options occurs before plugins are loaded through -load-pass-plugin, and basically each tool has its own way of dealing with the loading of pass plugins.
The patch tries to mimic what was done for the existing -load command line option and aims at providing common entry points for the lib and tools:

  • tools make sure to register the loaded plugin (through the option or manually)
  • any part of the lib/tools can query the registry to have the plugins register their callbacks.

As a result -load-pass-plugin has a behavior that matches the -load option, and tools can simply include the header defining the -load-pass-plugin option if they want to have the feature.

I'm not exactly sure why -load-pass-plugin wasn't created with the same behavior as -load, which makes me think I may be missing something: if the implementation I suggest doesn't look good to you I'd be happy to look into something else (hopefully the general idea of having a single load for old/new plugins does sound good to you).

This patch touches a couple of tools (clang, llvm-lto2, opt) in addition to llvm; I tried to include only a couple of appropriate reviewers; my apologies if I missed someone, please feel free to add more relevant people if you think it's necessary!

Diff Detail

Event Timeline

viroulep created this revision.Aug 1 2022, 2:19 AM
Herald added a project: Restricted Project. · View Herald Transcript
viroulep requested review of this revision.Aug 1 2022, 2:19 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 1 2022, 2:19 AM

Looks good. Thanks.

llvm/lib/Passes/PassPluginLoader.cpp
23

why do we need a lock?
Is there a use case where multiple threads enter this function.
Right now, the only place we create a PassPluginLoader is in cl::opt<PassPluginLoader> PassPlugins.

Thanks for your review @w2yehia!

llvm/lib/Passes/PassPluginLoader.cpp
23

That's a very good question that I couldn't answer: I based the implementation on what is done over in PluginLoader, but I too wondered why it was guarded (this commit introduces it but I couldn't find more details).

I'm fine with removing it unless someone can provide a use case where we need it; I'll update the patch!

viroulep updated this revision to Diff 450844.Aug 8 2022, 9:32 AM

I took into account @w2yehia's comment, and I also noticed there was no test in clang for the -fpass-plugin option, so I added one.