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.

Sign up for GitHub

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

Use the same mir-opt bless targets on all platforms #120060

Merged
merged 4 commits into from
Feb 7, 2024
Merged

Use the same mir-opt bless targets on all platforms #120060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4451bf4
Use a better set of targets for blessing mir-opt tests
saethlin Jan 7, 2024
2116ed7
Tweak a few mir-opt tests instead of using -Clink-dead-code
saethlin Jan 31, 2024
476f91c
Don't call std_cargo to build mir-only sysroots
saethlin Feb 6, 2024
3c7a8b9
Fixup async_closure_shims mir-opt test
saethlin Feb 7, 2024
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
34 changes: 24 additions & 10 deletions src/bootstrap/src/core/build_steps/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,20 @@ impl Std {
is_for_mir_opt_tests: false,
}
}

fn copy_extra_objects(
&self,
builder: &Builder<'_>,
compiler: &Compiler,
target: TargetSelection,
) -> Vec<(PathBuf, DependencyType)> {
let mut deps = Vec::new();
if !self.is_for_mir_opt_tests {
deps.extend(copy_third_party_objects(builder, &compiler, target));
deps.extend(copy_self_contained_objects(builder, &compiler, target));
}
deps
}
}

impl Step for Std {
Expand Down Expand Up @@ -159,8 +173,7 @@ impl Step for Std {
{
builder.info("WARNING: Using a potentially old libstd. This may not behave well.");

copy_third_party_objects(builder, &compiler, target);
copy_self_contained_objects(builder, &compiler, target);
self.copy_extra_objects(builder, &compiler, target);

builder.ensure(StdLink::from_std(self, compiler));
return;
Expand Down Expand Up @@ -193,15 +206,13 @@ impl Step for Std {

// Even if we're not building std this stage, the new sysroot must
// still contain the third party objects needed by various targets.
copy_third_party_objects(builder, &compiler, target);
copy_self_contained_objects(builder, &compiler, target);
self.copy_extra_objects(builder, &compiler, target);

builder.ensure(StdLink::from_std(self, compiler_to_use));
return;
}

target_deps.extend(copy_third_party_objects(builder, &compiler, target));
target_deps.extend(copy_self_contained_objects(builder, &compiler, target));
target_deps.extend(self.copy_extra_objects(builder, &compiler, target));

// The LLD wrappers and `rust-lld` are self-contained linking components that can be
// necessary to link the stdlib on some targets. We'll also need to copy these binaries to
Expand All @@ -222,10 +233,13 @@ impl Step for Std {
}
}

// We build a sysroot for mir-opt tests using the same trick that Miri does: A check build
// with -Zalways-encode-mir. This frees us from the need to have a target linker, and the
// fact that this is a check build integrates nicely with run_cargo.
let mut cargo = if self.is_for_mir_opt_tests {
let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "rustc");
cargo.arg("-p").arg("std").arg("--crate-type=lib");
std_cargo(builder, target, compiler.stage, &mut cargo);
let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "check");
cargo.rustflag("-Zalways-encode-mir");
cargo.arg("--manifest-path").arg(builder.src.join("library/sysroot/Cargo.toml"));
cargo
} else {
let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "build");
Expand Down Expand Up @@ -257,7 +271,7 @@ impl Step for Std {
vec![],
&libstd_stamp(builder, compiler, target),
target_deps,
false,
self.is_for_mir_opt_tests, // is_check
false,
);

Expand Down
116 changes: 37 additions & 79 deletions src/bootstrap/src/core/build_steps/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use crate::core::builder::{Builder, Compiler, Kind, RunConfig, ShouldRun, Step};
use crate::core::config::flags::get_completion;
use crate::core::config::flags::Subcommand;
use crate::core::config::TargetSelection;
use crate::utils;
use crate::utils::cache::{Interned, INTERNER};
use crate::utils::exec::BootstrapCommand;
use crate::utils::helpers::{
Expand All @@ -38,23 +37,6 @@ use crate::{envify, CLang, DocTests, GitRepo, Mode};

const ADB_TEST_DIR: &str = "/data/local/tmp/work";

// mir-opt tests have different variants depending on whether a target is 32bit or 64bit, and
// blessing them requires blessing with each target. To aid developers, when blessing the mir-opt
// test suite the corresponding target of the opposite pointer size is also blessed.
//
// This array serves as the known mappings between 32bit and 64bit targets. If you're developing on
// a target where a target with the opposite pointer size exists, feel free to add it here.
const MIR_OPT_BLESS_TARGET_MAPPING: &[(&str, &str)] = &[
// (32bit, 64bit)
("i686-unknown-linux-gnu", "x86_64-unknown-linux-gnu"),
("i686-unknown-linux-musl", "x86_64-unknown-linux-musl"),
("i686-pc-windows-msvc", "x86_64-pc-windows-msvc"),
("i686-pc-windows-gnu", "x86_64-pc-windows-gnu"),
// ARM Macs don't have a corresponding 32-bit target that they can (easily)
// build for, so there is no entry for "aarch64-apple-darwin" here.
// Likewise, i686 for macOS is no longer possible to build.
];

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct CrateBootstrap {
path: Interned<PathBuf>,
Expand Down Expand Up @@ -1487,46 +1469,18 @@ impl Step for MirOpt {
})
};

