Only specify --target by default for -Zgcc-ld=lld on wasm

[lqd]

On macOS, it's not yet clear which cases of clang/OS/target/SDK version impact how to find ld/lld/rust-lld. The --target argument is not needed on our current targets with a vanilla config to do so, but may be in some cases ?

That is, things look to be different and more subtle than suggested in #97402 (comment).

Specifying this argument unconditionally currently breaks -Zgcc-ld=lld on the 10.7+ targets on x64 macOS. Vanilla configs on x64 and aarch64 don't seem to need it to be able to find rust-lld.

This fixes #101653 on macOS (I've tried on x64 and aarch64: vanilla installs with the most recent command line tools available for each of the OS versions I have access to, run-make/issue-71519 passes).

I didn't expect the previous PRs to fail because of the existing tests, but CI does not actually run those tests yet, which explains the regressions. I was hoping to fix those in this PR but it's more involved (building lld is required for the tests to run, llvm/lld is not built on the test builders but on the dist builders, the dist builders don't run tests). This PR is just to unblock current users on macOS who have reported issues, but a regression could happen in the future by mistake until then.

r? @petrochenkov

[tschuett]

Maybe you will need the target on macOS for cross-compiling?

[lqd]

It wasn’t used before, so I’m not sure. And you can still pass it via -Clink-arg I presume ?

[Wyvern]

Looks like this problem still not fixed.

Another question. Why use -fuse-ld=lld generate clang: error: invalid linker name in argument '-fuse-ld=lld for now?

[lqd]

@Wyvern What do you mean not fixed, did you build this PR locally to test ? If so, what error did you get ?

[petrochenkov]

10.7 comes from this function in compiler\rustc_target\src\spec\apple_base.rs:

fn macos_default_deployment_target(arch: &str) -> (u32, u32) {
    if arch == "arm64" { (11, 0) } else { (10, 7) }
}

, this is the default version that is used if no MACOSX_DEPLOYMENT_TARGET is provided by the user.

So setting MACOSX_DEPLOYMENT_TARGET that matches your environment should work as a quick workaround for #101653.

UPD: Also this is why the reproduction for x64 and aarch64 is different.

[petrochenkov]

@Wyvern

Another question. Why use -fuse-ld=lld generate clang: error: invalid linker name in argument '-fuse-ld=lld for now?

This should only happen if your clang is old (or somehow modified) and doesn't support LLD. In that case it's reasonable that rustc using such clang doesn't support LLD either.

[petrochenkov]

So if no MACOSX_DEPLOYMENT_TARGET is specified, then rustc will always pass target x86_64-apple-macosx10.7.0 to LLVM and apparently that's fine for all the range of supported versions.
However, it's apparently not fine to pass it to clang with a different default macosx version.

[petrochenkov]

Maybe you will need the target on macOS for cross-compiling?

That's exactly why the --target option is passed, it's not necessary if we are not cross-compiling, clang should find the appropriate lld flavor for the host in that case.

There are several possible cases here:

• Not a cross compilation: --target is not necessary, the correct target is either hardcoded or used by default.
• Cross compilation:
  • Using a separate toolchain for the specific target (gcc is typically used like this), then --target is not necessary because it's hardcoded.
  • Using a toolchain that can switch targets dynamically (e.g. usual clang installed on the host), then --target is necessary for that clang to decide what triple to target.

[petrochenkov]

I think the initial fix should be

                    if we_are_cross_compiling() {
                        cmd.arg(format!("--target={}", sess.target.llvm_target));
                    }

, so the main question is how to implement the we_are_cross_compiling() part :D

sess.host == sess.target should work in most cases, but I'm not sure it will work for Apple targets specifically due to possible SDK version mismatches between the host and target.

[petrochenkov]

so the main question is how to implement the we_are_cross_compiling()

sess.target.lld_flavor != sess.host.lld_flavor!

[tschuett]

This could miss the case where I am compiling from OSX to WatchOS. The lld flavour is the same, but I need the target parameter.

we_are_cross_compiling() { there was a target parameter in the rustc invocation. }

[petrochenkov]

@tschuett
Sorry, I don't understand.
What do you do in this case when you are compiling without -Zgcc-lld? Exactly the same thing should work with -Zgcc-lld too.

[tschuett]

> RUSTFLAGS='-Z gcc-ld=lld' cargo +nightly build --target x86_64-apple-darwin
> RUSTFLAGS='-Z gcc-ld=lld' cargo +nightly build --target x86_64-apple-ios

I am on x86 OSX. The lld flavour is the same, but I would need the target parameter, i.e., the target parameter must be passed to the linker.

[petrochenkov]

@tschuett
I guess the current assumption is that you pass it manually (with or without -Z gcc-ld=lld).
Maybe it can be changed for Apple targets specifically, because the compiler there is guaranteed (?) to be clang and accept the --target option, but it's again orthogonal to -Z gcc-ld=lld.

[Wyvern]

Is there a plan to merge this PR into nightly build?

[lqd]

@Wyvern Yes, we're working on it. Until then, you can also use the MACOSX_DEPLOYMENT_TARGET env var and it'll work. Or a local lld via -Clink-arg=-fuse-ld=lld I guess.

[Wyvern]

MACOSX_DEPLOYMENT_TARGET=12.0 works, Thx.

[lqd]

@petrochenkov I've rebased over #101988, and added the simili cross-compiling test for macOS, for the possibly different linker flavors between host and targets. Is this now what you expected ?

I've tested this PR on x64 darwin (without cross compilation shenanigans) and -Zgcc-ld will now work. I've also opened a draft PR, waiting on this one, to fix CI so that needs-rust-lld tests will be executed to prevent things from regressing again.

@rustbot ready

[petrochenkov]

Sorry for the delay, I'm currently taking some break from Rust.
@rustbot author

[lqd]

Sorry for the delay, I'm currently taking some break from Rust.

@petrochenkov would you rather we re-roll for another reviewer ?

[petrochenkov]

@lqd
No, I reassigned most of my PRs so the remaining ones, including this, should be fine.

[lqd]

Removed the superfluous bool.

@rustbot ready

[petrochenkov]

@bors r+

[bors]

📌 Commit 7770b06 has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 7770b06 with merge 312c9a3...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 312c9a3 to master...

[rust-timer]

Finished benchmarking commit (312c9a3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

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)	2.9%	[2.9%, 2.9%]	1
Regressions ❌ (secondary)	2.8%	[2.8%, 2.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.9%	[2.9%, 2.9%]	1

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)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0