Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu` #124129

lqd · 2024-04-18T18:34:39Z

We believe we have done virtually all the internal work and tests we could to prepare for using lld as the default linker (at least on Linux). We're IMHO at a point where we'd need to expand testing and coverage in order to make progress on this effort.

Therefore, for further testing and gathering real-world feedback, unexpected issues and use-cases, this PR enables rust-lld as the default linker:

on nightly only (and dev channel)
on x86_64-unknown-linux-gnu only
when not using an external LLVM (except download-ci-llvm), so that distros are not impacted

as described in more detail in this zulip thread.

In case any issues happen to users, as e.g. lld is not bug-for-bug compatible with GNU ld, it's easy to disable with -Zlinker-features=-lld to revert to using the system's default linker.

I don't know who should review this kind of things, as it's somewhat of a crosscutting effort. Compiler contributor, compiler performance WG and infra member sounds perfect, so r? @Mark-Simulacrum.

The last crater run encountered a low number (44) of mainly avoidable issues, like small incompatibilities, user errors, and a difference between the two linkers about which default to use with --gc-sections. Here's the triage report, categorizing the issues, with some analyses and workarounds. I'd appreciate another set of eyes looking at these results.

The changes in this PR have been test-driven for CI changes, try builds with tests enabled, rustc-perf with bootstrapping, in PR #113382.

For infra, about the CI change: this PR forces rust.lld to false on vanilla LLVM builders, just to make sure we have coverage without rust-lld. Though to be clear, just using an external LLVM is already enough to keep rust.lld to false, in turn reverting everything to using the system's default linker.

cc @rust-lang/bootstrap for the bootstrap and config change
cc @petrochenkov for the small compiler change
cc @rust-lang/wg-compiler-performance

The blog post announcing the change, that we expect to merge around the same time as we merge this PR, is open on the blog repo.

Bootstrap change history: this PR changes the default of a config option on x86_64-unknown-linux-gnu. It's, however, not expected to cause issues, or require any changes to existing configurations. It's a big enough change that people should at least know about it, in case it causes unexpected problems. If that happens, set rust.lld = false in your config.toml (and open an issue).

lqd · 2024-04-18T18:53:43Z

@bors try @rust-timer queue

Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu` r? ghost Draft until final preparations are done (making a bootstrap change entry requires a PR number)

bors · 2024-04-18T18:54:54Z

⌛ Trying commit f63b7c5 with merge 5a8c051...

rustbot · 2024-04-18T19:11:29Z

Some changes occurred in run-make tests.

cc @jieyouxu

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies config.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

These commits modify compiler targets.
(See the Target Tier Policy.)

bors · 2024-04-18T20:27:11Z

☀️ Try build successful - checks-actions
Build commit: 5a8c051 (5a8c051e2c9b7cb9c5bbbf9bc9cf44f12477e577)

rust-timer · 2024-04-19T06:04:58Z

Finished benchmarking commit (5a8c051): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-30.9%	[-72.0%, -0.5%]	31
Improvements ✅ (secondary)	-26.4%	[-71.3%, -1.5%]	73
All ❌✅ (primary)	-30.9%	[-72.0%, -0.5%]	31

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	17.7%	[12.3%, 23.1%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	17.7%	[12.3%, 23.1%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	7.7%	[6.1%, 9.2%]	4
Improvements ✅ (primary)	-30.7%	[-63.0%, -1.0%]	27
Improvements ✅ (secondary)	-24.0%	[-61.3%, -2.0%]	72
All ❌✅ (primary)	-30.7%	[-63.0%, -1.0%]	27

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.1%	[0.6%, 4.3%]	28
Regressions ❌ (secondary)	2.0%	[0.4%, 5.5%]	74
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.2%]	4
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.1%]	2
All ❌✅ (primary)	1.8%	[-0.3%, 4.3%]	32

Bootstrap: 676.731s -> 669.514s (-1.07%)
Artifact size: 316.11 MiB -> 315.37 MiB (-0.23%)

albertlarsan68 · 2024-04-19T07:11:25Z

For the bootstrap change:
I would prefer another config, instead of overloading the already existing one.
Something like rust.allow_lld_default, which can only be true when rust.lld is true, seems better IMO.
Also, how does this interact with rust.use_lld?

Kobzol · 2024-04-19T07:16:53Z

