Page MenuHomePhabricator

[gold] Add support for loading pass plugins
Needs ReviewPublic

Authored by ddcc on Apr 7 2020, 9:25 PM.

Details

Summary

Add a -load and -load-pass-plugin for dynamically loading plugin passes into old/new PM during LTO when using gold. LLVM already exposes LTO pass extension points, e.g. EP_FullLinkTimeOptimizationEarly, but currently the gold plugin doesn't seem to support actually loading plugins.

Diff Detail

Event Timeline

ddcc created this revision.Apr 7 2020, 9:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2020, 9:25 PM

Maybe add a parent child relationship with D77705. I have some familiarity with gold plugins and can take a look at this, could you please add more details to the summary?

ddcc added a comment.EditedApr 8 2020, 11:07 PM

Sure. I've written some local optimizations in a loadable pass that uses one of the LTO extension points, EP_FullLinkTimeOptimizationEarly, but it seems that there's no way to actually load out-of-tree LTO passes into gold. This patch fixes that by modifying the gold plugin to support two additional arguments for loading external passes with the old/new pass manager, though I've only tested it with my local legacy pass.

However, I wasn't able to successfully load my pass until I rebuilt my local Clang/LLVM with LLVM_LINK_LLVM_DYLIB so that everything is dynamically linked to libLLVM.so. Otherwise, when both the loadable pass and the LLVM gold plugin are built statically, everything is initialized twice, which leads to an error at runtime about duplicate argument registration. Alternatively, when the loadable pass is built dynamically without an explicit dependency on libLLVM.so, it wasn't able to reuse the LLVM symbols provided by a statically-linked LLVM gold plugin that is already loaded, because gold doesn't call dlopen with RTLD_GLOBAL when it is loading gold plugins, and the LLVM gold plugin has a LLVM_EXPORTED_SYMBOL_FILE that restricts exported symbols. Furthermore, loadable passes can depend on other components of LLVM, e.g. my pass uses the Demangler, which isn't linked into the LLVM gold plugin anyway. I'm not too familiar with pass registration and this part of the LLVM codebase, so perhaps there's a better way to get things working.

Edit 1: lld might have a similar problem, but I didn't look into it since my local toolchain is only using gold.

Edit 2: The arguments for loading plugins seem to be inconsistent between opt (-load/-load-pass-plugin) and clang (-Xclang -load/-fplugin/-fplugin-pass), so I just reused the former.

ddcc edited the summary of this revision. (Show Details)Apr 8 2020, 11:09 PM

Sure. I've written some local optimizations in a loadable pass that uses one of the LTO extension points, EP_FullLinkTimeOptimizationEarly, but it seems that there's no way to actually load out-of-tree LTO passes into gold. This patch fixes that by modifying the gold plugin to support two additional arguments for loading external passes with the old/new pass manager, though I've only tested it with my local legacy pass.

However, I wasn't able to successfully load my pass until I rebuilt my local Clang/LLVM with LLVM_LINK_LLVM_DYLIB so that everything is dynamically linked to libLLVM.so. Otherwise, when both the loadable pass and the LLVM gold plugin are built statically, everything is initialized twice, which leads to an error at runtime about duplicate argument registration. Alternatively, when the loadable pass is built dynamically without an explicit dependency on libLLVM.so, it wasn't able to reuse the LLVM symbols provided by a statically-linked LLVM gold plugin that is already loaded, because gold doesn't call dlopen with RTLD_GLOBAL when it is loading gold plugins, and the LLVM gold plugin has a LLVM_EXPORTED_SYMBOL_FILE that restricts exported symbols. Furthermore, loadable passes can depend on other components of LLVM, e.g. my pass uses the Demangler, which isn't linked into the LLVM gold plugin anyway. I'm not too familiar with pass registration and this part of the LLVM codebase, so perhaps there's a better way to get things working.

Thanks for providing more details. I do understand your issue with LLVM gold plugin and how libLLVM is linked but I dont have a good solution. Maybe you can mail llvm-dev to see if this can be worked out.

Edit 1: lld might have a similar problem, but I didn't look into it since my local toolchain is only using gold.

Edit 2: The arguments for loading plugins seem to be inconsistent between opt (-load/-load-pass-plugin) and clang (-Xclang -load/-fplugin/-fplugin-pass), so I just reused the former.

Alright, is it feasible to add small tests or provide instructions on how you would load an example pass?

