[PATCH] D130889: [llvm] Introduce a pass plugin registry and loader
viroulep created this revision.
viroulep added reviewers: w2yehia, mehdi_amini, pcc.
Herald added subscribers: ormris, steven_wu, hiraditya, arichardson, mgorny,
emaste.
Herald added a reviewer: MaskRay.
Herald added a project: All.
viroulep requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, StephenFan.
Herald added projects: clang, LLVM.
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!
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130889
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Frontend/CMakeLists.txt
clang/lib/Frontend/CompilerInstance.cpp
lld/ELF/Config.h
lld/ELF/Driver.cpp
lld/ELF/LTO.cpp
lld/test/ELF/lto/ltopasses-extension.ll
lld/test/lit.cfg.py
llvm/include/llvm/LTO/Config.h
llvm/include/llvm/Passes/PassPluginLoader.h
llvm/include/llvm/Passes/PassPluginRegistry.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Passes/CMakeLists.txt
llvm/lib/Passes/PassPluginLoader.cpp
llvm/lib/Passes/PassPluginRegistry.cpp
llvm/tools/llvm-lto2/llvm-lto2.cpp
llvm/tools/opt/NewPMDriver.cpp
llvm/tools/opt/NewPMDriver.h
llvm/tools/opt/opt.cpp
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -39,7 +39,8 @@
#include "llvm/LinkAllPasses.h"
#include "llvm/MC/SubtargetFeature.h"
#include "llvm/MC/TargetRegistry.h"
-#include "llvm/Passes/PassPlugin.h"
+#include "llvm/Passes/PassPluginLoader.h"
+#include "llvm/Passes/PassPluginRegistry.h"
#include "llvm/Remarks/HotnessThresholdParser.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/FileSystem.h"
@@ -278,10 +279,6 @@
cl::desc("The format used for serializing remarks (default: YAML)"),
cl::value_desc("format"), cl::init("yaml"));
-static cl::list
-PassPlugins("load-pass-plugin",
-cl::desc("Load passes from plugin library"));
-
namespace llvm {
cl::opt
PGOKindFlag("pgo-kind", cl::init(NoPGO), cl::Hidden,
@@ -533,17 +530,6 @@
initializeExampleIRTransforms(Registry);
#endif
- SmallVector PluginList;
- PassPlugins.setCallback([&](const std::string &PluginPath) {
-auto Plugin = PassPlugin::Load(PluginPath);
-if (!Plugin) {
- errs() << "Failed to load passes from '" << PluginPath
- << "'. Request ignored.\n";
- return;
-}
-PluginList.emplace_back(Plugin.get());
- });
-
cl::ParseCommandLineOptions(argc, argv,
"llvm .bc -> .bc modular optimizer and analysis printer\n");
@@ -556,7 +542,7 @@
const bool UseNPM = (EnableNewPassManager && !shouldForceLegacyPM()) ||
PassPipeline.getNumOccurrences() > 0;
- if (!UseNPM && PluginList.size()) {
+ if (!UseNPM && PassPluginRegistry::getPlugins().size()) {
errs() << argv[0] << ": " << PassPlugins.ArgStr
<< " specified with legacy PM.\n";
return 1;
@@ -781,7 +767,7 @@
// layer.
return runPassPipeline(argv[0], *M, TM.get(), &TLII, Out.get(),
ThinLinkOut.get(), RemarksFile.get(), Pipeline,
- Passes, PluginList, OK, VK, PreserveAssemblyUseListOrder,
+ Passes, OK, VK, PreserveAssemblyUseListOrder,
PreserveBitcodeUseListOrder, EmitSummaryIndex,
Em
[PATCH] D130889: [llvm] Introduce a pass plugin registry and loader
viroulep added a comment.
Thanks for your review @w2yehia!
Comment at: llvm/lib/Passes/PassPluginLoader.cpp:23
+void PassPluginLoader::operator=(const std::string &Filename) {
+ sys::SmartScopedLock Lock(*PluginsLock);
+ auto PassPlugin = PassPlugin::Load(Filename);
w2yehia wrote:
> 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 PassPlugins`.
That's a very good question that I couldn't answer: I based the implementation
on what is done over in [[
https://github.com/llvm/llvm-project/blame/main/llvm/lib/Support/PluginLoader.cpp
| PluginLoader ]], but I too wondered why it was guarded ([[
https://github.com/llvm/llvm-project/commit/bee30f58fb3f65d980fa15c14aa9546145521619
| 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!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130889/new/
https://reviews.llvm.org/D130889
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D130889: [llvm] Introduce a pass plugin registry and loader
viroulep updated this revision to Diff 450844.
viroulep added a comment.
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130889/new/
https://reviews.llvm.org/D130889
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Frontend/CMakeLists.txt
clang/lib/Frontend/CompilerInstance.cpp
clang/test/CMakeLists.txt
clang/test/Frontend/load_pass_plugin.ll
clang/test/lit.cfg.py
clang/test/lit.site.cfg.py.in
lld/ELF/Config.h
lld/ELF/Driver.cpp
lld/ELF/LTO.cpp
lld/test/ELF/lto/ltopasses-extension.ll
lld/test/lit.cfg.py
llvm/include/llvm/LTO/Config.h
llvm/include/llvm/Passes/PassPluginLoader.h
llvm/include/llvm/Passes/PassPluginRegistry.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Passes/CMakeLists.txt
llvm/lib/Passes/PassPluginLoader.cpp
llvm/lib/Passes/PassPluginRegistry.cpp
llvm/tools/llvm-lto2/llvm-lto2.cpp
llvm/tools/opt/NewPMDriver.cpp
llvm/tools/opt/NewPMDriver.h
llvm/tools/opt/opt.cpp
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -39,7 +39,8 @@
#include "llvm/LinkAllPasses.h"
#include "llvm/MC/SubtargetFeature.h"
#include "llvm/MC/TargetRegistry.h"
-#include "llvm/Passes/PassPlugin.h"
+#include "llvm/Passes/PassPluginLoader.h"
+#include "llvm/Passes/PassPluginRegistry.h"
#include "llvm/Remarks/HotnessThresholdParser.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/FileSystem.h"
@@ -278,10 +279,6 @@
cl::desc("The format used for serializing remarks (default: YAML)"),
cl::value_desc("format"), cl::init("yaml"));
-static cl::list
-PassPlugins("load-pass-plugin",
-cl::desc("Load passes from plugin library"));
-
namespace llvm {
cl::opt
PGOKindFlag("pgo-kind", cl::init(NoPGO), cl::Hidden,
@@ -533,17 +530,6 @@
initializeExampleIRTransforms(Registry);
#endif
- SmallVector PluginList;
- PassPlugins.setCallback([&](const std::string &PluginPath) {
-auto Plugin = PassPlugin::Load(PluginPath);
-if (!Plugin) {
- errs() << "Failed to load passes from '" << PluginPath
- << "'. Request ignored.\n";
- return;
-}
-PluginList.emplace_back(Plugin.get());
- });
-
cl::ParseCommandLineOptions(argc, argv,
"llvm .bc -> .bc modular optimizer and analysis printer\n");
@@ -556,7 +542,7 @@
const bool UseNPM = (EnableNewPassManager && !shouldForceLegacyPM()) ||
PassPipeline.getNumOccurrences() > 0;
- if (!UseNPM && PluginList.size()) {
+ if (!UseNPM && PassPluginRegistry::getPlugins().size()) {
errs() << argv[0] << ": " << PassPlugins.ArgStr
<< " specified with legacy PM.\n";
return 1;
@@ -781,7 +767,7 @@
// layer.
return runPassPipeline(argv[0], *M, TM.get(), &TLII, Out.get(),
ThinLinkOut.get(), RemarksFile.get(), Pipeline,
- Passes, PluginList, OK, VK, PreserveAssemblyUseListOrder,
+ Passes, OK, VK, PreserveAssemblyUseListOrder,
PreserveBitcodeUseListOrder, EmitSummaryIndex,
EmitModuleHash, EnableDebugify,
VerifyDebugInfoPreserve)
Index: llvm/tools/opt/NewPMDriver.h
===
--- llvm/tools/opt/NewPMDriver.h
+++ llvm/tools/opt/NewPMDriver.h
@@ -25,7 +25,6 @@
namespace llvm {
class StringRef;
class Module;
-class PassPlugin;
class TargetMachine;
class ToolOutputFile;
class TargetLibraryInfoImpl;
@@ -72,8 +71,7 @@
TargetLibraryInfoImpl *TLII, ToolOutputFile *Out,
ToolOutputFile *ThinLinkOut, ToolOutputFile *OptRemarkFile,
StringRef PassPipeline, ArrayRef PassInfos,
- ArrayRef PassPlugins, opt_tool::OutputKind OK,
- opt_tool::VerifierKind VK,
+ opt_tool::OutputKind OK, opt_tool::VerifierKind VK,
bool ShouldPreserveAssemblyUseListOrder,
bool ShouldPreserveBitcodeUseListOrder,
bool EmitSummaryIndex, bool EmitModuleHash,
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -27,7 +27,7 @@
#include "llvm/IR/PassManager.h"
#include "llvm/IR/Verifier.h"
#include "llvm/Passes/PassBuilder.h"
-#include "llvm/Passes/PassPlugin.h"
+#include "llvm/Passes/PassPluginRegistry.h"
#include "llvm/Passes/StandardInstrumentations.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ToolOutputFile.h"
@@ -288,7 +288,6 @@
ToolOutputFile *ThinLTOLinkOut,
ToolOutputFile *OptRemarkFile,