The bootstrap change is "temporary", in the sense that once this change lands on beta, it will be enabled by default and bootstrap won't need to configure it, if I understand it correctly. So the bootstrap change doesn't really modify the meaning of rust.lld, it just enables the new default if LLD is built.

The annoying thing here is that we have many axes on what to configure - should LLD be built? (this should arguably be in the llvm section, not rust), should LLD be added to sysroot? should LLD be used when linking the toolchain? should it be used when linking Rust programs? should the self-contained or external LLD be used? And some of these are only relevant for rustc developers. We already have two LLD options that are quite inconsistent in their behavior, I'm not sure if adding a third option makes that situation better 😅 If we go this route, I would rather completely revamp the way we configure LLD (and linker usage in general), so that it makes more sense in bootstrap.

src/bootstrap/src/core/build_steps/compile.rs

src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile

Mark-Simulacrum · 2024-04-20T14:02:40Z

The annoying thing here is that we have many axes on what to configure - should LLD be built? (this should arguably be in the llvm section, not rust), should LLD be added to sysroot? should LLD be used when linking the toolchain? should it be used when linking Rust programs? should the self-contained or external LLD be used? And some of these are only relevant for rustc developers. We already have two LLD options that are quite inconsistent in their behavior, I'm not sure if adding a third option makes that situation better 😅 If we go this route, I would rather completely revamp the way we configure LLD (and linker usage in general), so that it makes more sense in bootstrap.

I think this makes sense to me as the right direction to go in, rather than trying to add another option in this PR.

Overall this PR looks good to me at a high level, though I can't review the correctness of the compiler change (@petrochenkov should probably sign off on that), it seems OK to me. I think we should proceed with it. One thing that surprises me though is that this had a positive effect on the binary sizes for what we ship. Were we not already using LLD during that build? Perhaps we were picking up an old LLD from somewhere?

I agree that we should have a blog post notifying users of this change (similar to the parallel frontend one). Once that's ready and a few nits here are fixed, I'm happy to r+

lqd · 2024-04-20T17:24:12Z

Were we not already using LLD during that build? Perhaps we were picking up an old LLD from somewhere?

The artifact size reduction is also somewhat surprising to me. I believe that we are indeed already using LLD for that build, via rust.use-lld and that this should find and use the system's LLD 18.1.0 rather than an old one.

lqd · 2024-04-22T10:00:19Z

I wonder if these size reductions may be some kind of noise, they were not present in the previous runs of #113382 (other than the regular small fluctuations; the only difference with this PR is more builders are enabled there) including this one from yesterday.

