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

Compilation: Enable LTO for libraries by default #22228

Closed
wants to merge 1 commit into from

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Dec 13, 2024

Just curious to see if this'll work in CI. If it does, we can merge after #22171 to avoid conflicts in that.

@TCROC
Copy link
Contributor

TCROC commented Dec 14, 2024

LTO in the std lib is gonna be 🔥

@alexrp alexrp closed this Dec 14, 2024
@alexrp alexrp deleted the lto-libs branch December 14, 2024 05:08
@TCROC
Copy link
Contributor

TCROC commented Dec 14, 2024

@alexrp did you have issues with lto on std lib?

@alexrp
Copy link
Member Author

alexrp commented Dec 14, 2024

LTO doesn't really do much for the Zig standard library because Zig uses a "unity build" compilation model where all the Zig code reachable from the root module is compiled and optimized together.

LTO is mainly relevant when objects compiled from Zig code and C-family code are linked together, or when building pure C-family code. I guess technically also if you build a Zig static library with a C API and link it to other Zig code... though that seems a rather esoteric/pointless use case; just call the Zig code directly instead.

Anyway, I closed this PR because it exposed (in CI) some fundamental flaws in LLD's LTO support which currently lead to nasty miscompilations on some targets. Fixing this would require significant design and implementation work in LLVM and LLD, and would be pointless since we're most likely dropping the LLD dependency in the next release cycle anyway.

In fact, the results of this PR shifted my opinion in the opposite direction: I'm going to disable LTO by default instead. Better to have people explicitly opt into it.

@TCROC
Copy link
Contributor

TCROC commented Dec 14, 2024

Sounds like a plan. When passing commands like -flto=full and -flto=thin, does zig apply lto to c and c++ std libs via zig cc and zig c++?

@alexrp
Copy link
Member Author

alexrp commented Dec 14, 2024

  • Yes for any statically-linked libc bundled with Zig (musl, MinGW-w64, wasi-libc). Also yes for bundled libc++ and libc++abi.
  • No for bundled libunwind because of an LLVM bug that still isn't fixed.
  • No for any dynamically-linked libc bundled with Zig (musl, glibc, libSystem). LTO would make no sense there since we link against libc stub symbols, not real code.
  • No for any non-bundled stdlib.

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.

2 participants