Skip to content

Conversation

Hoverbear
Copy link
Contributor

On Ferrocene, we set rust.lld = true and have recently added a check to validate that the -fuse-ld=lld argument is passed in ferrocene/ferrocene#1733. When running our tests I noticed the aarch64-unknown-linux-gnu target was failing.

Looking into it, I noticed that the aarch64 target lacks what the x86_64 target has in this regard:

// 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();
}

I adjusted the target to the included the related changes to Ferrocene and ran our full suite as well as validated the check from ferrocene/ferrocene#1733 also passes.

Testing

You can validate this by taking an aarch64-unknown-linux-gnu host with a config like:

profile = "compiler"

[rust]
lld = true

Validate that rustc doesn't pass anything about lld:

./x.py build --stage 1

Then, with a your toolchain link, as specified in the rustc-dev-guide, run the following:

echo "fn main() {}" > main.rs
rustc +stage1 main.rs --print link-args | grep lld

Notice no output is provided. Next, using this branch:

./x.py build --stage 1
rustc +stage1 main.rs --print link-args | grep lld

Validate how the output includes -fuse-ld=lld.

@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 2025

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Hoverbear Hoverbear changed the title Allow LLD to be enabled on aarch64-unknown-linux-gnu Allow lld to be enabled on aarch64-unknown-linux-gnu Sep 15, 2025
@workingjubilee
Copy link
Member

cc @lqd If you have a spare cycle, do you have any insight to what the proper handling for this would be?

@Kobzol
Copy link
Member

Kobzol commented Sep 15, 2025

The CFG_USE_SELF_CONTAINED_LINKER mechanism is a bit hacky, but if we do use it for multiple targets, I wonder if it should be set in the default target options, rather than reading this environment variable in each target that wants to default to LLD individually.

@workingjubilee
Copy link
Member

Targets already have strong control over what linker will get used and how, assuming no arguments are passed, so that seems good to me.

@mati865
Copy link
Member

mati865 commented Sep 16, 2025

The CFG_USE_SELF_CONTAINED_LINKER mechanism is a bit hacky, but if we do use it for multiple targets, I wonder if it should be set in the default target options, rather than reading this environment variable in each target that wants to default to LLD individually.

Given that this environment variable is compile-time checked this would be sensible.

Regardless of the direction this will require an update to rust.lld comment in bootstrap.example.toml.

@lqd
Copy link
Member

lqd commented Sep 16, 2025

CFG_USE_SELF_CONTAINED_LINKER only exists to ensure bootstrap controls whether changing the default linker is enabled or not, so that we don't forcefully and silently make distros depend on a tool they don't distribute. I initially implemented this as a feature flag in rustc_target, and t-bootstrap asked for the env var.

The "quality" of lld support will vary per-target, so centralization is also a trade-off versus the signal the opt-in suggests in the target spec. I'm not sure the ergonomics are particularly useful to optimize, but why not in the future. The internals of relying on linker flavors should be moved to using linker features instead: this wasn't a blocker for stabilization but may be so if more targets want to do such a thing.

On Ferrocene, we set rust.lld = true and have recently added a check to validate that the -fuse-ld=lld

rust.lld = true is not a signal to use lld by default. But if this was a wish of multiple target maintainers, then we should take a better look from bootstrap to targets and tests and CI to have a good strat on how to support this for everyone (Mateusz's request for some documentation at the very least). The current setup is made for a target with different constraints after all.

do you have any insight to what the proper handling for this would be?

The maintainers for aarch64-unknown-linux-gnu are not documented, but they at least would be the ones to first weigh in on whether this is a desirable change. Apart from the reliance on the "modern linker flavors" that didn't pan out and should be migrated to linker-features, if this change would be OK for this target maintainers, how the PR does it is not the end of the world (it's how the x64 linux target does it for now). I'm not seeing any tests to ensure this works, can be disabled, etc and to catch regressions on our side, or lld's side, or both.

