Page MenuHomePhabricator

[cmake] Add LLVM_USE_GDB_INDEX option for gold/lld linkers
Needs ReviewPublic

Authored by ddcc on Mar 31 2020, 11:00 AM.

Details

Summary

Add option for -Wl,--gdb-index and update documentation

Diff Detail

Event Timeline

ddcc created this revision.Mar 31 2020, 11:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2020, 11:00 AM
ddcc retitled this revision from [cmake] Add LLVM_USE_GDB_INDEX option for gold linker to [cmake] Add LLVM_USE_GDB_INDEX option for gold/lld linkers.Mar 31 2020, 12:00 PM
MaskRay added a subscriber: MaskRay.EditedMar 31 2020, 2:18 PM

This seems fine. I have -DCMAKE_EXE_LINKER_FLAGS=-Wl,--gdb-index -DCMAKE_SHARED_LINKER_FLAGS=-Wl,--gdb-index -DLLVM_USE_SPLIT_DWARF=On in my long cmake command line. I am just wondering whether this new option will be discoverable.

Some people don't use -DLLVM_USE_LINKER=lld, but use -DLLVM_ENABLE_LLD=On.

ddcc added a comment.Mar 31 2020, 2:29 PM

Some people don't use -DLLVM_USE_LINKER=lld, but use -DLLVM_ENABLE_LLD=On.

It's a bit confusing, but earlier in HandleLLVMOptions.cmake, LLVM_ENABLE_LLD is normalized into LLVM_USE_LINKER. There's also an internal variable LINKER_IS_LLD_LINK that is set by either.

hliao added a comment.Mar 31 2020, 2:39 PM

The same patch posted previously, https://reviews.llvm.org/D58628

hliao added a comment.Mar 31 2020, 2:42 PM

This seems fine. I have -DCMAKE_EXE_LINKER_FLAGS=-Wl,--gdb-index -DCMAKE_SHARED_LINKER_FLAGS=-Wl,--gdb-index -DLLVM_USE_SPLIT_DWARF=On in my long cmake command line. I am just wondering whether this new option will be discoverable.

Some people don't use -DLLVM_USE_LINKER=lld, but use -DLLVM_ENABLE_LLD=On.

As I mentioned in https://reviews.llvm.org/D58628, if your system's default linker doesn't recognize --gdb-index, adding option into CMAKE_EXE_LINKER_FLAGS fails the make detection stage, where the default linker is used.

ddcc added a subscriber: smeenai.Mar 31 2020, 3:15 PM

The same patch posted previously, https://reviews.llvm.org/D58628

Oops, I saw your patch to LLVM_USE_SPLIT_DWARF that landed, but missed that one.

As I mentioned in https://reviews.llvm.org/D58628, if your system's default linker doesn't recognize --gdb-index, adding option into CMAKE_EXE_LINKER_FLAGS fails the make detection stage, where the default linker is used.

It seems like -fuse-ld=gold/lld would be the simplest way forward? Do you plan to revise your patch to address that, lld, and @smeenai comment about -ggnu-pubnames? My only other feedback would be to update the docs, since even LLVM_USE_SPLIT_DWARF isn't currently listed, and that incorporating objcopy would make things more complicated.

This seems fine. I have -DCMAKE_EXE_LINKER_FLAGS=-Wl,--gdb-index -DCMAKE_SHARED_LINKER_FLAGS=-Wl,--gdb-index -DLLVM_USE_SPLIT_DWARF=On in my long cmake command line. I am just wondering whether this new option will be discoverable.

Some people don't use -DLLVM_USE_LINKER=lld, but use -DLLVM_ENABLE_LLD=On.

As I mentioned in https://reviews.llvm.org/D58628, if your system's default linker doesn't recognize --gdb-index, adding option into CMAKE_EXE_LINKER_FLAGS fails the make detection stage, where the default linker is used.

Perhaps that issue should be fixed (so that LLVM_ENABLE_LLD/LLVM_USE_LINKER kicks in early enough (same as putting -fuse-ld in CMAKE_EXE_LINKER_FLAGS) - seems a bit problematic to have different compilation modes at different points along that (but I guess make those LLD/linker enabling features are designed to use make detection to silently not do the thing you asked for if it's unsupported)

To my mind this all seems a bit complicated - and I wonder what the point of all these cmake supported features is compared to users specifying the flags in the flag variables directly?