This is an archive of the discontinued LLVM Phabricator instance.

Support compiler extensions as a normal component
ClosedPublic

Authored by serge-sans-paille on Apr 15 2020, 3:30 AM.

Details

Summary

The approach here is to create a new component, Extensions, where all statically compiled extensions add their dependency. That way we're more natively compatible with LLVMBuild and llvm-config.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2020, 3:30 AM
serge-sans-paille retitled this revision from [WIP] tentative support of compiler extension in llvm-config to Tentative support of compiler extension in llvm-config.

Properly unparse dependency list from plugins into component / lib list, so that we can fill llvm-config with it.

Did you consider making https://github.com/intel/opencl-clang/blob/83837744ef076c1285ed2ee9349febc2208d3252/CMakeLists.txt#L266 work as well. I haven't checked whether Polly is indeed not added by "LINK_COMPONENTS all", but otherwise I think it would not have been reported to not work.

Meinersbur added inline comments.Apr 17 2020, 8:22 PM
llvm/tools/llvm-config/llvm-config.cpp
153

Wasn't the intention to add it only 'all' only? LLVMPasses does not directly depend on statically linked pass extensions.

Meinersbur added inline comments.Apr 19 2020, 10:34 PM
llvm/tools/llvm-config/llvm-config.cpp
153

Thinking about, LLVMPasses seems to be a bag for all passes in other libs, so actually ,might be a good fit. Assuming that when using llvm-config passes -libs, other libraries don't just expect the vanilla passes.

Instead of linking all compiler extensions to the different possible tagets (clang, opt lto etc), link them in a new component, named Extensions. That way a tool (or component) that needs to use compiler extension can just declare that dependency in the traditional way.

This is compatible with llvm-build and llvm-config, through a specific handler in llvm-config though.

Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2020, 7:37 AM

@Meinersbur with the latest updat, it should also be compatible with https://github.com/intel/opencl-clang/blob/83837744ef076c1285ed2ee9349febc2208d3252/CMakeLists.txt#L266 as all contains the Extensions component.

serge-sans-paille retitled this revision from Tentative support of compiler extension in llvm-config to Support compiler extensions as a normal component.
serge-sans-paille edited the summary of this revision. (Show Details)

Note that this should solve in an elegant (?) way the multiple link errors found when linking clang with LLVM_DYLIB.

Is this ready to try out?

Note that this should solve in an elegant (?) way the multiple link errors found when linking clang with LLVM_DYLIB.

Aren't these fixed already?

I looks to work fine under my Windows and Linux configuration. One Point though:

llvm-config --libs Extensions

returns

[...] -lPollyPPCG -lPollyISL -lPolly

I think -lPolly must be first to work with bfd.ld order semantics. See https://stackoverflow.com/questions/45135/why-does-the-order-in-which-libraries-are-linked-sometimes-cause-errors-in-gcc , lld and Microsoft's link.exe don't care less about the order.

Is this ready to try out?

It is!

Note that this should solve in an elegant (?) way the multiple link errors found when linking clang with LLVM_DYLIB.

Aren't these fixed already?

Indeed, but the fix is not generic (it made change to the polly Cmake code), and it's not compatible with Polly declaring component dependencies (see https://reviews.llvm.org/D78358).
Anyway, that's just a side effect of the cleanup, didn't want to emphazise too much on that.

Fix dependency order for extension libraries, as spotted by @Meinersbur

Meinersbur accepted this revision.Apr 23 2020, 2:40 PM

LGTM.

As with every change to the build system, please look out broken configurations after committing. You will have noticed already that there is a lot of variety in how others build LLVM, and its not possible for one person to test them all.

llvm/cmake/modules/AddLLVM.cmake
952–953

[typo] double 'be'

959

In contrast to the LLVMBuild.txt system, this actually uses cmake to get the dependency. Very cool! I am not sure whether LINK_LIBRARIES also includes transitive dependencies.

I think the community would love to see the LLVMBuild.txt gone completely. Interested in working on that?

This revision is now accepted and ready to land.Apr 23 2020, 2:40 PM
This revision was automatically updated to reflect the committed changes.

The patch landed without much errors, only added the patchset e307eeba0137700e75893089cf0de03383d851ca