-
Notifications
You must be signed in to change notification settings - Fork 43
Add basic LLVM_DISTRIBUTION_COMPONENTS compatibility #694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
will this patch allow me to do |
|
Yep! Playing with this a bit though now that you mention it, it looks like doing just a |
great, this is useful
probably need to add LW as ld.eld's dependency |
b5cd5b2 to
0c38316
Compare
Yep! I think that works |
I'm not an expert here/not 100% sure on the exact requirements of LLVM_DISTRIBUTION_COMPONENTS, but this seems to work well enough in that ld.eld and the linker wrappers (riscv-link, etc.) will be installed as expected either through ex: install-distribution or install-ld.eld. There's three parts here: - install-ld.eld and install-ld.eld-stripped needed to be valid targets--I think `add_llvm_install_targets` is the easiest (and a recommended) way to do this. - Without adding the `COMPONENT ld.eld` bits to the `install` commands, ld.eld and the linker wrappers weren't installed. - Without declaring the dependency on LW for the install targets, ld.eld would be installed when using ex: `ninja install-ld.eld`, but would always fail with errors about missing the LW library. Which, that doesn't seem great, so make sure LW is installed first if we're installing eld. Poking at this code a bit, it *might* be possible to simplify things by using ex: `llvm_add_tool_symlink`, but the code added here seems sufficient and less intrusive, so I didn't go that route. Fixes qualcomm#693 Signed-off-by: Jonathon Penix <jpenix@quicinc.com>
75c0d6b to
2a0beca
Compare
|
The header files for linker plugin API need to be in the distribution too. How will that work ? The header files are installed from llvm/tools/eld/include/eld |
|
Oh, whoops. It should just be adding the relevant install targets again (llvm projects do similar things, ex: https://github.com/llvm/llvm-project/blob/de3e9b04178cc8fca77ef6396ba604f47ffdeb6f/clang/CMakeLists.txt#L408-L414) then ex: As a more general question though, does it make sense that these should all be installed/listed separately? Seems like LW is essentially required by eld and the plugin headers are basically always enabled as far as I can tell (the cache var is |
I'm not an expert here/not 100% sure on the exact requirements of
LLVM_DISTRIBUTION_COMPONENTS, but this seems to work well enough in
that ld.eld and the linker wrappers (riscv-link, etc.) will be installed
as expected either through ex: install-distribution or install-ld.eld.
There's three parts here:
think
add_llvm_install_targetsis the easiest (and a recommended) wayto do this.
COMPONENT ld.eldbits to theinstallcommands,ld.eld and the linker wrappers weren't installed.
would be installed when using ex:
ninja install-ld.eld, but wouldalways fail with errors about missing the LW library. Which, that
doesn't seem great, so make sure LW is installed first if we're
installing eld.
Poking at this code a bit, it might be possible to simplify things by
using ex:
llvm_add_tool_symlink, but the code added here seemssufficient and less intrusive, so I didn't go that route.
Fixes #693