aeubanks added a comment.
In D137103#3906667 <https://reviews.llvm.org/D137103#3906667>, @speryt wrote:
> Addressed (hopefully) all review comments.
> I tried to understand how the naming schemas should work for registering new pass plugin, but there is a chance I might have missed something or misunderstood.
> Bottom-line question is, what should I change, so I can call this pass with flags `-tut-simplifycfg -tut-simplifycfg-version=XX`?
`-passes=tut-simplifycfg` will call into the lambda passed to `registerPipelineParsingCallback`. `-tut-simplifycfg-version=XX` should still be the same
> My best guess is that I might be missing one extra registration step. I added `-DLLVM_INCLUDE_EXAMPLES=ON` to cmake which resulted in `-- Registering SimplifyCFG as a pass plugin (static build: OFF)` which indicates that I should somehow register plugin with PassBuilder as dynamically linked. As described here:
>
>> // Load plugin dynamically.
>> auto Plugin = PassPlugin::Load(PathToPlugin);
>> if (!Plugin)
>> report_error();
>> // Register plugin extensions in PassBuilder.
>> Plugin.registerPassBuilderCallbacks(PB);
>
> But from documentation and look at sources it is unclear how/where exactly it should be done. I'll appreciate any pointers.
`-DLLVM_INCLUDE_EXAMPLES=ON` is the default, you actually want `-DLLVM_BUILD_EXAMPLES=ON` (it's very confusing but the tests wouldn't run with `LLVM_INCLUDE_EXAMPLES`, only with `LLVM_BUILD_EXAMPLES` (found by following through `config.build_examples` in `llvm/test/Examples/lit.local.cfg`)
that code is already done in `opt` in `NewPMDriver.cpp` which is aware of example in-tree plugins; the docs were meant to be for if you're writing your own tool:
// For any loaded plugins, let them register pass builder callbacks.
for (auto &PassPlugin : PassPlugins)
PassPlugin.registerPassBuilderCallbacks(PB);
diff --git a/llvm/examples/IRTransforms/CMakeLists.txt b/llvm/examples/IRTransforms/CMakeLists.txt
index 45bc20bdc022..9ad056048090 100644
--- a/llvm/examples/IRTransforms/CMakeLists.txt
+++ b/llvm/examples/IRTransforms/CMakeLists.txt
@@ -1,19 +1,12 @@
-if(LLVM_SIMPLIFYCFG_LINK_INTO_TOOLS)
- message(WARNING "Setting LLVM_SIMPLIFYCFG_LINK_INTO_TOOLS=ON only makes sense for testing purpose")
+if(LLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS)
+ message(WARNING "Setting LLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS=ON only makes sense for testing purpose")
endif()
-
-set(LLVM_LINK_COMPONENTS
- Analysis
- Core
- Support
- )
-
-add_llvm_pass_plugin(SimplifyCFG
+add_llvm_pass_plugin(ExampleIRTransforms
SimplifyCFG.cpp
- ADDITIONAL_HEADER_DIRS
DEPENDS
intrinsics_gen
+ BUILDTREE_ONLY
)
install(TARGETS ${name} RUNTIME DESTINATION "${LLVM_EXAMPLES_INSTALL_DIR}")
diff --git a/llvm/examples/IRTransforms/SimplifyCFG.cpp b/llvm/examples/IRTransforms/SimplifyCFG.cpp
index 9aafa5839120..29e705d11643 100644
--- a/llvm/examples/IRTransforms/SimplifyCFG.cpp
+++ b/llvm/examples/IRTransforms/SimplifyCFG.cpp
@@ -395,7 +395,7 @@ struct SimplifyCFGPass : public PassInfoMixin<SimplifyCFGPass> {
} // namespace
/* New PM Registration */
-llvm::PassPluginLibraryInfo getSimplifyCFGPluginInfo() {
+llvm::PassPluginLibraryInfo getExampleIRTransformsPluginInfo() {
return {LLVM_PLUGIN_API_VERSION, "SimplifyCFG", LLVM_VERSION_STRING,
[](PassBuilder &PB) {
PB.registerVectorizerStartEPCallback(
@@ -417,6 +417,6 @@ llvm::PassPluginLibraryInfo getSimplifyCFGPluginInfo() {
#ifndef LLVM_SIMPLIFYCFG_LINK_INTO_TOOLS
extern "C" LLVM_ATTRIBUTE_WEAK ::llvm::PassPluginLibraryInfo
llvmGetPassPluginInfo() {
- return getSimplifyCFGPluginInfo();
+ return getExampleIRTransformsPluginInfo();
}
#endif
diff --git a/llvm/test/CMakeLists.txt b/llvm/test/CMakeLists.txt
index 01e2a6ebf6c7..b42aa2d014b8 100644
--- a/llvm/test/CMakeLists.txt
+++ b/llvm/test/CMakeLists.txt
@@ -16,6 +16,7 @@ llvm_canonicalize_cmake_booleans(
LLVM_BUILD_EXAMPLES
LLVM_ENABLE_PLUGINS
LLVM_BYE_LINK_INTO_TOOLS
+ LLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS
LLVM_HAVE_TF_AOT
LLVM_HAVE_TF_API
LLVM_INLINER_MODEL_AUTOGENERATED
diff --git a/llvm/test/Examples/IRTransforms/SimplifyCFG/tut-simplify-cfg1.ll b/llvm/test/Examples/IRTransforms/SimplifyCFG/tut-simplify-cfg1.ll
index 20f20ea5a102..35dac1dba927 100644
--- a/llvm/test/Examples/IRTransforms/SimplifyCFG/tut-simplify-cfg1.ll
+++ b/llvm/test/Examples/IRTransforms/SimplifyCFG/tut-simplify-cfg1.ll
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -tut-simplifycfg -tut-simplifycfg-version=v1 -enable-new-pm=0 -S < %s | FileCheck %s
-; RUN: opt -tut-simplifycfg -tut-simplifycfg-version=v2 -enable-new-pm=0 -S < %s | FileCheck %s
-; RUN: opt -tut-simplifycfg -tut-simplifycfg-version=v3 -enable-new-pm=0 -S < %s | FileCheck %s
+; RUN: opt %loadexampleirtransforms -passes=tut-simplifycfg -tut-simplifycfg-version=v1 -S < %s | FileCheck %s
+; RUN: opt %loadexampleirtransforms -passes=tut-simplifycfg -tut-simplifycfg-version=v2 -S < %s | FileCheck %s
+; RUN: opt %loadexampleirtransforms -passes=tut-simplifycfg -tut-simplifycfg-version=v3 -S < %s | FileCheck %s
define i32 @simp1() {
; CHECK-LABEL: @simp1(
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index 500cbd68b494..12d9801050e3 100644
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index 500cbd68b494..12d9801050e3 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -288,6 +288,13 @@ else:
.format(config.llvm_shlib_dir,
config.llvm_shlib_ext)))
+if config.linked_exampleirtransforms_extension:
+ config.substitutions.append(('%loadexampleirtransforms',''))
+else:
+ config.substitutions.append(('%loadexampleirtransforms',
+ '-load-pass-plugin={}/ExampleIRTransforms{}'
+ .format(config.llvm_shlib_dir,
+ config.llvm_shlib_ext)))
# Static libraries are not built if BUILD_SHARED_LIBS is ON.
if not config.build_shared_libs and not config.link_llvm_dylib:
diff --git a/llvm/test/lit.site.cfg.py.in b/llvm/test/lit.site.cfg.py.in
index 5531732e16bc..bc27c04bc9db 100644
--- a/llvm/test/lit.site.cfg.py.in
+++ b/llvm/test/lit.site.cfg.py.in
@@ -53,6 +53,7 @@ config.have_opt_viewer_modules = @LLVM_HAVE_OPT_VIEWER_MODULES@
config.libcxx_used = @LLVM_LIBCXX_USED@
config.has_plugins = @LLVM_ENABLE_PLUGINS@
config.linked_bye_extension = @LLVM_BYE_LINK_INTO_TOOLS@
+config.linked_exampleirtransforms_extension = @LLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS@
config.have_tf_aot = @LLVM_HAVE_TF_AOT@
config.have_tf_api = @LLVM_HAVE_TF_API@
config.llvm_inliner_model_autogenerated = @LLVM_INLINER_MODEL_AUTOGENERATED@
diff --git a/llvm/tools/opt/CMakeLists.txt b/llvm/tools/opt/CMakeLists.txt
index e5d9d28d5006..5d2d4cd5196e 100644
--- a/llvm/tools/opt/CMakeLists.txt
+++ b/llvm/tools/opt/CMakeLists.txt
@@ -38,7 +38,3 @@ add_llvm_tool(opt
SUPPORT_PLUGINS
)
export_executable_symbols_for_plugins(opt)
-
-if(LLVM_BUILD_EXAMPLES)
- target_link_libraries(opt PRIVATE ExampleIRTransforms)
-endif(LLVM_BUILD_EXAMPLES)
diff --git a/llvm/test/lit.site.cfg.py.in b/llvm/test/lit.site.cfg.py.in
index 5531732e16bc..bc27c04bc9db 100644
--- a/llvm/test/lit.site.cfg.py.in
+++ b/llvm/test/lit.site.cfg.py.in
@@ -53,6 +53,7 @@ config.have_opt_viewer_modules = @LLVM_HAVE_OPT_VIEWER_MODULES@
config.libcxx_used = @LLVM_LIBCXX_USED@
config.has_plugins = @LLVM_ENABLE_PLUGINS@
config.linked_bye_extension = @LLVM_BYE_LINK_INTO_TOOLS@
+config.linked_exampleirtransforms_extension = @LLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS@
config.have_tf_aot = @LLVM_HAVE_TF_AOT@
config.have_tf_api = @LLVM_HAVE_TF_API@
config.llvm_inliner_model_autogenerated = @LLVM_INLINER_MODEL_AUTOGENERATED@
diff --git a/llvm/tools/opt/CMakeLists.txt b/llvm/tools/opt/CMakeLists.txt
index e5d9d28d5006..5d2d4cd5196e 100644
--- a/llvm/tools/opt/CMakeLists.txt
+++ b/llvm/tools/opt/CMakeLists.txt
@@ -38,7 +38,3 @@ add_llvm_tool(opt
SUPPORT_PLUGINS
)
export_executable_symbols_for_plugins(opt)
-
-if(LLVM_BUILD_EXAMPLES)
- target_link_libraries(opt PRIVATE ExampleIRTransforms)
-endif(LLVM_BUILD_EXAMPLES)
diff --git a/llvm/tools/opt/opt.cpp b/llvm/tools/opt/opt.cpp
index 8dac02a356b1..123a7209c482 100644
--- a/llvm/tools/opt/opt.cpp
+++ b/llvm/tools/opt/opt.cpp
@@ -392,10 +392,6 @@ static TargetMachine* GetTargetMachine(Triple TheTriple, StringRef CPUStr,
codegen::getExplicitCodeModel(), GetCodeGenOptLevel());
}
-#ifdef BUILD_EXAMPLES
-void initializeExampleIRTransforms(llvm::PassRegistry &Registry);
-#endif
-
struct TimeTracerRAII {
TimeTracerRAII(StringRef ProgramName) {
if (TimeTrace)
@@ -531,10 +527,6 @@ int main(int argc, char **argv) {
initializeReplaceWithVeclibLegacyPass(Registry);
initializeJMCInstrumenterPass(Registry);
-#ifdef BUILD_EXAMPLES
- initializeExampleIRTransforms(Registry);
-#endif
-
SmallVector<PassPlugin, 1> PluginList;
PassPlugins.setCallback([&](const std::string &PluginPath) {
auto Plugin = PassPlugin::Load(PluginPath);
I tried matching `Bye`'s CMakeLists.txt as much as possible, then also copied the lit substitutions to specify the `.so` file when the plugin is dynamically linked. The rest of the changes fix build errors with and without `-DLLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS=ON`.
================
Comment at: llvm/examples/IRTransforms/SimplifyCFG.cpp:40
#include "llvm/IR/PatternMatch.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/CommandLine.h"
----------------
should be able to remove this and Pass.h
================
Comment at: llvm/examples/IRTransforms/SimplifyCFG.cpp:379
case V2: {
- auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
- return doSimplify_v2(F, DT);
+ DominatorTree &DT = FAM.getResult<DominatorTreeAnalysis>(F);
+ doSimplify_v2(F, DT);
----------------
still wrong indentation
================
Comment at: llvm/examples/IRTransforms/SimplifyCFG.cpp:388-389
}
+ default:
+ llvm_unreachable("Unsupported version");
}
----------------
```
warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]
default:
```
should just delete this
================
Comment at: llvm/examples/IRTransforms/SimplifyCFG.cpp:443
+ [](PassBuilder &PB) {
+ PB.registerVectorizerStartEPCallback(
+ [](llvm::FunctionPassManager &PM, OptimizationLevel Level) {
----------------
aeubanks wrote:
> I don't think the legacy pass does this, drop this part?
`registerVectorizerStartEPCallback` part should still be removed
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137103/new/
https://reviews.llvm.org/D137103