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

boulder: Numerous toolchain changes #381

Merged
merged 9 commits into from
Jan 17, 2025
Merged

boulder: Numerous toolchain changes #381

merged 9 commits into from
Jan 17, 2025

Conversation

ReillyBrogan
Copy link
Contributor

Note: I bundled these together because most of them are pretty small and I worked on all of them at the same time, but I can split them out if needed (they're separate commits per-change).

Changes:

  • Add build-id flags and enable it by default
  • Default to compressing debug symbols with zstd. This reduces on-disk space of debug symbols significantly at the cost of higher package sizes.
  • Define -flto=%(jobs) for GCC as "thin" LTO since it behaves roughly similar to the LLVM equivalent. Add -flto=%(jobs) -flto-partition=one as a new "full" variant of LTO for GCC.
  • Add -ffat-lto-objects flags and enable them by default. These are needed if building static archives with LTO that are used by the alternate toolchain (IE if the static archives are built with GCC then fat-lto-objects is needed in order to link them with LLVM). Interestingly this also reduced binary sizes with LTO over not having the flag, likely because more information was preserved to the linker.
  • Add some error flags recommended by Gentoo which can provide an early warning that LTO is likely to result in runtime issues. Enable by default.
  • Enable thin LTO by default. Several other distros have switched to this as the default by now and it's better to deal with the fallout of it now rather than when we have a few thousand extra packages.
  • Add direct support for mold including automatically adding it as a builddep and setting the appropriate error flags
  • Changed many dependencies added by macros to be binary(*) dependencies instead of named ones.

Testing:

  • Did many, many builds between LLVM/GCC/Mold with the new flags in order to ensure that they worked everywhere as expected.

@joebonrichie
Copy link
Contributor

LGTM

Copy link
Collaborator

@tarkah tarkah left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -48,6 +48,8 @@ pub struct Recipe {
pub tuning: Vec<KeyValue<Tuning>>,
#[serde(default, deserialize_with = "stringy_bool")]
pub emul32: bool,
#[serde(default, deserialize_with = "stringy_bool")]
pub mold: bool,
Copy link
Member

Choose a reason for hiding this comment

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

what happens when you want to support another experimental linker? Another root-level key? This part needs more consideration before changing here.

flags
.iter()
.filter_map(|flag| flag.get(tuning::CompilerFlag::Rust, toolchain)),
);

if recipe.parsed.mold {
Copy link
Member

Choose a reason for hiding this comment

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

surely we can use substitutions for the ld plugin and not export full format strings here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean? We only need to add the flag for mold, not substitute an existing one.

c : "-Werror=odr -Werror=strict-aliasing"
cxx : "-Werror=odr -Werror=strict-aliasing"

# Enable build-id (ON)
Copy link
Member

Choose a reason for hiding this comment

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

im not really happy with this one - some times we'll encounter packages that ignore the LDFLAGS partly or fully, and sometimes we do it on purpose (for example bootstrap of toolchain) - so we need the build-id guarantee baked into the toolchain itself. We also rely on build-id for generation of debuginfo assets so I don't actually see a reason to support disabling this? Lastly our current default is actually xxhash, why the undocumented switch to sha1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Way ahead of you on baking it into the toolchain:

The purpose of allowing --build-id=none to be set is moreso a just-in-case thing. Since we're setting it at the toolchain level having --build-id=sha1 in the flags is mostly so that packagers can see that it's there, and since it's baked in just removing the flag isn't going to disable it in the toolchain. We need an explicit --build-id=none to turn it off.

I don't really see it being necessary to disable, but some things like golang handle their own buildid generation and it's better to have the escape than not I think.

Lastly our current default is actually xxhash, why the undocumented switch to sha1?

We discussed this in Matrix already but I'm going to open up an issue to track these flag changes.

Changes:
- Move "fat" GCC LTO configuration to "thin" as it's a better match in terms of what it does
- Add a "fat" GCC LTO configuration that is much more similar to fat LTO with LLVM

Signed-off-by: Reilly Brogan <[email protected]>
These flags enable both traditional bitcode and LLVM/GCC intermediate object language to be built at the same time. This is necessary when using a static library built with LTO on LLVM/GCC with the other. Also, from some testing this also seems to improve the ability for LTO to minimize the binary size further.

This _does_ increase the size of the static archive files but not to a massive degree and it allows for more low-level LTO with packages that use the same toolchain and use static archives built in another package.

Tested with both LLVM and GCC, full LTO, thin LTO, and no LTO. Mold was also tested with both LLVM and GCC.

Signed-off-by: Reilly Brogan <[email protected]>
As recommended by the Gentoo wiki, this adds a number of compiler flags that indicate a high probability of runtime-related LTO issues. They are enabled by default.

Signed-off-by: Reilly Brogan <[email protected]>
Thin LTO for both GCC and LLVM has relatively little build-time loss compared to non-LTO and consistently sees improvements to binary size and runtime memory use. Enable it by default following in the footsteps of several other distributions that have done the same.

Signed-off-by: Reilly Brogan <[email protected]>
…efault

These reduce on-disk size of debug symbols significantly. They are fully supported by LLVM, GCC/Binutils, Mold, and elfutils.

Signed-off-by: Reilly Brogan <[email protected]>
Mold is often noticeably faster than LLD (and much faster than BFD) and is already used by numerous packages for that reason. Make it more ergonomic to use by adding a top-level `mold` key which adds it as a builddep and sets environmental flags appropriately to use it.

Tested with both the gnu and LLVM toolchains.

Signed-off-by: Reilly Brogan <[email protected]>
@ikeycode
Copy link
Member

Per matrix convo toolchain was just a busted-ass notion and we'll clean up in port to KDL

@ikeycode ikeycode merged commit ecbd710 into main Jan 17, 2025
2 checks passed
@ikeycode ikeycode deleted the tooling-flags-2025 branch January 17, 2025 02:32
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.

4 participants