// We use custom logic to bless the mir-opt suite: mir-opt tests have multiple variants
// (32bit vs 64bit, and panic=abort vs panic=unwind), and all of them needs to be blessed.
// When blessing, we try best-effort to also bless the other variants, to aid developers.
if builder.config.cmd.bless() {
let targets = MIR_OPT_BLESS_TARGET_MAPPING
.iter()
.filter(|(target_32bit, target_64bit)| {
*target_32bit == &*self.target.triple || *target_64bit == &*self.target.triple
})
.next()
.map(|(target_32bit, target_64bit)| {
let target_32bit = TargetSelection::from_user(target_32bit);
let target_64bit = TargetSelection::from_user(target_64bit);

// Running compiletest requires a C compiler to be available, but it might not
// have been detected by bootstrap if the target we're testing wasn't in the
// --target flags.
if !builder.cc.borrow().contains_key(&target_32bit) {
utils::cc_detect::find_target(builder, target_32bit);
}
if !builder.cc.borrow().contains_key(&target_64bit) {
utils::cc_detect::find_target(builder, target_64bit);
}
// All that we really need to do is cover all combinations of 32/64-bit and unwind/abort,
// but while we're at it we might as well flex our cross-compilation support. This
// selection covers all our tier 1 operating systems and architectures using only tier
// 1 targets.

vec![target_32bit, target_64bit]
})
.unwrap_or_else(|| {
eprintln!(
"\
Note that not all variants of mir-opt tests are going to be blessed, as no mapping between
a 32bit and a 64bit target was found for {target}.
You can add that mapping by changing MIR_OPT_BLESS_TARGET_MAPPING in src/bootstrap/test.rs",
target = self.target,
);
vec![self.target]
});

for target in targets {
run(target);
for target in ["aarch64-unknown-linux-gnu", "i686-pc-windows-msvc"] {
run(TargetSelection::from_user(target));
}

for target in ["x86_64-apple-darwin", "i686-unknown-linux-musl"] {
let target = TargetSelection::from_user(target);
let panic_abort_target = builder.ensure(MirOptPanicAbortSyntheticTarget {
compiler: self.compiler,
base: target,
Expand Down Expand Up @@ -1616,27 +1570,27 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
.ensure(dist::DebuggerScripts { sysroot: builder.sysroot(compiler), host: target });
}

if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target));
} else {
builder.ensure(compile::Std::new(compiler, target));
}
// Also provide `rust_test_helpers` for the host.
builder.ensure(TestHelpers { target: compiler.host });

// ensure that `libproc_macro` is available on the host.
builder.ensure(compile::Std::new(compiler, compiler.host));

// Also provide `rust_test_helpers` for the host.
builder.ensure(TestHelpers { target: compiler.host });

// As well as the target, except for plain wasm32, which can't build it
if suite != "mir-opt" && !target.contains("wasm") && !target.contains("emscripten") {
builder.ensure(TestHelpers { target });
}

builder.ensure(RemoteCopyLibs { compiler, target });

let mut cmd = builder.tool_cmd(Tool::Compiletest);

if suite == "mir-opt" {
builder.ensure(compile::Std::new_for_mir_opt_tests(compiler, target));
} else {
builder.ensure(compile::Std::new(compiler, target));
}

builder.ensure(RemoteCopyLibs { compiler, target });

// compiletest currently has... a lot of arguments, so let's just pass all
// of them!

Expand Down Expand Up @@ -1745,11 +1699,13 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
flags.push(format!("-Cdebuginfo={}", builder.config.rust_debuginfo_level_tests));
flags.extend(builder.config.cmd.rustc_args().iter().map(|s| s.to_string()));

if let Some(linker) = builder.linker(target) {
cmd.arg("--target-linker").arg(linker);
}
if let Some(linker) = builder.linker(compiler.host) {
cmd.arg("--host-linker").arg(linker);
if suite != "mir-opt" {
if let Some(linker) = builder.linker(target) {
cmd.arg("--target-linker").arg(linker);
}
if let Some(linker) = builder.linker(compiler.host) {
cmd.arg("--host-linker").arg(linker);
}
}

let mut hostflags = flags.clone();
Expand Down Expand Up @@ -1936,15 +1892,17 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--remote-test-client").arg(builder.tool_exe(Tool::RemoteTestClient));
}

// Running a C compiler on MSVC requires a few env vars to be set, to be
// sure to set them here.
//
// Note that if we encounter `PATH` we make sure to append to our own `PATH`
// rather than stomp over it.
if !builder.config.dry_run() && target.is_msvc() {
for &(ref k, ref v) in builder.cc.borrow()[&target].env() {
if k != "PATH" {
cmd.env(k, v);
if suite != "mir-opt" {
// Running a C compiler on MSVC requires a few env vars to be set, to be
// sure to set them here.
//
// Note that if we encounter `PATH` we make sure to append to our own `PATH`
// rather than stomp over it.
if !builder.config.dry_run() && target.is_msvc() {
for &(ref k, ref v) in builder.cc.borrow()[&target].env() {
if k != "PATH" {
cmd.env(k, v);
}
}
}
}
Expand Down
Loading
Loading