Sorry if my comments aren't too helpful :(.

ddcc added a comment.EditedApr 13 2020, 10:22 AM

Alright, is it feasible to add small tests or provide instructions on how you would load an example pass?

Sure, I'm just not sure how to fit this into the testing infrastructure. I don't think this could work with any of the FileCheck-based test since I'd need to build a separate pass and load the binary? I can also modify the GoldPlugin.rst to mention these arguments for loading passes? Ideally though, it would be pretty transparent; once the Clang driver changes land, there'd be no difference compared to loading a regular LLVM pass.

Do you have suggestions for any other reviewers?

It looks like this overlaps with https://reviews.llvm.org/D76866 ; maybe wait for that to merge, then we can handle the gold-specific changes as a followup.

(Probably you need export_executable_symbols_for_plugins to fix the missing symbol issues.)

ddcc added a comment.Apr 13 2020, 12:05 PM

It looks like this overlaps with https://reviews.llvm.org/D76866 ; maybe wait for that to merge, then we can handle the gold-specific changes as a followup.

(Probably you need export_executable_symbols_for_plugins to fix the missing symbol issues.)

Oh, didn't see that you're working on something similar. Sure, thanks for the suggestions.

ddcc added a comment.Apr 14 2020, 10:39 PM

Here's the updated patch, though it still only works when LLVM is built with LLVM_LINK_LLVM_DYLIB=ON. Otherwise, with or without the LLVM_EXPORTED_SYMBOL_FILE definition, I get the following error with a static build:

Unable to load plugin: <pass>.so: undefined symbol: _ZNK4llvm4Pass11getPassNameEv
ddcc retitled this revision from [LTO][gold] Add support for loading pass plugins to [gold] Add support for loading pass plugins.Apr 14 2020, 10:58 PM
efriedma added inline comments.Apr 15 2020, 12:39 PM
llvm/tools/gold/CMakeLists.txt
0–1

Oh, duh, I just realized the problem with the symbol exports. The LLVM_EXPORTED_SYMBOL_FILE line suppresses exporting all the symbols pass plugins would normally use.

(And I should have realized export_executable_symbols_for_plugins shouldn't be necessary for a shared library: shared libraries export all symbols by default.)

ddcc marked an inline comment as done.Apr 15 2020, 4:22 PM
ddcc added inline comments.
llvm/tools/gold/CMakeLists.txt
0–1

Hmm, I tried both with and without LLVM_EXPORTED_SYMBOL_FILE, but it didn't seem to make a difference.

I think the fundamental problem here is that when gold calls dlopen(LLVMgold.so, RTLD_LAZY), it doesn't load provide the RTLD_GLOBAL argument, so any symbols in LLVMgold.so aren't available to subsequent dynamically-loaded plugins.

efriedma added inline comments.Apr 15 2020, 4:45 PM
llvm/tools/gold/CMakeLists.txt
0–1

Oh, hmm, that makes sense. That's harder to solve.

We could make LLVMgold.so dlopen() itself, I guess? That seems terribly hacky, though.

ddcc marked an inline comment as done.Apr 16 2020, 7:00 PM
ddcc added inline comments.
llvm/tools/gold/CMakeLists.txt
0–1

I haven't tried it, but I'm not sure if that would work. Wouldn't there be two copies of LLVM loaded into gold, with each initialized separately? It seems like a proper fix would be in gold itself, but otherwise, I'd consider having this working for LLVM_LINK_LLVM_DYLIB still an improvement.

efriedma added inline comments.Apr 17 2020, 11:29 AM
llvm/tools/gold/CMakeLists.txt
0–1

Shared libraries are reference-counted, so it should be possible to do this without loading a second copy.

ddcc updated this revision to Diff 259981.Apr 24 2020, 1:51 PM

Support statically-built passes

Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2020, 1:51 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ddcc marked an inline comment as done.Apr 24 2020, 1:53 PM

Thanks, seems to be working now for statically-built passes on a default LLVM build without LLVM_LINK_LLVM_DYLIB.

ddcc added a comment.Apr 24 2020, 1:56 PM

In order to reload the gold plugin, I'm modifying the Clang driver to pass in our own path as a separate argument, which is the most generic approach. Another method would be to use e.g. dladdr() to grab our own path from one of our exported functions, but that method appears to be a glibc extension which isn't cross-platform.

ddcc added a comment.May 17 2020, 4:33 PM

ping, any feedback?