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

[lqd]

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]

@bors try @rust-timer queue

[bors]

⌛ Trying commit f63b7c5 with merge 5a8c051...

[rustbot]

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]

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

[rust-timer]

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]

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]

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.

[Mark-Simulacrum]

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]

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]

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

[rust-log-analyzer]

The job aarch64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[RUSTC-TIMING] rustc_symbol_mangling test:false 14.766
   Compiling rustc_transmute v0.0.0 (/checkout/compiler/rustc_transmute)
[RUSTC-TIMING] rustc_pattern_analysis test:false 22.331
   Compiling rustc_trait_selection v0.0.0 (/checkout/compiler/rustc_trait_selection)
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.

Session terminated, killing shell...::group::Clock drift check
  local time:  ...killed.
##[error]The operation was canceled.

[bors]

💔 Test failed - checks-actions

[lqd]

@bors retry The runner has received a shutdown signal

[bors]

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

[bors]

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

[rust-timer]

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%)