Page MenuHomePhabricator

Make static linking for LLVM plugins more reliable.
AbandonedPublic

Authored by efriedma on Mar 23 2020, 3:03 PM.

Details

Summary

Linking a static library into a binary doesn't do anything if none of the symbols are used. For some combinations of plugin/host binary, we apparently get lucky (I think due to inline definitions from headers). But in general, it won't work.

To make sure this works properly, use an object library from CMake.

This is a bit hacky; suggestions welcome for a better way to solve the issue.

Diff Detail

Event Timeline

efriedma created this revision.Mar 23 2020, 3:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2020, 3:03 PM
Herald added a subscriber: mgorny. · View Herald Transcript
llvm/cmake/modules/AddLLVM.cmake
928

In that case set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY LINK_LIBRARIES ${llvm_extension}) is no longer needed, is it?

Symbols should be pulled by the responsible tools, e.g. bugpoint: https://reviews.llvm.org/D61446#C1743754NL232 . AFAIK there is no requirements that plugins generate object libraries. For instance, in my typical build configuration, there are no "obj.LLVM*" targets. It depends on the build configuration I guess, whether shared objects and static libraries are both built.

beanz requested changes to this revision.Mar 26 2020, 8:18 AM

Yea... this is not how plugin linking should work.

Plugin host tools should link against the libraries. We do pass linker flags where necessary to reduce symbol dead stripping, but there is no expectation that all static archive symbols are in the tools. Which is why plugins themselves should be linked against first the plugin loader tool, then the LLVM static libraries that they themselves rely on.

See the LLVMHello Transform library plugin as an example.

This revision now requires changes to proceed.Mar 26 2020, 8:18 AM
efriedma abandoned this revision.Mar 26 2020, 9:39 AM

Oh, I didn't realize there was an extra mechanism to pull in static plugins; RegisterStandardPasses doesn't depend on the callback.

I'll look into adding the appropriate calls to LTO.

This is one of the things I insisted on in https://reviews.llvm.org/D61446#1681841. There was a previous discussion to also load plugins in lld for LTO (see https://reviews.llvm.org/D61446?id=198588#inline-548338), but we wanted to finish D61446 first.

In my point of view, having only the new PM cause the plugin symbols to be pulled-in was sufficient, thinking that we will move away from the legacy pass manager anyway.