Skip to content

Commit

Permalink
Auto merge of rust-lang#124129 - lqd:enable-lld, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Enable `rust-lld` on nightly `x86_64-unknown-linux-gnu`

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](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Enabling.20.60rust-lld.60.20on.20nightly.20.60x86_64-unknown-linux-gnu.60/near/433709343).

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](https://hackmd.io/OAJxlxc6Te6YUot9ftYSKQ?view), 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 rust-lang#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](rust-lang/blog.rust-lang.org#1319).

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).
  • Loading branch information
bors committed May 16, 2024
2 parents 8c127df + 3a90967 commit 0349539
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 14 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,12 @@ impl LinkSelfContainedDefault {
_ => "crt-objects-fallback",
}
}

/// Creates a `LinkSelfContainedDefault` enabling the self-contained linker for target specs
/// (the equivalent of `-Clink-self-contained=+linker` on the CLI).
pub fn with_linker() -> LinkSelfContainedDefault {
LinkSelfContainedDefault::WithComponents(LinkSelfContainedComponents::LINKER)
}
}

bitflags::bitflags! {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ pub fn target() -> Target {
| SanitizerSet::THREAD;
base.supports_xray = true;

// When we're asked to use the `rust-lld` linker by default, set the appropriate lld-using
// linker flavor, and self-contained linker component.
if option_env!("CFG_USE_SELF_CONTAINED_LINKER").is_some() {
base.linker_flavor = LinkerFlavor::Gnu(Cc::Yes, Lld::Yes);
base.link_self_contained = crate::spec::LinkSelfContainedDefault::with_linker();
}

Target {
llvm_target: "x86_64-unknown-linux-gnu".into(),
metadata: crate::spec::TargetMetadata {
Expand Down
9 changes: 6 additions & 3 deletions config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -653,9 +653,12 @@
# when no explicit backend is specified.
#codegen-backends = ["llvm"]

# Indicates whether LLD will be compiled and made available in the sysroot for
# rustc to execute.
#lld = false
# Indicates whether LLD will be compiled and made available in the sysroot for rustc to execute, and
# whether to set it as rustc's default linker on `x86_64-unknown-linux-gnu`. This will also only be
# when *not* building an external LLVM (so only when using `download-ci-llvm` or building LLVM from
# the in-tree source): setting `llvm-config` in the `[target.x86_64-unknown-linux-gnu]` section will
# make this default to false.
#lld = false in all cases, except on `x86_64-unknown-linux-gnu` as described above, where it is true

# Indicates whether LLD will be used to link Rust crates during bootstrap on
# supported platforms.
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,11 @@ pub fn rustc_cargo_env(
cargo.env("CFG_DEFAULT_LINKER", s);
}

// Enable rustc's env var for `rust-lld` when requested.
if builder.config.lld_enabled {
cargo.env("CFG_USE_SELF_CONTAINED_LINKER", "1");
}

if builder.config.rust_verify_llvm_ir {
cargo.env("RUSTC_VERIFY_LLVM_IR", "1");
}
Expand Down
51 changes: 40 additions & 11 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1532,6 +1532,7 @@ impl Config {
let mut debuginfo_level_tests = None;
let mut optimize = None;
let mut omit_git_hash = None;
let mut lld_enabled = None;

if let Some(rust) = toml.rust {
let Rust {
Expand Down Expand Up @@ -1565,7 +1566,7 @@ impl Config {
dist_src,
save_toolstates,
codegen_backends,
lld,
lld: lld_enabled_toml,
llvm_tools,
llvm_bitcode_linker,
deny_warnings,
Expand Down Expand Up @@ -1620,6 +1621,7 @@ impl Config {
debuginfo_level_std = debuginfo_level_std_toml;
debuginfo_level_tools = debuginfo_level_tools_toml;
debuginfo_level_tests = debuginfo_level_tests_toml;
lld_enabled = lld_enabled_toml;

config.rust_split_debuginfo_for_build_triple = split_debuginfo
.as_deref()
Expand Down Expand Up @@ -1653,18 +1655,8 @@ impl Config {
config.incremental = true;
}
set(&mut config.lld_mode, lld_mode);
set(&mut config.lld_enabled, lld);
set(&mut config.llvm_bitcode_linker_enabled, llvm_bitcode_linker);

if matches!(config.lld_mode, LldMode::SelfContained)
&& !config.lld_enabled
&& flags.stage.unwrap_or(0) > 0
{
panic!(
"Trying to use self-contained lld as a linker, but LLD is not being added to the sysroot. Enable it with rust.lld = true."
);
}

config.llvm_tools_enabled = llvm_tools.unwrap_or(true);
config.rustc_parallel =
parallel_compiler.unwrap_or(config.channel == "dev" || config.channel == "nightly");
Expand Down Expand Up @@ -1954,6 +1946,43 @@ impl Config {
config.llvm_plugins = llvm_plugins.unwrap_or(false);
config.rust_optimize = optimize.unwrap_or(RustOptimize::Bool(true));

// We make `x86_64-unknown-linux-gnu` use the self-contained linker by default, so we will
// build our internal lld and use it as the default linker, by setting the `rust.lld` config
// to true by default:
// - on the `x86_64-unknown-linux-gnu` target
// - on the `dev` and `nightly` channels
// - when building our in-tree llvm (i.e. the target has not set an `llvm-config`), so that
// we're also able to build the corresponding lld
// - or when using an external llvm that's downloaded from CI, which also contains our prebuilt
// lld
// - otherwise, we'd be using an external llvm, and lld would not necessarily available and
// thus, disabled
// - similarly, lld will not be built nor used by default when explicitly asked not to, e.g.
// when the config sets `rust.lld = false`
if config.build.triple == "x86_64-unknown-linux-gnu"
&& config.hosts == [config.build]
&& (config.channel == "dev" || config.channel == "nightly")
{
let no_llvm_config = config
.target_config
.get(&config.build)
.is_some_and(|target_config| target_config.llvm_config.is_none());
let enable_lld = config.llvm_from_ci || no_llvm_config;
// Prefer the config setting in case an explicit opt-out is needed.
config.lld_enabled = lld_enabled.unwrap_or(enable_lld);
} else {
set(&mut config.lld_enabled, lld_enabled);
}

if matches!(config.lld_mode, LldMode::SelfContained)
&& !config.lld_enabled
&& flags.stage.unwrap_or(0) > 0
{
panic!(
"Trying to use self-contained lld as a linker, but LLD is not being added to the sysroot. Enable it with rust.lld = true."
);
}

let default = debug == Some(true);
config.rust_debug_assertions = debug_assertions.unwrap_or(default);
config.rust_debug_assertions_std =
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Info,
summary: r#"The compiler profile now defaults to rust.debuginfo-level = "line-tables-only""#,
},
ChangeInfo {
change_id: 124129,
severity: ChangeSeverity::Warning,
summary: "`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.",
},
];
5 changes: 5 additions & 0 deletions tests/run-make/rust-lld-by-default/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Test linking using `cc` with `rust-lld`, which is on by default on the x86_64-unknown-linux-gnu
// target.
// See https://github.com/rust-lang/compiler-team/issues/510 for more info

fn main() {}
43 changes: 43 additions & 0 deletions tests/run-make/rust-lld-by-default/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Ensure that rust-lld is used as the default linker on `x86_64-unknown-linux-gnu`, and that it can
// also be turned off with a CLI flag.

//@ needs-rust-lld
//@ only-x86_64-unknown-linux-gnu

extern crate run_make_support;

use run_make_support::regex::Regex;
use run_make_support::rustc;
use std::process::Output;

fn main() {
// A regular compilation should use rust-lld by default. We'll check that by asking the linker
// to display its version number with a link-arg.
let output = rustc()
.env("RUSTC_LOG", "rustc_codegen_ssa::back::link=info")
.link_arg("-Wl,-v")
.input("main.rs")
.run();
assert!(
find_lld_version_in_logs(output),
"the LLD version string should be present in the output logs"
);

// But it can still be disabled by turning the linker feature off.
let output = rustc()
.env("RUSTC_LOG", "rustc_codegen_ssa::back::link=info")
.link_arg("-Wl,-v")
.arg("-Zlinker-features=-lld")
.input("main.rs")
.run();
assert!(
!find_lld_version_in_logs(output),
"the LLD version string should not be present in the output logs"
);
}

fn find_lld_version_in_logs(output: Output) -> bool {
let lld_version_re = Regex::new(r"^LLD [0-9]+\.[0-9]+\.[0-9]+").unwrap();
let stderr = std::str::from_utf8(&output.stderr).unwrap();
stderr.lines().any(|line| lld_version_re.is_match(line))
}

0 comments on commit 0349539

Please sign in to comment.