Consolidate LLVM skip in check builds in bootstrap

src/bootstrap/src/core/build_steps/check.rs

@@ -5,8 +5,7 @@ use std::path::{Path, PathBuf};
 
 use crate::core::backend::CodegenBackendKind;
 use crate::core::build_steps::compile::{
-    ArtifactKeepMode, add_to_sysroot, run_cargo, rustc_cargo, rustc_cargo_env, std_cargo,
-    std_crates_for_make_run,
+    ArtifactKeepMode, add_to_sysroot, run_cargo, rustc_cargo, std_cargo, std_crates_for_make_run,
 };
 use crate::core::build_steps::tool;
 use crate::core::build_steps::tool::{
@@ -582,7 +581,6 @@ impl CommandLineStep for CraneliftCodegenBackend {
         cargo
             .arg("--manifest-path")
             .arg(builder.src.join("compiler/rustc_codegen_cranelift/Cargo.toml"));
-        rustc_cargo_env(builder, &mut cargo, target);
         self.build_compiler.configure_cargo(&mut cargo);
 
         let _guard = builder.msg(
@@ -665,7 +663,6 @@ impl CommandLineStep for GccCodegenBackend {
         );
 
         cargo.arg("--manifest-path").arg(builder.src.join("compiler/rustc_codegen_gcc/Cargo.toml"));
-        rustc_cargo_env(builder, &mut cargo, target);
         self.build_compiler.configure_cargo(&mut cargo);
 
         let _guard =

src/bootstrap/src/core/build_steps/compile.rs

@@ -1221,9 +1221,10 @@ pub fn rustc_cargo(
     build_compiler: &Compiler,
     crates: &[String],
 ) {
+    let kind = cargo.kind();
     cargo
         .arg("--features")
-        .arg(builder.rustc_features(builder.kind, target, crates))
+        .arg(builder.rustc_features(kind, target, crates))
         .arg("--manifest-path")
         .arg(builder.src.join("compiler/rustc/Cargo.toml"));
 
@@ -1317,7 +1318,7 @@ pub fn rustc_cargo(
     rustc_cargo_env(builder, cargo, target);
 }
 
-pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) {
+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
@@ -1386,18 +1387,29 @@ pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetS
     //   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.
+    //   possible, to avoid unnecessary rebuilds due to cache-busting (in the same stage).
     //
-    // 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 we have either:
+    // - LLVM already locally built
+    // - download-ci-llvm enabled
+    // - LLVM provided externally through a llvm-config
+    //
+    // and we do a check-like build, we run rustc_llvm as normally, to maintain a
+    // consistent environment between check and non-check builds
+    //
+    // However, if neither from the above three bullet points is true, and we do a check-like build,
+    // we skip running rustc_llvm by setting the RUST_CHECK environment variable.
+    //
+    // Note that if download-ci-llvm is enabled, `prebuilt_llvm_output` will *eagerly* download
+    // LLVM from CI, thus making it locally available.
     if builder.config.llvm_enabled(target) {
         let building_llvm_is_expensive = prebuilt_llvm_output(builder, target).is_none();
 
-        let skip_llvm = (cargo.kind() == Kind::Check) && building_llvm_is_expensive;
-        if !skip_llvm {
-            rustc_llvm_env(builder, cargo, target)
+        let skip_llvm = cargo.kind().is_check_like() && building_llvm_is_expensive;
+        if skip_llvm {
+            cargo.env("RUST_CHECK", "1");
+        } else {
+            rustc_llvm_env(builder, cargo, target);
         }
     }
 
@@ -1420,7 +1432,7 @@ pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetS
 /// 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
+/// Note that calling this function 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) {
     let llvm_output = builder.ensure(llvm::Llvm { target });
@@ -1718,7 +1730,6 @@ impl CommandLineStep for GccCodegenBackend {
             Kind::Build,
         );
         cargo.arg("--manifest-path").arg(builder.src.join("compiler/rustc_codegen_gcc/Cargo.toml"));
-        rustc_cargo_env(builder, &mut cargo, host);
 
         let _guard =
             builder.msg(Kind::Build, "codegen backend gcc", Mode::Codegen, build_compiler, host);
@@ -1789,7 +1800,6 @@ impl CommandLineStep for CraneliftCodegenBackend {
         cargo
             .arg("--manifest-path")
             .arg(builder.src.join("compiler/rustc_codegen_cranelift/Cargo.toml"));
-        rustc_cargo_env(builder, &mut cargo, target);
 
         let _guard = builder.msg(
             Kind::Build,

src/bootstrap/src/core/build_steps/test.rs

@@ -4418,7 +4418,6 @@ impl CommandLineStep 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);
 
         // Avoid incremental cache issues when changing rustc
         cargo.env("CARGO_BUILD_INCREMENTAL", "false");
@@ -4544,7 +4543,6 @@ impl CommandLineStep 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);
         add_cg_gcc_cargo_flags(&mut cargo, &gcc);
 
         // Avoid incremental cache issues when changing rustc

src/bootstrap/src/core/builder/cargo.rs

@@ -5,7 +5,6 @@ use std::{env, fs};
 
 use super::{Builder, Kind};
 use crate::core::build_steps::compile::is_lto_stage;
-use crate::core::build_steps::llvm::prebuilt_llvm_output;
 use crate::core::build_steps::test;
 use crate::core::build_steps::tool::SourceType;
 use crate::core::compiler::Compiler;
@@ -752,20 +751,6 @@ impl Builder<'_> {
             cargo.env("REAL_LIBRARY_PATH", e);
         }
 
-        // Set a flag for `check`/`clippy`/`fix`, so that certain build
-        // scripts can do less work (i.e. not building/requiring LLVM).
-        if matches!(cmd_kind, Kind::Check | Kind::Clippy | Kind::Fix) {
-            // If we've not yet built LLVM, or it's stale, then bust
-            // the rustc_llvm cache. That will always work, even though it
-            // may mean that on the next non-check build we'll need to rebuild
-            // rustc_llvm. But if LLVM is stale, that'll be a tiny amount
-            // of work comparatively, and we'd likely need to rebuild it anyway,
-            // so that's okay.
-            if prebuilt_llvm_output(self, target).is_none() {
-                cargo.env("RUST_CHECK", "1");
-            }
-        }
-
         // Forward `./x fix --allow-dirty` from bootstrap to cargo.
         if matches!(cmd_kind, Kind::Fix)
             && let Subcommand::Fix { allow_dirty } = self.config.cmd

src/bootstrap/src/core/builder/mod.rs

@@ -688,6 +688,28 @@ impl Kind {
         }
         .to_owned()
     }
+
+    /// Is this a command similar to check, which only runs the compiler frontend and doesn't
+    /// build code for the target? (it can still build code for the host, i.e. proc macros).
+    pub fn is_check_like(&self) -> bool {
+        match self {
+            Kind::Check | Kind::Clippy | Kind::Fix | Kind::Doc => true,
+            Kind::Build
+            | Kind::Format
+            | Kind::Test
+            | Kind::Miri
+            | Kind::MiriSetup
+            | Kind::MiriTest
+            | Kind::Bench
+            | Kind::Clean
+            | Kind::Dist
+            | Kind::Install
+            | Kind::Run
+            | Kind::Setup
+            | Kind::Vendor
+            | Kind::Perf => false,
+        }
+    }
 }
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]

src/bootstrap/src/core/builder/tests.rs

@@ -2541,9 +2541,8 @@ mod snapshot {
             ctx.config("doc")
                 .path("compiler")
                 .stage(1)
-                .render_steps(), @r"
+                .render_steps(), @"
         [build] rustdoc 0 <host>
-        [build] llvm <host>
         [doc] rustc 0 <host> -> rustc 1 <host>
         ");
     }
@@ -2643,10 +2642,7 @@ mod snapshot {
         insta::assert_snapshot!(
             ctx.config("clippy")
                 .path("compiler")
-                .render_steps(), @r"
-        [build] llvm <host>
-        [clippy] rustc 0 <host> -> rustc 1 <host>
-        ");
+                .render_steps(), @"[clippy] rustc 0 <host> -> rustc 1 <host>");
     }
 
     #[test]
@@ -3024,10 +3020,7 @@ mod snapshot {
     #[test]
     fn fix_compiler() {
         let ctx = TestCtx::new();
-        insta::assert_snapshot!(ctx.config("fix").path("compiler").render_steps(), @r"
-        [build] llvm <host>
-        [fix] rustc 0 <host> -> rustc 1 <host> (77 crates)
-        ");
+        insta::assert_snapshot!(ctx.config("fix").path("compiler").render_steps(), @"[fix] rustc 0 <host> -> rustc 1 <host> (77 crates)");
     }
 }
 

src/bootstrap/src/core/session.rs

@@ -698,7 +698,7 @@ impl Session {
         {
             features.push(allocator_feature_name);
         }
-        if (self.config.llvm_enabled(target) || kind == Kind::Check) && check("llvm") {
+        if self.config.llvm_enabled(target) && check("llvm") {
             features.push("llvm");
         }
         if self.config.llvm_offload {
@@ -708,7 +708,7 @@ impl Session {
         if self.config.rust_randomize_layout && check("rustc_randomized_layouts") {
             features.push("rustc_randomized_layouts");
         }
-        if self.config.compile_time_deps && kind == Kind::Check {
+        if self.config.compile_time_deps && kind.is_check_like() {
             features.push("check_only");
         }