Skip to content
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

[CMake] Turn on LLVM_USE_SPLIT_DWARF by default for Linux Debug build #80328

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jsji
Copy link
Member

@jsji jsji commented Feb 1, 2024

split-dwarf feature can help reducing compile time and build footprint.
See examples from:
https://www.productive-cpp.com/improving-cpp-builds-with-split-dwarf/

RelWithDebInfo build of X86 target shows 15%-37% footprint reduction.

The footprint w and w/o this PR are as below:

Default(BUILD_SHARED_LIBS=OFF):

Compile:
63G vs. 100G (37%)

Check-all:
119G vs. 187G (37%)

Shared lib (BUILD_SHARED_LIBS=ON):
Compile:
20G vs. 24G (16%)

Check-all:
28G vs. 33G (15%)

Debuggability should not be affected. Should help with memory usage in linker and compile time,
especially incremental build as well.

RFC: https://discourse.llvm.org/t/rfc-turn-on-llvm-use-split-dwarf-by-default-for-linux-debug-build/76724

split-dwarf feature can help reducing compile time and build footprint.
See examples from:
https://www.productive-cpp.com/improving-cpp-builds-with-split-dwarf/

RelWithDebInfo build of X86 target shows 15%-37% footprint reduction.

The footprint w and w/o this PR are as below:

Default(BUILD_SHARED_LIBS=OFF):

Compile:
 63G vs. 100G  (37%)

Check-all:
 119G vs. 187G  (37%)

Shared lib (BUILD_SHARED_LIBS=ON):
Compile:
 20G vs. 24G (16%)

Check-all:
 28G vs. 33G (15%)

Debugability should not be affected.  Should help with compile time,
especially incremental build as well.
@jsji jsji self-assigned this Feb 2, 2024
@jsji jsji requested a review from joker-eph February 2, 2024 22:20
option(LLVM_USE_SPLIT_DWARF
"Use -gsplit-dwarf when compiling llvm and --gdb-index when linking." OFF)
# Turn on split-dwarf by default for Linux, take effect on Debug/RelWithDbgInfo builds only.
if (CMAKE_SYSTEM_NAME MATCHES "Linux")
Copy link
Member

@MaskRay MaskRay Feb 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps enable this for all ELF OSes as well, including *BSD, Android, Fuchsia.

We only need to disable this for AIX|Darwin|OS390|Windows.
LLVM_USE_SPLIT_DWARF checks whether the toolchain supports -gsplit-dwarf and -Wl,--gdb-index. Having the off default for these non-ELF OSes saves two cc/ld invocations.

@brad0 @DimitryAndric @petrhosek

@ayermolo
Copy link
Contributor

ayermolo commented Feb 5, 2024

GCC and LLVM have now DWARF5 on by default. It has it's own accelerator tables .debug_names.
It looks like side affect of this is that we will also be generating --gdb-index in the linker.
I don't think this is the behavior we want with DWARF5.

@MaskRay
Copy link
Member

MaskRay commented Feb 5, 2024

GCC and LLVM have now DWARF5 on by default. It has it's own accelerator tables .debug_names. It looks like side affect of this is that we will also be generating --gdb-index in the linker. I don't think this is the behavior we want with DWARF5.

I respectfully disagree with changing the default due to the diverse configurations and their inherent trade-offs. -gsplit-dwarf deviates from common practices of other project build systems (this is an advanced option), and the .gdb_index section doesn't benefit lldb users (haha, even if I primarily use gdb due to rr).

I acknowledge that people's opinions differ and some prefer making convenience changes to specific configurations.

Perhaps we should not do too smart things here like making gdb-index LLVM_USE_SPLIT_DWARF the default and add
a .debug_names configuration when linker --debug-names work is complete. These advanced options can be learned by users.

@dwblaikie
Copy link
Collaborator

GCC and LLVM have now DWARF5 on by default. It has it's own accelerator tables .debug_names. It looks like side affect of this is that we will also be generating --gdb-index in the linker. I don't think this is the behavior we want with DWARF5.

I respectfully disagree with changing the default due to the diverse configurations and their inherent trade-offs.

^ this comment is a general one on the whole proposal (here and on discourse) - you're suggesting we should not enable Split DWARF by default for LLVM Debug builds?

nod I don't feel /too/ strongly about it - I don't think most people are terribly storage-constrained to such a degree that this is super important to most LLVM developers. We do document the Split DWARF configuration/suggest it for folks struggling with build times and such.

-gsplit-dwarf deviates from common practices of other project build systems (this is an advanced option), and the .gdb_index section doesn't benefit lldb users (haha, even if I primarily use gdb due to rr).

gdb is still the predominant debugger on these platforms - Clang still defaults to gdb debugger tuning on them, so I don't feel too badly about enabling gdb_index by default on these platforms for LLVM Debug builds. It substantially improves gdb startup time.

Folks using lldb can add -glldb to their build flags if they want to tune for lldb, but it isn't the default.

Perhaps we should not do too smart things here like making gdb-index LLVM_USE_SPLIT_DWARF the default and add a .debug_names configuration when linker --debug-names work is complete. These advanced options can be learned by users.

They can be - but at some cost of failure/frustraction and eventual discovery of these better options. Given how many folks still link with bfd ld despite the time cost because they don't know that's what's happening nor know that there are better options - I'm not against trying to make the defaults better behaved for most people, and I do think gdb_index and Split DWARF are better defaults for most people on the platforms where they're applicable.

@joker-eph
Copy link
Collaborator

I'm not against trying to make the defaults better behaved for most people,

I'm very pro "better default" instead of "default to suboptimal legacy", because of discoverability, etc.
For the longest, the default cmake invocation was a Debug build, and we finally changed this to be explicit instead (that was a case where no default could be found).

ideally we'd be able to detect when the "default" aren't suitable for the current platform/toolchain and error out at CMake configuration time with a very helpful message.

@jsji
Copy link
Member Author

jsji commented Feb 7, 2024

ideally we'd be able to detect when the "default" aren't suitable for the current platform/toolchain and error out at CMake configuration time with a very helpful message.

Thanks @dwblaikie and @joker-eph . Yes, I am happy to proactively work on attempts like #80500 if we know what to detect, just let me know.
There might be situations that we don't know yet, then we can response to problem report in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants