[libcxx-commits] [PATCH] D152967: [runtimes] Enable LTO when supported

Thu Jun 15 00:21:21 PDT 2023

phosek added subscribers: paulkirth, phosek.
phosek added a comment.

In D152967#4423591 <https://reviews.llvm.org/D152967#4423591>, @mstorsjo wrote:

> While enabling LTO, if possible probably is fine for the shared library, it's a much less clear cut deal for the static library.
>
> Regarding "if possible", the CI run seems to show that it was enabled in cases where it probably didn't work in the end - e.g. the CI linux configurations, compiling with clang but linking with the system GNU linkers, probably without a suitable LLVM LTO plugin, or perhaps it's a case where it would need to use `llvm-ar` instead of GNU `ar` or something like that.
>
> With the static library, if built with LTO, it pushes the compilation of the library onto every single user who links an executable. If linking a single executable that's probably fine, but for a generic toolchain that can be used in a multitude of ways, it's probably a different tradeoff. If linking e.g. hundreds of small executables against a static libc++ (for users who don't enable LTO otherwise), you'd now end up compiling the libc++ code hundreds of times, with no actual benefit if the rest of the executables linked isn't built with LTO.
>
> I believe this issue should show up in CI too, for the static-only configurations, where the runtime of the testsuite that builds thousands of test executables probably would skyrocket. (The current CI run seems to have failed earlier than that, so I couldn't check how it actually went this time.)
>
> So IMO, whether to enable LTO for the static library build or not should probably be a build time option, for toolchain vendors to decide upon.

I agree completely, for static libraries LTO is undesirable since it affects usage of those libraries and isn't transparent to users. This is one of the motivations for FatLTO support @paulkirth is working on (see the stack of changes starting at D146776 <https://reviews.llvm.org/D146776>) which allows building a static library that can be used for regular builds as well as LTO. Those changes should be landing soon. Even though only ELF is supported at the moment, there's no technical reason why we couldn't support FatLTO for COFF or Mach-O, we just need to implement the necessary LLD support.

I think we should have two flags to separately control the use of LTO for shared and static libraries. For shared libraries, we could default to `ON` (we should also have a CI configuration to test the `OFF` configuration). For static libraries the default should be `OFF` for now. Once FatLTO support lands, we can revisit this and consider changing the default for static libraries to `ON`. Alternatively, we could omit this change altogether and let the user control the use of LTO via `CMAKE_INTERPROCEDURAL_OPTIMIZATION`.

Regarding size reduction, in my experience ThinLTO doesn't typically help with size, in fact it often leads to size increase due to a more aggressive inlining and devirtualization. We had a much better success with regular LTO which I think is a better fit for runtime libraries due to their small size.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152967/new/

https://reviews.llvm.org/D152967