The artifacts in the dist build are not linked with the self-contained linker (rust-lld/lld 18.1.4), but via --use-lld (CI's lld 18.1.0), and this can be verified in the .comment section. That is, this PR only makes x86_64-unknown-linux-gnu use rust-lld by default. Dist on the contrary specifies clang as the linker. When that happens, rustc needs to infer a linker flavor: clang is inferred to a C/C++ compiler not using lld, and the self-contained linker is not enabled. That's what already happens today.

Let's maybe see if another run has different results.

edit: done below, no significant change

lqd · 2024-05-17T01:28:23Z

@bors retry The runner has received a shutdown signal

bors · 2024-05-17T02:12:14Z

⌛ Testing commit 3a90967 with merge 8af67ba...

bors · 2024-05-17T04:17:10Z

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 8af67ba to master...

rust-timer · 2024-05-17T05:31:26Z

Finished benchmarking commit (8af67ba): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-30.8%	[-71.8%, -0.5%]	31
Improvements ✅ (secondary)	-25.6%	[-70.9%, -0.5%]	75
All ❌✅ (primary)	-30.8%	[-71.8%, -0.5%]	31

Max RSS (memory usage)

Results (primary 16.6%, secondary 3.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	16.6%	[9.9%, 23.4%]	2
Regressions ❌ (secondary)	3.8%	[2.4%, 5.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	16.6%	[9.9%, 23.4%]	2

Cycles

Results (primary -30.6%, secondary -24.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-30.6%	[-62.4%, -1.0%]	27
Improvements ✅ (secondary)	-24.2%	[-61.6%, -2.3%]	71
All ❌✅ (primary)	-30.6%	[-62.4%, -1.0%]	27

Binary size

Results (primary 2.0%, secondary 1.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.2%	[0.6%, 4.2%]	28
Regressions ❌ (secondary)	1.9%	[0.5%, 5.5%]	74
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	3
Improvements ✅ (secondary)	-0.6%	[-1.0%, -0.1%]	2
All ❌✅ (primary)	2.0%	[-0.3%, 4.2%]	31

Bootstrap: 678.599s -> 669.726s (-1.31%)
Artifact size: 316.13 MiB -> 316.16 MiB (0.01%)

Link is currently failing with rust-lld (rust-lang/rust#124129).

With rust-lang/rust#124129, x64 linux switches to use LLD by default, but this causes dependency issues on NixOS. Disable LLD for now. This is tracked by rust-lang/rust#125321.

Refs: dtolnay/linkme#88, rust-lang/rust#124129

Here are all the changes. I went through them one-by-one and confirmed that they should not be affecting us. In paritcular, we explicitly set rust.lld = false (because we want to use the lld that ships with clang), so the change in default does not affect us. There have been changes to x.py since you last updated: [INFO] New option `build.lldb` that will override the default lldb binary path used in debuginfo tests - PR Link rust-lang/rust#124501 [INFO] The compiler profile now defaults to rust.debuginfo-level = "line-tables-only" - PR Link rust-lang/rust#123337 [WARNING] `rust.lld` has a new default value of `true` on `x86_64-unknown-linux-gnu`. Starting at stage1, `rust-lld` will thus be this target's default linker. No config changes should be necessary. - PR Link rust-lang/rust#124129 [WARNING] Removed `dist.missing-tools` configuration as it was deprecated long time ago. - PR Link rust-lang/rust#125535 [WARNING] `llvm.lld` is enabled by default for the dist profile. If set to false, `lld` will not be included in the dist build. - PR Link rust-lang/rust#126701 [WARNING] `debug-logging` option has been removed from the default `tools` profile. - PR Link rust-lang/rust#127913 [INFO] the `wasm-component-ld` tool is now built as part of `build.extended` and can be a member of `build.tools` - PR Link rust-lang/rust#127866 [INFO] Removed android-ndk r25b support in favor of android-ndk r26d. - PR Link rust-lang/rust#120593 [WARNING] For tarball sources, default value for `rust.channel` will be taken from `src/ci/channel` file. - PR Link rust-lang/rust#125181 [INFO] New option `llvm.libzstd` to control whether llvm is built with zstd support. - PR Link rust-lang/rust#125642 [WARNING] ./x test --rustc-args was renamed to --compiletest-rustc-args as it only applies there. ./x miri --rustc-args was also removed. - PR Link rust-lang/rust#128841 [INFO] The `build.profiler` option now tries to use source code from `download-ci-llvm` if possible, instead of checking out the `src/llvm-project` submodule. - PR Link rust-lang/rust#129295 Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/recipes/+/1120078 Original-Revision: 27df37a30e50b14b9ffefc872b6997790f03d4ea GitOrigin-RevId: 341e222f002e36886b9960645b21faeaed633f90 Change-Id: Id1eb54a677a6f538bf7666d65b85d5fdba17ea42

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 18, 2024

rustbot assigned Mark-Simulacrum Apr 18, 2024

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2024

lqd marked this pull request as ready for review April 18, 2024 19:11

lqd added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 19, 2024

Mark-Simulacrum reviewed Apr 20, 2024

View reviewed changes

src/bootstrap/src/core/build_steps/compile.rs Outdated Show resolved Hide resolved

src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile Outdated Show resolved Hide resolved

lqd force-pushed the enable-lld branch 2 times, most recently from 1978bc2 to ded5bfb Compare April 20, 2024 16:52

This comment was marked as resolved.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 22, 2024

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 17, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label May 17, 2024

bors merged commit 8af67ba into rust-lang:master May 17, 2024
7 checks passed

rustbot added this to the 1.80.0 milestone May 17, 2024

lqd deleted the enable-lld branch May 17, 2024 04:30

This was referenced May 17, 2024

Use lld by default on x64 Ubuntu 20.04 LTS #71515

Open

Linking with LLD #39915

Open

dtolnay mentioned this pull request May 18, 2024

Disable rust-lld in CI dtolnay/linkme#88

Merged

Emilgardis mentioned this pull request May 18, 2024

Nightly builds fail on many platforms now that nightly sets -fuse-ld=lld cross-rs/cross#1496

Closed

11 tasks

This was referenced May 18, 2024

Nightly CI failed: collect2: fatal error: cannot find 'ld' RalfJung/cargo-careful#31

Closed

rustc can't find the linker when --sysroot is set to a local build of the sysroot #125246

Closed

plusvic added a commit to VirusTotal/yara-x that referenced this pull request May 18, 2024

chore: disable rust-lld in nightly build.

7bc22bd

Link is currently failing with rust-lld (rust-lang/rust#124129).

pnkfelix mentioned this pull request May 20, 2024

lld doesn't work on NixOS #125321

Closed

nbdd0121 added a commit to Rust-for-Linux/klint that referenced this pull request May 20, 2024

Disable LLD

bd3ef2e

With rust-lang/rust#124129, x64 linux switches to use LLD by default, but this causes dependency issues on NixOS. Disable LLD for now. This is tracked by rust-lang/rust#125321.

torokati44 mentioned this pull request May 20, 2024

ci: Disable rust-lld on Rust nightly ruffle-rs/ruffle#16390

Merged

kezhuw added a commit to kezhuw/zookeeper-client-rust that referenced this pull request May 21, 2024

Disable rust-lld on nightly to fix ci failure

b2d6e58

Refs: dtolnay/linkme#88, rust-lang/rust#124129

kezhuw added a commit to kezhuw/spawns that referenced this pull request May 23, 2024

Disable rust-lld on nightly to fix coverage ci failure

4bae981

Refs: dtolnay/linkme#88, rust-lang/rust#124129

kezhuw mentioned this pull request May 23, 2024

Disable rust-lld on nightly to fix coverage ci failure kezhuw/spawns#5

Merged

kezhuw added a commit to kezhuw/spawns that referenced this pull request May 23, 2024

Disable rust-lld on nightly to fix coverage ci failure (#5)

c8b4683

Refs: dtolnay/linkme#88, rust-lang/rust#124129

This was referenced May 28, 2024

Support zstd-compressed ELF sections. rust-lang/backtrace-rs#626

Merged

Enable zstd for debug compression. #125642

Merged

nbdd0121 mentioned this pull request Aug 29, 2024

Rust build support doesn't use NIX_LDFLAGS in link flags NixOS/nixpkgs#326944

Open

DianQK mentioned this pull request Oct 5, 2024

rust 1.81.0 buildable with LLVM 17.x? #131291

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu` #124129

Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu` #124129

lqd commented Apr 18, 2024 •

edited

Loading

lqd commented Apr 18, 2024

This comment has been minimized.

bors commented Apr 18, 2024

rustbot commented Apr 18, 2024

bors commented Apr 18, 2024

This comment has been minimized.

rust-timer commented Apr 19, 2024

albertlarsan68 commented Apr 19, 2024

Kobzol commented Apr 19, 2024 •

edited

Loading

Mark-Simulacrum commented Apr 20, 2024

lqd commented Apr 20, 2024

lqd commented Apr 22, 2024 •

edited

Loading

This comment was marked as resolved.

lqd commented May 17, 2024

bors commented May 17, 2024

bors commented May 17, 2024

rust-timer commented May 17, 2024

Enable rust-lld on nightly x86_64-unknown-linux-gnu #124129

Enable rust-lld on nightly x86_64-unknown-linux-gnu #124129

Conversation

lqd commented Apr 18, 2024 • edited Loading

lqd commented Apr 18, 2024

This comment has been minimized.

bors commented Apr 18, 2024

rustbot commented Apr 18, 2024

bors commented Apr 18, 2024

This comment has been minimized.

rust-timer commented Apr 19, 2024

Overall result: ✅ improvements - no action needed

albertlarsan68 commented Apr 19, 2024

Kobzol commented Apr 19, 2024 • edited Loading

Mark-Simulacrum commented Apr 20, 2024

lqd commented Apr 20, 2024

lqd commented Apr 22, 2024 • edited Loading

This comment was marked as resolved.

lqd commented May 17, 2024

bors commented May 17, 2024

bors commented May 17, 2024

rust-timer commented May 17, 2024

Overall result: ✅ improvements - no action needed

Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu` #124129

Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu` #124129

lqd commented Apr 18, 2024 •

edited

Loading

Kobzol commented Apr 19, 2024 •

edited

Loading

lqd commented Apr 22, 2024 •

edited

Loading