[llvm] f617ab1 - [NPM] Resolve llvmGetPassPluginInfo to the plugin being loaded
Tomas Matheson via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 30 10:11:36 PDT 2021
Author: Tomas Matheson
Date: 2021-06-30T18:11:28+01:00
New Revision: f617ab10445148ae44d67484f9dc9486efcfbcc8
URL: https://github.com/llvm/llvm-project/commit/f617ab10445148ae44d67484f9dc9486efcfbcc8
DIFF: https://github.com/llvm/llvm-project/commit/f617ab10445148ae44d67484f9dc9486efcfbcc8.diff
LOG: [NPM] Resolve llvmGetPassPluginInfo to the plugin being loaded
Dynamically loaded plugins for the new pass manager are initialised by
calling llvmGetPassPluginInfo. This is defined as a weak symbol so that
it is continually redefined by each plugin that is loaded. When loading
a plugin from a shared library, the intention is that
llvmGetPassPluginInfo will be resolved to the definition in the most
recent plugin. However, using a global search for this resolution can
fail in situations where multiple plugins are loaded.
Currently:
* If a plugin does not define llvmGetPassPluginInfo, then it will be
silently resolved to the previous plugin's definition.
* If loading the same plugin twice with another in between, e.g. plugin
A/plugin B/plugin A, then the second load of plugin A will resolve to
llvmGetPassPluginInfo in plugin B.
* The previous case can also occur when a dynamic library defines both
NPM and legacy plugins; the legacy plugins are loaded first and then
with `-fplugin=A -fpass-plugin=B -fpass-plugin=A`: A will be loaded as
a legacy plugin and define llvmGetPassPluginInfo; B will be loaded
and redefine it; and finally when A is loaded as an NPM plugin it will
be resolved to the definition from B.
Instead of searching globally, restrict the symbol lookup to the library
that is currently being loaded.
Differential Revision: https://reviews.llvm.org/D104916
Added:
llvm/unittests/Passes/DoublerPlugin.cpp
Modified:
llvm/lib/Passes/PassPlugin.cpp
llvm/unittests/Passes/CMakeLists.txt
llvm/unittests/Passes/PluginsTest.cpp
llvm/unittests/Passes/TestPlugin.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Passes/PassPlugin.cpp b/llvm/lib/Passes/PassPlugin.cpp
index ceefa25a703b6..6182cbbb1fddd 100644
--- a/llvm/lib/Passes/PassPlugin.cpp
+++ b/llvm/lib/Passes/PassPlugin.cpp
@@ -23,8 +23,11 @@ Expected<PassPlugin> PassPlugin::Load(const std::string &Filename) {
inconvertibleErrorCode());
PassPlugin P{Filename, Library};
+
+ // llvmGetPassPluginInfo should be resolved to the definition from the plugin
+ // we are currently loading.
intptr_t getDetailsFn =
- (intptr_t)Library.SearchForAddressOfSymbol("llvmGetPassPluginInfo");
+ (intptr_t)Library.getAddressOfSymbol("llvmGetPassPluginInfo");
if (!getDetailsFn)
// If the symbol isn't found, this is probably a legacy plugin, which is an
diff --git a/llvm/unittests/Passes/CMakeLists.txt b/llvm/unittests/Passes/CMakeLists.txt
index 50bf3901ab1ea..075b47c8d07e6 100644
--- a/llvm/unittests/Passes/CMakeLists.txt
+++ b/llvm/unittests/Passes/CMakeLists.txt
@@ -1,5 +1,5 @@
# Needed by LLVM's CMake checks because this file defines multiple targets.
-set(LLVM_OPTIONAL_SOURCES PluginsTest.cpp TestPlugin.cpp PassBuilderBindingsTest.cpp)
+set(LLVM_OPTIONAL_SOURCES PluginsTest.cpp TestPlugin.cpp DoublerPlugin.cpp PassBuilderBindingsTest.cpp)
# If plugins are disabled, this test will disable itself at runtime. Otherwise,
# reconfiguring with plugins disabled will leave behind a stale executable.
@@ -20,19 +20,20 @@ if (NOT WIN32)
target_link_libraries(PluginsTests PRIVATE LLVMTestingSupport)
set(LLVM_LINK_COMPONENTS)
- add_llvm_library(TestPlugin MODULE BUILDTREE_ONLY
- TestPlugin.cpp
- )
+ foreach(PLUGIN TestPlugin DoublerPlugin)
+ add_llvm_library(${PLUGIN} MODULE BUILDTREE_ONLY ${PLUGIN}.cpp)
- # Put plugin next to the unit test executable.
- set_output_directory(TestPlugin
- BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}
- LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}
- )
- set_target_properties(TestPlugin PROPERTIES FOLDER "Tests")
+ # Put PLUGIN next to the unit test executable.
+ set_output_directory(${PLUGIN}
+ BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}
+ LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}
+ )
+ set_target_properties(${PLUGIN} PROPERTIES FOLDER "Tests")
+
+ add_dependencies(${PLUGIN} intrinsics_gen)
+ add_dependencies(PluginsTests ${PLUGIN})
+ endforeach()
- add_dependencies(TestPlugin intrinsics_gen)
- add_dependencies(PluginsTests TestPlugin)
endif()
set(LLVM_LINK_COMPONENTS Support Passes Core Target native AllTargetsInfos)
@@ -40,3 +41,4 @@ add_llvm_unittest(PassesBindingsTests
PassBuilderBindingsTest.cpp
)
target_link_libraries(PassesBindingsTests PRIVATE LLVMTestingSupport)
+
diff --git a/llvm/unittests/Passes/DoublerPlugin.cpp b/llvm/unittests/Passes/DoublerPlugin.cpp
new file mode 100644
index 0000000000000..57d48261796e2
--- /dev/null
+++ b/llvm/unittests/Passes/DoublerPlugin.cpp
@@ -0,0 +1,44 @@
+//===- unittests/Passes/DoublerPlugin.cpp
+//--------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Passes/PassBuilder.h"
+#include "llvm/Passes/PassPlugin.h"
+
+using namespace llvm;
+
+struct DoublerModulePass : public PassInfoMixin<DoublerModulePass> {
+
+ // Double the value of the initializer
+ PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM) {
+ auto *GV = cast<GlobalVariable>(M.getNamedValue("doubleme"));
+ auto *Init = GV->getInitializer();
+ auto *Init2 = ConstantExpr::getAdd(Init, Init);
+ GV->setInitializer(Init2);
+
+ return PreservedAnalyses::none();
+ }
+
+ static void registerCallbacks(PassBuilder &PB) {
+ PB.registerPipelineParsingCallback(
+ [](StringRef Name, ModulePassManager &PM,
+ ArrayRef<PassBuilder::PipelineElement> InnerPipeline) {
+ if (Name == "doubler-pass") {
+ PM.addPass(DoublerModulePass());
+ return true;
+ }
+ return false;
+ });
+ }
+};
+
+extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
+llvmGetPassPluginInfo() {
+ return {LLVM_PLUGIN_API_VERSION, "DoublerPlugin", "2.2-unit",
+ DoublerModulePass::registerCallbacks};
+}
diff --git a/llvm/unittests/Passes/PluginsTest.cpp b/llvm/unittests/Passes/PluginsTest.cpp
index 9fa5a0bdaf983..49fc8284f5e63 100644
--- a/llvm/unittests/Passes/PluginsTest.cpp
+++ b/llvm/unittests/Passes/PluginsTest.cpp
@@ -7,7 +7,9 @@
//===----------------------------------------------------------------------===//
#include "llvm/Analysis/CGSCCPassManager.h"
+#include "llvm/AsmParser/Parser.h"
#include "llvm/Config/config.h"
+#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Passes/PassBuilder.h"
#include "llvm/Passes/PassPlugin.h"
@@ -58,3 +60,80 @@ TEST(PluginsTests, LoadPlugin) {
Plugin->registerPassBuilderCallbacks(PB);
ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, "plugin-pass"), Succeeded());
}
+
+// Test that llvmGetPassPluginInfo from DoublerPlugin is called twice with
+// -fpass-plugin=DoublerPlugin -fpass-plugin=TestPlugin
+// -fpass-plugin=DoublerPlugin.
+TEST(PluginsTests, LoadMultiplePlugins) {
+#if !defined(LLVM_ENABLE_PLUGINS)
+ // Disable the test if plugins are disabled.
+ return;
+#endif
+
+ auto DoublerPluginPath = LibPath("DoublerPlugin");
+ auto TestPluginPath = LibPath("TestPlugin");
+ ASSERT_NE("", DoublerPluginPath);
+ ASSERT_NE("", TestPluginPath);
+
+ Expected<PassPlugin> DoublerPlugin1 = PassPlugin::Load(DoublerPluginPath);
+ ASSERT_TRUE(!!DoublerPlugin1)
+ << "Plugin path: " << DoublerPlugin1->getFilename();
+
+ Expected<PassPlugin> TestPlugin = PassPlugin::Load(TestPluginPath);
+ ASSERT_TRUE(!!TestPlugin) << "Plugin path: " << TestPlugin->getFilename();
+
+ // If llvmGetPassPluginInfo is resolved as a weak symbol taking into account
+ // all loaded symbols, the second call to PassPlugin::Load will actually
+ // return the llvmGetPassPluginInfo from the most recently loaded plugin, in
+ // this case TestPlugin.
+ Expected<PassPlugin> DoublerPlugin2 = PassPlugin::Load(DoublerPluginPath);
+ ASSERT_TRUE(!!DoublerPlugin2)
+ << "Plugin path: " << DoublerPlugin2->getFilename();
+
+ ASSERT_EQ("DoublerPlugin", DoublerPlugin1->getPluginName());
+ ASSERT_EQ("2.2-unit", DoublerPlugin1->getPluginVersion());
+ ASSERT_EQ(TEST_PLUGIN_NAME, TestPlugin->getPluginName());
+ ASSERT_EQ(TEST_PLUGIN_VERSION, TestPlugin->getPluginVersion());
+ // Check that the plugin name/version is set correctly when loaded a second
+ // time
+ ASSERT_EQ("DoublerPlugin", DoublerPlugin2->getPluginName());
+ ASSERT_EQ("2.2-unit", DoublerPlugin2->getPluginVersion());
+
+ PassBuilder PB;
+ ModulePassManager PM;
+ const char *PipelineText = "module(doubler-pass,plugin-pass,doubler-pass)";
+ ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText), Failed());
+ TestPlugin->registerPassBuilderCallbacks(PB);
+ DoublerPlugin1->registerPassBuilderCallbacks(PB);
+ DoublerPlugin2->registerPassBuilderCallbacks(PB);
+ ASSERT_THAT_ERROR(PB.parsePassPipeline(PM, PipelineText), Succeeded());
+
+ LLVMContext C;
+ SMDiagnostic Err;
+ std::unique_ptr<Module> M =
+ parseAssemblyString(R"IR(@doubleme = constant i32 7)IR", Err, C);
+
+ // Check that the initial value is 7
+ {
+ auto *GV = M->getNamedValue("doubleme");
+ auto *Init = cast<GlobalVariable>(GV)->getInitializer();
+ auto *CI = cast<ConstantInt>(Init);
+ ASSERT_EQ(CI->getSExtValue(), 7);
+ }
+
+ ModuleAnalysisManager MAM;
+ // Register required pass instrumentation analysis.
+ MAM.registerPass([&] { return PassInstrumentationAnalysis(); });
+ PM.run(*M, MAM);
+
+ // Check that the final value is 28 because DoublerPlugin::run was called
+ // twice, indicating that the llvmGetPassPluginInfo and registerCallbacks
+ // were correctly called.
+ {
+ // Check the value was doubled twice
+ auto *GV = M->getNamedValue("doubleme");
+ auto *Init = cast<GlobalVariable>(GV)->getInitializer();
+ auto *CI = cast<ConstantInt>(Init);
+ ASSERT_EQ(CI->getSExtValue(), 28);
+ }
+}
diff --git a/llvm/unittests/Passes/TestPlugin.cpp b/llvm/unittests/Passes/TestPlugin.cpp
index e0ae861f16fc1..edc71aaf3ff07 100644
--- a/llvm/unittests/Passes/TestPlugin.cpp
+++ b/llvm/unittests/Passes/TestPlugin.cpp
@@ -1,4 +1,4 @@
-//===- unittests/Passes/Plugins/Plugin.cpp --------------------------------===//
+//===- unittests/Passes/TestPlugin.cpp --------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -17,22 +17,22 @@ struct TestModulePass : public PassInfoMixin<TestModulePass> {
PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM) {
return PreservedAnalyses::all();
}
-};
-void registerCallbacks(PassBuilder &PB) {
- PB.registerPipelineParsingCallback(
- [](StringRef Name, ModulePassManager &PM,
- ArrayRef<PassBuilder::PipelineElement> InnerPipeline) {
- if (Name == "plugin-pass") {
- PM.addPass(TestModulePass());
- return true;
- }
- return false;
- });
-}
+ static void registerCallbacks(PassBuilder &PB) {
+ PB.registerPipelineParsingCallback(
+ [](StringRef Name, ModulePassManager &PM,
+ ArrayRef<PassBuilder::PipelineElement> InnerPipeline) {
+ if (Name == "plugin-pass") {
+ PM.addPass(TestModulePass());
+ return true;
+ }
+ return false;
+ });
+ }
+};
extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
llvmGetPassPluginInfo() {
return {LLVM_PLUGIN_API_VERSION, TEST_PLUGIN_NAME, TEST_PLUGIN_VERSION,
- registerCallbacks};
+ TestModulePass::registerCallbacks};
}
More information about the llvm-commits
mailing list