Enable new passmanager plugin support for LTO.

[efriedma]

Summary

This should make both static and dynamic NewPM plugins work with LTO. And as a bonus, it makes static linking of OldPM plugins more reliable for plugins with both an OldPM and NewPM interface.

I only implemented the command-line flag to specify NewPM plugins in llvm-lto2, to show it works. Support can be added for other tools later.

I'm not confident the dynamic plugin bits work the way they should, but it's at least enough to make the included test pass.

[efriedma]

Ping

[serge-sans-paille]

I'd love a test similar to llvm/test/Feature/load_extension.ll in llvm/test/tools/llvm-lto !

[efriedma]

The current version only supports static plugins. Supporting dynamic plugins is a more difficult puzzle because I'd have to figure out how to thread the option through... I'll give it a shot, I guess.

[efriedma]

Added support/tests for dynamically loaded NewPM LTO plugins. (Statically linked plugins can't be tested in regression tests because there are none by default.)

Depending on the review responses, I might split the parts necessary for static plugins into a separate patch, since dynamic plugins are a lot more complicated.

[serge-sans-paille]

Statically linked plugins can't be tested in regression tests because there are none by default

Yeah, if you add

REQUIRES: plugins, examples

you have the guarantee that the `Bye plugin will be compiled. But by default it's compiled dynamically, and you need to set -DLLVM_BYE_LINK_INTO_TOOLS=ON` to have the static test. Your approach looks good.

llvm/test/Feature/load_extension.ll

1 ↗

(On Diff #254662)

why?

llvm/test/lit.cfg.py

212 ↗

(On Diff #254662)

typo: `%loadbyenewpm != %loadnewpmbye`

[efriedma]

llvm/test/Feature/load_extension.ll

1 ↗

(On Diff #254662)

Can't run LTO without specifying a target.

Maybe there's some way to get around that if we don't actually generate code; I can experiment a bit.

[efriedma]

Fix review comment and lint

[efriedma]

llvm/test/Feature/load_extension.ll

1 ↗

(On Diff #254662)

Looked at this more. There isn't any easy solution here, but I guess there are a few options:

1. Add a special mode to LTO where it doesn't require a target machine, and just spits out IR.
2. Force the LLVM default target triple. But it's not clear we can reliably generate an object file with that.
3. Force the LLVM default target triple, and add a mode to LTO where it doesn't run codegen.

[efriedma]

Ping

[serge-sans-paille]

llvm/examples/Bye/Bye.cpp

48 ↗

(On Diff #254881)

Why did you change the extension point? Not that it's particularly important for that pass, just being curious.

llvm/lib/LTO/LTOBackend.cpp

268

That's the exact same lines as above. Maybe it's worth putting that in a function?

llvm/test/Feature/load_extension.ll

1 ↗

(On Diff #254662)

ok, make sense if that's a LTO requirement. I don't think it significantly decreases the coverage of the test.

[efriedma]

llvm/examples/Bye/Bye.cpp

48 ↗

(On Diff #254881)

EP_EarlyAsPossible doesn't run for LTO.

llvm/lib/LTO/LTOBackend.cpp

268

Yes, probably; I'll factor it out.

[efriedma]

Address review comment

[serge-sans-paille]

Thanks @efriedma for the updates. This LGTM, we should probably be waiting for @chandlerc and/or @asbirlea and/or @dblaikie to povide additional opinion though?

[dblaikie]

In D76866#1979678, @serge-sans-paille wrote:

Thanks @efriedma for the updates. This LGTM, we should probably be waiting for @chandlerc and/or @asbirlea and/or @dblaikie to povide additional opinion though?

Was more just meant as an "FYI" to those folks. I thought/think Chandler might have some thoughts on plugin infrastructure for the NPM, but probably best to... plug on in the absence of that feedback & can come back to it if he wants to chime in at some point.

[sylvestre.ledru]

@eli.friedman @serge-sans-paille
Looks like it caused:
https://bugs.llvm.org/show_bug.cgi?id=45571
breaking builds on Debian & Ubuntu

[Meinersbur]

IIUC, this adds statically linked plugins as a link dependency to LLVMLTO. In https://bugs.llvm.org/show_bug.cgi?id=44870 we are currently discussing the downstream issues of these plugins to be added as dependency of clangCodeGen. Are similar problems to be expected for LLVMLTO? Maybe D78192 resolves both.

[efriedma]

That sort of looks related, but I don't think it's precisely the same issue.

I think what's happening here is that when LLVM_LINK_LLVM_DYLIB is enabled, the static plugin somehow acquires a dependency on libLLVM.so. And that's obviously broken: if we expect static plugins to be used as part of LTO, they have to be linked into libLLVM.so, so they can't depend on it.

If we're building libLLVM.so, it probably makes sense to link static plugins into it. I'll have to mess with the CMake a bit to figure out how to make that work.

[efriedma]

Posted https://reviews.llvm.org/D78332

lld/tools/lld/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_lld_tool(lld
   lld.cpp
 
-  ENABLE_PLUGINS
   SUPPORT_PLUGINS
   )
 export_executable_symbols_for_plugins(lld)
 
 target_link_libraries(lld
   PRIVATE
   lldCommon
   lldCOFF

llvm/lib/LTO/CMakeLists.txt

 add_llvm_component_library(LLVMLTO
   Caching.cpp
   LTO.cpp
   LTOBackend.cpp
   LTOModule.cpp
   LTOCodeGenerator.cpp
   SummaryBasedOptimizations.cpp
   UpdateCompilerUsed.cpp
   ThinLTOCodeGenerator.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/LTO
 
+  ENABLE_PLUGINS
+
   DEPENDS
   intrinsics_gen
   llvm_vcsrevision_h
 )

llvm/lib/LTO/LTOBackend.cpp

 #include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Passes/PassBuilder.h"
+#include "llvm/Passes/PassPlugin.h"
 #include "llvm/Passes/StandardInstrumentations.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
CombinedIndexHook =
           reportOpenError(Path, EC.message());
         Index.exportToDot(OSDot, GUIDPreservedSymbols);
         return true;
       };
 
   return Error::success();
 }
 
+#define HANDLE_EXTENSION(Ext)                                                  \
+  llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
+#include "llvm/Support/Extension.def"
+
 namespace {
 
 std::unique_ptr<TargetMachine>
 createTargetMachine(const Config &Conf, const Target *TheTarget, Module &M) {
   StringRef TheTriple = M.getTargetTriple();
   SubtargetFeatures Features;
   Features.getDefaultSubtargetFeatures(Triple(TheTriple));
   for (const std::string &A : Conf.MAttrs)
static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
   SI.registerCallbacks(PIC);
   PassBuilder PB(TM, Conf.PTO, PGOOpt, &PIC);
   AAManager AA;
 
   // Parse a custom AA pipeline if asked to.
   if (auto Err = PB.parseAAPipeline(AA, "default"))
     report_fatal_error("Error parsing default AA pipeline");
 
+#define HANDLE_EXTENSION(Ext)                                                  \
+  get##Ext##PluginInfo().RegisterPassBuilderCallbacks(PB);
+#include "llvm/Support/Extension.def"
+
   LoopAnalysisManager LAM(Conf.DebugPassManager);
   FunctionAnalysisManager FAM(Conf.DebugPassManager);
   CGSCCAnalysisManager CGAM(Conf.DebugPassManager);
   ModuleAnalysisManager MAM(Conf.DebugPassManager);
 
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return std::move(AA); });
 
static void runNewPMCustomPasses(Module &Mod, TargetMachine *TM,
 
   // Register all the basic analyses with the managers.
   PB.registerModuleAnalyses(MAM);
   PB.registerCGSCCAnalyses(CGAM);
   PB.registerFunctionAnalyses(FAM);
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 
   ModulePassManager MPM;
 
   // Always verify the input.
   MPM.addPass(VerifierPass());
 
   // Now, add all the passes we've been requested to.
   if (auto Err = PB.parsePassPipeline(MPM, PipelineDesc))
     report_fatal_error("unable to parse pass pipeline description '" +
                        PipelineDesc + "': " + toString(std::move(Err)));