Rollup merge of #144011 - Zalathar:check-compiler-no-llvm, r=Kobzol
bootstrap: Don't trigger an unnecessary LLVM build from check builds
Coming back to r-l/r development after a few weeks away, I found a major regression in my dev workflows: running `x check compiler` (either manually or via rust-analyzer) would have the side-effect of building LLVM, even though that shouldn't be necessary.
For my main build directory this would be a minor annoyance, but for my separate rust-analyzer build directory it's a huge problem because it causes a completely separate build of LLVM, which takes a long time and should be completely unnecessary.
---
After some investigation, I tracked down the problem to the `can_skip_build` check in this code:
3014e79f9c/src/bootstrap/src/core/build_steps/compile.rs (L1382-L1396)
Historically, this would skip the LLVM build for stage 0 check builds. But after the recent stage 0 std redesign and some associated check stage renumbering (e.g. rust-lang/rust#143048), the condition `builder.top_stage == build_stage` is now false, because `top_stage` is 1 (due to the renumbering) but `build_stage` is 0 (because a “stage 1” non-library check build still uses the stage 0 compiler).
---
Because this is a critical contributor roadblock for me, I have tried to fix this in a relatively narrow way. It's possible that all of this surrounding logic could be greatly simplified (especially in light of the stage redesign changes), but I didn't want this fix to be held back by scope creep.
---
(Zulip thread: https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Bootstrap.20incorrectly.20building.20LLVM.20for.20check.20builds/near/528991035)
This commit is contained in:
commit
2a4a4112b3
4 changed files with 32 additions and 28 deletions
|
|
@ -355,7 +355,7 @@ impl Step for CodegenBackend {
|
|||
cargo
|
||||
.arg("--manifest-path")
|
||||
.arg(builder.src.join(format!("compiler/rustc_codegen_{backend}/Cargo.toml")));
|
||||
rustc_cargo_env(builder, &mut cargo, target, build_compiler.stage);
|
||||
rustc_cargo_env(builder, &mut cargo, target);
|
||||
|
||||
let _guard = builder.msg_check(format!("rustc_codegen_{backend}"), target, None);
|
||||
|
||||
|
|
|
|||
|
|
@ -1316,15 +1316,10 @@ pub fn rustc_cargo(
|
|||
cargo.env("RUSTC_WRAPPER", ccache);
|
||||
}
|
||||
|
||||
rustc_cargo_env(builder, cargo, target, build_compiler.stage);
|
||||
rustc_cargo_env(builder, cargo, target);
|
||||
}
|
||||
|
||||
pub fn rustc_cargo_env(
|
||||
builder: &Builder<'_>,
|
||||
cargo: &mut Cargo,
|
||||
target: TargetSelection,
|
||||
build_stage: u32,
|
||||
) {
|
||||
pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) {
|
||||
// Set some configuration variables picked up by build scripts and
|
||||
// the compiler alike
|
||||
cargo
|
||||
|
|
@ -1379,18 +1374,24 @@ pub fn rustc_cargo_env(
|
|||
cargo.rustflag("--cfg=llvm_enzyme");
|
||||
}
|
||||
|
||||
// Note that this is disabled if LLVM itself is disabled or we're in a check
|
||||
// build. If we are in a check build we still go ahead here presuming we've
|
||||
// detected that LLVM is already built and good to go which helps prevent
|
||||
// busting caches (e.g. like #71152).
|
||||
// These conditionals represent a tension between three forces:
|
||||
// - For non-check builds, we need to define some LLVM-related environment
|
||||
// variables, requiring LLVM to have been built.
|
||||
// - For check builds, we want to avoid building LLVM if possible.
|
||||
// - Check builds and non-check builds should have the same environment if
|
||||
// possible, to avoid unnecessary rebuilds due to cache-busting.
|
||||
//
|
||||
// Therefore we try to avoid building LLVM for check builds, but only if
|
||||
// building LLVM would be expensive. If "building" LLVM is cheap
|
||||
// (i.e. it's already built or is downloadable), we prefer to maintain a
|
||||
// consistent environment between check and non-check builds.
|
||||
if builder.config.llvm_enabled(target) {
|
||||
let building_is_expensive =
|
||||
let building_llvm_is_expensive =
|
||||
crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target, false)
|
||||
.should_build();
|
||||
// `top_stage == stage` might be false for `check --stage 1`, if we are building the stage 1 compiler
|
||||
let can_skip_build = builder.kind == Kind::Check && builder.top_stage == build_stage;
|
||||
let should_skip_build = building_is_expensive && can_skip_build;
|
||||
if !should_skip_build {
|
||||
|
||||
let skip_llvm = (builder.kind == Kind::Check) && building_llvm_is_expensive;
|
||||
if !skip_llvm {
|
||||
rustc_llvm_env(builder, cargo, target)
|
||||
}
|
||||
}
|
||||
|
|
@ -1407,6 +1408,9 @@ pub fn rustc_cargo_env(
|
|||
|
||||
/// Pass down configuration from the LLVM build into the build of
|
||||
/// rustc_llvm and rustc_codegen_llvm.
|
||||
///
|
||||
/// Note that this has the side-effect of _building LLVM_, which is sometimes
|
||||
/// unwanted (e.g. for check builds).
|
||||
fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) {
|
||||
if builder.config.is_rust_llvm(target) {
|
||||
cargo.env("LLVM_RUSTLLVM", "1");
|
||||
|
|
@ -1665,7 +1669,7 @@ impl Step for CodegenBackend {
|
|||
cargo
|
||||
.arg("--manifest-path")
|
||||
.arg(builder.src.join(format!("compiler/rustc_codegen_{backend}/Cargo.toml")));
|
||||
rustc_cargo_env(builder, &mut cargo, target, compiler.stage);
|
||||
rustc_cargo_env(builder, &mut cargo, target);
|
||||
|
||||
// Ideally, we'd have a separate step for the individual codegen backends,
|
||||
// like we have in tests (test::CodegenGCC) but that would require a lot of restructuring.
|
||||
|
|
|
|||
|
|
@ -3387,7 +3387,7 @@ impl Step for CodegenCranelift {
|
|||
cargo
|
||||
.arg("--manifest-path")
|
||||
.arg(builder.src.join("compiler/rustc_codegen_cranelift/build_system/Cargo.toml"));
|
||||
compile::rustc_cargo_env(builder, &mut cargo, target, compiler.stage);
|
||||
compile::rustc_cargo_env(builder, &mut cargo, target);
|
||||
|
||||
// Avoid incremental cache issues when changing rustc
|
||||
cargo.env("CARGO_BUILD_INCREMENTAL", "false");
|
||||
|
|
@ -3519,7 +3519,7 @@ impl Step for CodegenGCC {
|
|||
cargo
|
||||
.arg("--manifest-path")
|
||||
.arg(builder.src.join("compiler/rustc_codegen_gcc/build_system/Cargo.toml"));
|
||||
compile::rustc_cargo_env(builder, &mut cargo, target, compiler.stage);
|
||||
compile::rustc_cargo_env(builder, &mut cargo, target);
|
||||
add_cg_gcc_cargo_flags(&mut cargo, &gcc);
|
||||
|
||||
// Avoid incremental cache issues when changing rustc
|
||||
|
|
|
|||
|
|
@ -712,7 +712,11 @@ mod snapshot {
|
|||
[build] llvm <host>
|
||||
[build] rustc 0 <host> -> rustc 1 <host>
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_rustc_no_explicit_stage() {
|
||||
let ctx = TestCtx::new();
|
||||
insta::assert_snapshot!(
|
||||
ctx.config("build")
|
||||
.path("rustc")
|
||||
|
|
@ -1303,17 +1307,19 @@ mod snapshot {
|
|||
ctx.config("check")
|
||||
.path("compiler")
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> cranelift 1 <host>
|
||||
[check] rustc 0 <host> -> gcc 1 <host>
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_rustc_no_explicit_stage() {
|
||||
let ctx = TestCtx::new();
|
||||
insta::assert_snapshot!(
|
||||
ctx.config("check")
|
||||
.path("rustc")
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
");
|
||||
}
|
||||
|
|
@ -1333,7 +1339,6 @@ mod snapshot {
|
|||
.path("compiler")
|
||||
.stage(1)
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> cranelift 1 <host>
|
||||
[check] rustc 0 <host> -> gcc 1 <host>
|
||||
|
|
@ -1465,7 +1470,6 @@ mod snapshot {
|
|||
.paths(&["library", "compiler"])
|
||||
.args(&args)
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> cranelift 1 <host>
|
||||
[check] rustc 0 <host> -> gcc 1 <host>
|
||||
|
|
@ -1479,7 +1483,6 @@ mod snapshot {
|
|||
ctx.config("check")
|
||||
.path("miri")
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> Miri 1 <host>
|
||||
");
|
||||
|
|
@ -1500,7 +1503,6 @@ mod snapshot {
|
|||
.path("miri")
|
||||
.stage(1)
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> Miri 1 <host>
|
||||
");
|
||||
|
|
@ -1553,7 +1555,6 @@ mod snapshot {
|
|||
ctx.config("check")
|
||||
.path("rustc_codegen_cranelift")
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> cranelift 1 <host>
|
||||
[check] rustc 0 <host> -> gcc 1 <host>
|
||||
|
|
@ -1567,7 +1568,6 @@ mod snapshot {
|
|||
ctx.config("check")
|
||||
.path("rust-analyzer")
|
||||
.render_steps(), @r"
|
||||
[build] llvm <host>
|
||||
[check] rustc 0 <host> -> rustc 1 <host>
|
||||
[check] rustc 0 <host> -> rust-analyzer 1 <host>
|
||||
");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue