Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Linker flavors: next steps #119906

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 4 additions & 25 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2284,6 +2284,10 @@ fn add_order_independent_options(
out_filename: &Path,
tmpdir: &Path,
) {
if flavor.uses_clang() && sess.target.linker_flavor != sess.host.linker_flavor {
cmd.arg(format!("--target={}", sess.target.llvm_target));
Comment on lines +2287 to +2288
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this condition sufficient for the general case of adding --target when cross-compiling? The linker flavor doesn't contain any information about the target triple (unless I'm missing something), so I don't see how this can reliably detect cross-compiling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I know --target only affects which linker is looked up (ld.lld, or ld64.lld, etc) if clang is used for linking only.
So if the required flavor is the same as host, then clang without --target will already use the right linker.

On the other hand, there are some issues with crt object versions on macOS if --target is specified (fixed by #101792), maybe target takes priority over some relevant environment variables.
So always passing --target leads to issues.

We'll likely need to tweak this logic based on reported issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For lld this may not matter, since lld is also a cross-linker.
My main concern is if gnu ld is used (linker flavor (Cc::Clang, Lld::No) for both host and target), then --target is required for clang to select e.g. aarch64-linux-gnu-ld instead of just ld.

}

// Take care of the flavors and CLI options requesting the `lld` linker.
add_lld_args(cmd, sess, flavor, self_contained_components);

Expand Down Expand Up @@ -3053,29 +3057,4 @@ fn add_lld_args(
// 2. Implement the "linker flavor" part of this feature by asking `cc` to use some kind of
// `lld` as the linker.
cmd.arg("-fuse-ld=lld");

if !flavor.is_gnu() {
// Tell clang to use a non-default LLD flavor.
// Gcc doesn't understand the target option, but we currently assume
// that gcc is not used for Apple and Wasm targets (#97402).
//
// Note that we don't want to do that by default on macOS: e.g. passing a
// 10.7 target to LLVM works, but not to recent versions of clang/macOS, as
// shown in issue #101653 and the discussion in PR #101792.
//
// It could be required in some cases of cross-compiling with
// LLD, but this is generally unspecified, and we don't know
// which specific versions of clang, macOS SDK, host and target OS
// combinations impact us here.
//
// So we do a simple first-approximation until we know more of what the
// Apple targets require (and which would be handled prior to hitting this
// LLD codepath anyway), but the expectation is that until then
// this should be manually passed if needed. We specify the target when
// targeting a different linker flavor on macOS, and that's also always
// the case when targeting WASM.
if sess.target.linker_flavor != sess.host.linker_flavor {
cmd.arg(format!("--target={}", sess.target.llvm_target));
}
}
}
35 changes: 30 additions & 5 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use rustc_fs_util::try_canonicalize;
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
use rustc_span::symbol::{kw, sym, Symbol};
use serde_json::Value;
use std::assert_matches::assert_matches;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::hash::{Hash, Hasher};
Expand Down Expand Up @@ -437,6 +438,17 @@ impl LinkerFlavor {
| LinkerFlavor::Ptx => false,
}
}

pub fn uses_clang(self) -> bool {
match self {
LinkerFlavor::Gnu(Cc::Clang, _)
| LinkerFlavor::Darwin(Cc::Clang, _)
| LinkerFlavor::WasmLld(Cc::Clang)
| LinkerFlavor::Unix(Cc::Clang)
| LinkerFlavor::EmCc => true,
_ => false,
}
}
}

macro_rules! linker_flavor_cli_impls {
Expand Down Expand Up @@ -2185,21 +2197,34 @@ fn add_link_args_iter(
match flavor {
LinkerFlavor::Gnu(cc, lld) => {
assert_eq!(lld, Lld::No);
assert_matches!(cc, Cc::No | Cc::Yes);
insert(LinkerFlavor::Gnu(cc, Lld::Yes));
if cc == Cc::Yes {
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::No));
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::Yes));
}
}
LinkerFlavor::Darwin(cc, lld) => {
assert_eq!(lld, Lld::No);
assert_matches!(cc, Cc::No | Cc::Yes);
insert(LinkerFlavor::Darwin(cc, Lld::Yes));
if cc == Cc::Yes {
insert(LinkerFlavor::Darwin(Cc::Clang, Lld::No));
insert(LinkerFlavor::Darwin(Cc::Clang, Lld::Yes));
}
}
LinkerFlavor::Msvc(lld) => {
assert_eq!(lld, Lld::No);
insert(LinkerFlavor::Msvc(Lld::Yes));
}
LinkerFlavor::WasmLld(..)
| LinkerFlavor::Unix(..)
| LinkerFlavor::EmCc
| LinkerFlavor::Bpf
| LinkerFlavor::Ptx => {}
LinkerFlavor::WasmLld(cc) | LinkerFlavor::Unix(cc) => {
assert_matches!(cc, Cc::No | Cc::Yes);
if cc == Cc::Yes {
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::No));
insert(LinkerFlavor::Gnu(Cc::Clang, Lld::Yes));
Comment on lines +2247 to +2248
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly still WIP: these linker args should have a WasmLld or Unix principal flavor here, rather than Gnu?

}
}
LinkerFlavor::EmCc | LinkerFlavor::Bpf | LinkerFlavor::Ptx => {}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,7 @@ pub fn target() -> Target {
"--no-entry",
],
);
options.add_pre_link_args(
LinkerFlavor::WasmLld(Cc::Yes),
&[
// Make sure clang uses LLD as its linker and is configured appropriately
// otherwise
"--target=wasm32-unknown-unknown",
"-Wl,--no-entry",
],
);
options.add_pre_link_args(LinkerFlavor::WasmLld(Cc::Yes), &["-Wl,--no-entry"]);

Target {
llvm_target: "wasm32-unknown-unknown".into(),
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_target/src/spec/targets/wasm32_wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,12 @@

use crate::spec::crt_objects;
use crate::spec::LinkSelfContainedDefault;
use crate::spec::{base, Cc, LinkerFlavor, Target};
use crate::spec::{base, Target};

pub fn target() -> Target {
let mut options = base::wasm::options();

options.os = "wasi".into();
options.add_pre_link_args(LinkerFlavor::WasmLld(Cc::Yes), &["--target=wasm32-wasi"]);

options.pre_link_objects_self_contained = crt_objects::pre_wasi_self_contained();
options.post_link_objects_self_contained = crt_objects::post_wasi_self_contained();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,7 @@ pub fn target() -> Target {
);
options.add_pre_link_args(
LinkerFlavor::WasmLld(Cc::Yes),
&[
"--target=wasm32-wasi-threads",
"-Wl,--import-memory",
"-Wl,--export-memory,",
"-Wl,--shared-memory",
],
&["-Wl,--import-memory", "-Wl,--export-memory,", "-Wl,--shared-memory"],
);

options.pre_link_objects_self_contained = crt_objects::pre_wasi_self_contained();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,7 @@ pub fn target() -> Target {
"-mwasm64",
],
);
options.add_pre_link_args(
LinkerFlavor::WasmLld(Cc::Yes),
&[
// Make sure clang uses LLD as its linker and is configured appropriately
// otherwise
"--target=wasm64-unknown-unknown",
"-Wl,--no-entry",
],
);
options.add_pre_link_args(LinkerFlavor::WasmLld(Cc::Yes), &["-Wl,--no-entry"]);

// Any engine that implements wasm64 will surely implement the rest of these
// features since they were all merged into the official spec by the time
Expand Down