In addition to maybe waiting a bit to see how the first use of this turns out in practice when the next stable is released on Thursday, and what if anything needs to be changed in response.

@Hoverbear Hoverbear force-pushed the hoverbear/allow-aarch64-to-use-lld branch from d5a84ce to 49dd168 Compare September 16, 2025 13:40
@rustbot
Copy link
Collaborator

rustbot commented Sep 16, 2025

This PR modifies bootstrap.example.toml.

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

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 16, 2025
@Hoverbear Hoverbear force-pushed the hoverbear/allow-aarch64-to-use-lld branch from 49dd168 to 229be70 Compare September 16, 2025 13:42
@Hoverbear
Copy link
Contributor Author

I made an update to the bootstrap.example.toml and added it to the change tracker.

@Hoverbear Hoverbear force-pushed the hoverbear/allow-aarch64-to-use-lld branch from 229be70 to 9cbfaae Compare September 16, 2025 14:19
@Hoverbear Hoverbear force-pushed the hoverbear/allow-aarch64-to-use-lld branch from 9cbfaae to 358f344 Compare September 16, 2025 15:15
@jyn514
Copy link
Member

jyn514 commented Sep 16, 2025

rust.lld = true is not a signal to use lld by default. But if this was a wish of multiple target maintainers, then we should take a better look from bootstrap to targets and tests and CI to have a good strat on how to support this for everyone (Mateusz's request for some documentation at the very least). The current setup is made for a target with different constraints after all.

I think we should have an option to use lld by default, but it should be different from the option to ship lld with the compiler. In particular, I suggest that we split the rust.lld config option into build.build-lld = bool and rust.default-linker = "self-contained-lld" | "platform" or something like that, and error if you set build-lld = false and default-linker = self-contained. Then we don't have to do weird things with hard-coded target specs, we simply have bootstrap's default for default-linker be the platform linker everywhere except x86_64 linux, where it will be LLD. Ana and Ferrocene will set default-linker = self-contained on aarch64, rust-lang/rust will continue to default to default-linker = "platform" so we aren't imposing on the platform maintainers.

(Note that all of this is unrelated to rust.use-lld, which only controls whether lld is used while bootstrapping the compiler, not anything about what the distributed compiler does at runtime. I would like to rename that to rust.bootstrap-use-lld or something like that in the future.)

@Kobzol
Copy link
Member

Kobzol commented Sep 16, 2025

We already have both rust.default_linker and <target>.default_linker (

if let Some(s) = target_config.and_then(|c| c.default_linker.as_ref()) {
), it's just that on Linux this does not actually modify what linker gets used 😅 Not sure if we should just interpret this field differently on Linux, or use a separate config field (the latter is probably a better idea).

I agree that once more Linux targets than just x64 start switching to a different default linker, we should make this more first-class in bootstrap, rather than implicitly changing the behavior of rust.lld, we have kinda agreed in the past to do this in the future, once the current hack becomes too annoying.

I created an issue where we can discuss this further: #146640

@bors
Copy link
Collaborator

bors commented Sep 18, 2025

☔ The latest upstream changes (presumably #146685) made this pull request unmergeable. Please resolve the merge conflicts.

@bjorn3 bjorn3 mentioned this pull request Sep 18, 2025
@petrochenkov
Copy link
Contributor

r? @Kobzol you can better decide when to land this relatively to #146640.
This is a bootstrap change really, not compiler.

@rustbot rustbot assigned Kobzol and unassigned petrochenkov Sep 26, 2025
@Kobzol
Copy link
Member

Kobzol commented Sep 26, 2025

@Hoverbear if you don't mind, I'd like to finish #146640 during next week, and prepare a way for targets to opt into LLD. Then this PR should become even smaller.

@Hoverbear
Copy link
Contributor Author

@Kobzol sounds lovely. :) I'll adapt this once it lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants