Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Nov 25, 2024

As far as I can tell there are 2 parallel plugin mechanisms.
opt -load=plugin does not work, and is ignored. opt -load-pass-plugin
does work. PluginLoader.h forces a static definition of the "load" cl::opt into included TUs. Delete the cases with no tests.

Copy link
Contributor Author

arsenm commented Nov 25, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

@arsenm arsenm added the llvm Umbrella label for LLVM issues label Nov 25, 2024 — with Graphite App
@arsenm arsenm requested a review from aeubanks November 25, 2024 23:35
@arsenm arsenm marked this pull request as ready for review November 25, 2024 23:35
@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2024

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Matt Arsenault (arsenm)

Changes

As far as I can tell there are 2 parallel plugin mechanisms.
opt -load=plugin does not work, and is ignored. opt -load-pass-plugin
does work. The only user of PluginLoader appears to be bugpoint.


Full diff: https://github.com/llvm/llvm-project/pull/117644.diff

6 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (-1)
  • (modified) lld/tools/lld/lld.cpp (-1)
  • (modified) llvm/tools/llc/llc.cpp (-1)
  • (modified) llvm/tools/lli/lli.cpp (-1)
  • (modified) llvm/tools/llvm-lto2/llvm-lto2.cpp (-1)
  • (modified) llvm/tools/opt/optdriver.cpp (-1)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index d42dafa8ffc362..6e6bda6774da68 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -21,7 +21,6 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/InitLLVM.h"
-#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
diff --git a/lld/tools/lld/lld.cpp b/lld/tools/lld/lld.cpp
index d6800fa1eea4b9..3aaed7e83104fc 100644
--- a/lld/tools/lld/lld.cpp
+++ b/lld/tools/lld/lld.cpp
@@ -35,7 +35,6 @@
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/LLVMDriver.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
 #include "llvm/TargetParser/Host.h"
 #include "llvm/TargetParser/Triple.h"
diff --git a/llvm/tools/llc/llc.cpp b/llvm/tools/llc/llc.cpp
index 150dd50ef2936e..88168cc7e24303 100644
--- a/llvm/tools/llc/llc.cpp
+++ b/llvm/tools/llc/llc.cpp
@@ -44,7 +44,6 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/InitLLVM.h"
-#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
diff --git a/llvm/tools/lli/lli.cpp b/llvm/tools/lli/lli.cpp
index dd275b73a0c7ec..21da7fe02b9569 100644
--- a/llvm/tools/lli/lli.cpp
+++ b/llvm/tools/lli/lli.cpp
@@ -60,7 +60,6 @@
 #include "llvm/Support/Memory.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/SourceMgr.h"
diff --git a/llvm/tools/llvm-lto2/llvm-lto2.cpp b/llvm/tools/llvm-lto2/llvm-lto2.cpp
index d4f022ef021a44..aefdf12a5d4e91 100644
--- a/llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ b/llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -25,7 +25,6 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
-#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Threading.h"
 #include <atomic>
diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp
index 8ef249e1708b95..e6c3928396f9d3 100644
--- a/llvm/tools/opt/optdriver.cpp
+++ b/llvm/tools/opt/optdriver.cpp
@@ -43,7 +43,6 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
-#include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/SystemUtils.h"
 #include "llvm/Support/TargetSelect.h"

@AaronBallman
Copy link
Collaborator

Can we get rid of PluginLoader.h/cpp now as well? Also, precommit CI failures look relevant.

@arsenm
Copy link
Contributor Author

arsenm commented Nov 26, 2024

Apparently the header adds a static define of the option into the included header. This means that it's untested/unused in the llc, lld, lli, llvm-lto, and opt cases. It does appear to have some tests for bug point and clang tidy

@arsenm arsenm force-pushed the users/arsenm/tools-remove-plugin-loader-includes branch from e713e5f to e2e6d24 Compare November 26, 2024 17:54
@arsenm arsenm changed the title tools: Remove unused PluginLoader includes tools: Remove untested PluginLoader includes Nov 26, 2024
@nikic
Copy link
Contributor

nikic commented Nov 26, 2024

Hm, I would have expected the removal in opt to break polly tests using %loadPolly (which still use the legacy PM).

@HerrCai0907 HerrCai0907 removed their request for review January 17, 2025 13:42
@arsenm arsenm force-pushed the users/arsenm/tools-remove-plugin-loader-includes branch 3 times, most recently from 130ee9e to fea958e Compare March 7, 2025 05:22
@arsenm
Copy link
Contributor Author

arsenm commented Mar 13, 2025

ping

As far as I can tell there are 2 parallel plugin mechanisms.
opt -load=plugin does not work, and is ignored. opt -load-pass-plugin
does work. The only user of PluginLoader appears to be bugpoint.
@arsenm arsenm force-pushed the users/arsenm/tools-remove-plugin-loader-includes branch from fea958e to 49ca1c9 Compare November 11, 2025 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang-tidy clang-tools-extra lld llvm Umbrella label for LLVM issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants