Rollup merge of #145310 - Kobzol:compiler-for-revamp, r=jieyouxu

Reduce usage of `compiler_for` in bootstrap While working on refactoring/fixing `dist` steps, I realized that `build.full-bootstrap` does much more than it should, and that it its documentation is wrong. It seems that the main purpose of this option should be to enable/disable stdlib/compiler uplifting (https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Purpose.20of.20.60build.2Efull-bootstrap.60/with/533985624), but currently it also affects staging, or more precisely which compiler will be used to build selected steps, because this option is used in the cursed `compiler_for` function. I would like to change the option it so that it *only* affects uplifting, and doesn't affect stage selection, which I (partially) did in this PR. I removed the usage of `compiler_for` from the `Std` and `Rustc` steps, and explicitly implemented uplifting, without going through `compiler_for`. The only remaining usages of `compiler_for` are in dist steps (which I'm currently refactoring, will send a PR later) and test steps (which I will take a look at after dist). After that we can finally remove the function. I tried to document the case when uplifting was happening during cross-compilation, which was very implicit before. I also did a slight change in the uplifting logic for rustc when cross-compiling. Before, we would attempt to uplift a stage1 rustc, but that is not really a thing when cross-compiling. r? `@jieyouxu`

Author: Kobzol

bootstrap.example.toml

@@ -345,9 +345,9 @@
 # want to use vendoring. See https://forge.rust-lang.org/infra/other-installation-methods.html#source-code.
 #build.vendor = if "is a tarball source" && "vendor" dir exists && ".cargo/config.toml" file exists { true } else { false }
 
-# Typically the build system will build the Rust compiler twice. The second
-# compiler, however, will simply use its own libraries to link against. If you
-# would rather to perform a full bootstrap, compiling the compiler three times,
+# If you build the compiler more than twice (stage3+) or the standard library more than once
+# (stage 2+), the third compiler and second library will get uplifted from stage2 and stage1,
+# respectively. If you would like to disable this uplifting, and rather perform a full bootstrap,
 # then you can set this option to true.
 #
 # This is only useful for verifying that rustc generates reproducible builds.

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

@@ -37,11 +37,12 @@ use crate::{
  debug, trace,
 };
 
-/// Build a standard library for the given `target` using the given `compiler`.
+/// Build a standard library for the given `target` using the given `build_compiler`.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Std {
  pub target: TargetSelection,
- pub compiler: Compiler,
+ /// Compiler that builds the standard library.
+ pub build_compiler: Compiler,
  /// Whether to build only a subset of crates in the standard library.
  ///
  /// This shouldn't be used from other steps; see the comment on [`Rustc`].
@@ -54,10 +55,10 @@ pub struct Std {
 }
 
 impl Std {
- pub fn new(compiler: Compiler, target: TargetSelection) -> Self {
+ pub fn new(build_compiler: Compiler, target: TargetSelection) -> Self {
  Self {
  target,
- compiler,
+ build_compiler,
  crates: Default::default(),
  force_recompile: false,
  extra_rust_args: &[],
@@ -120,7 +121,7 @@ impl Step for Std {
  trace!(force_recompile);
 
  run.builder.ensure(Std {
- compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
+ build_compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
  target: run.target,
  crates,
  force_recompile,
@@ -138,8 +139,8 @@ impl Step for Std {
  let target = self.target;
 
  // We already have std ready to be used for stage 0.
- if self.compiler.stage == 0 {
- let compiler = self.compiler;
+ if self.build_compiler.stage == 0 {
+ let compiler = self.build_compiler;
  builder.ensure(StdLink::from_std(self, compiler));
 
  return;
@@ -148,9 +149,10 @@ impl Step for Std {
  let build_compiler = if builder.download_rustc() && self.force_recompile {
  // When there are changes in the library tree with CI-rustc, we want to build
  // the stageN library and that requires using stageN-1 compiler.
- builder.compiler(self.compiler.stage.saturating_sub(1), builder.config.host_target)
+ builder
+ .compiler(self.build_compiler.stage.saturating_sub(1), builder.config.host_target)
  } else {
- self.compiler
+ self.build_compiler
  };
 
  // When using `download-rustc`, we already have artifacts for the host available. Don't
@@ -187,51 +189,50 @@ impl Step for Std {
 
  let mut target_deps = builder.ensure(StartupObjects { compiler: build_compiler, target });
 
- let compiler_to_use =
- builder.compiler_for(build_compiler.stage, build_compiler.host, target);
- trace!(?compiler_to_use);
-
- if compiler_to_use != build_compiler
- // Never uplift std unless we have compiled stage 1; if stage 1 is compiled,
- // uplift it from there.
- //
- // FIXME: improve `fn compiler_for` to avoid adding stage condition here.
- && build_compiler.stage > 1
+ // Stage of the stdlib that we're building
+ let stage = build_compiler.stage;
+
+ // If we're building a stage2+ libstd, full bootstrap is
+ // disabled and we have a stage1 libstd already compiled for the given target,
+ // then simply uplift a previously built stage1 library.
+ if build_compiler.stage > 1
+ && !builder.config.full_bootstrap
+ // This estimates if a stage1 libstd exists for the given target. If we're not
+ // cross-compiling, it should definitely exist by the time we're building a stage2
+ // libstd.
+ // Or if we are cross-compiling, and we are building a cross-compiled rustc, then that
+ // rustc needs to link to a cross-compiled libstd, so again we should have a stage1
+ // libstd for the given target prepared.
+ // Even if we guess wrong in the cross-compiled case, the worst that should happen is
+ // that we build a fresh stage1 libstd below, and then we immediately uplift it, so we
+ // don't pay the libstd build cost twice.
+ && (target == builder.host_target || builder.config.hosts.contains(&target))
  {
- trace!(
- ?compiler_to_use,
- ?build_compiler,
- "build_compiler != compiler_to_use, uplifting library"
- );
+ let build_compiler_for_std_to_uplift = builder.compiler(1, builder.host_target);
+ builder.std(build_compiler_for_std_to_uplift, target);
 
- builder.std(compiler_to_use, target);
- let msg = if compiler_to_use.host == target {
+ let msg = if build_compiler_for_std_to_uplift.host == target {
  format!(
- "Uplifting library (stage{} -> stage{})",
- compiler_to_use.stage, build_compiler.stage
+ "Uplifting library (stage{} -> stage{stage})",
+ build_compiler_for_std_to_uplift.stage
  )
  } else {
  format!(
- "Uplifting library (stage{}:{} -> stage{}:{})",
- compiler_to_use.stage, compiler_to_use.host, build_compiler.stage, target
+ "Uplifting library (stage{}:{} -> stage{stage}:{target})",
+ build_compiler_for_std_to_uplift.stage, build_compiler_for_std_to_uplift.host,
  )
  };
+
  builder.info(&msg);
 
  // Even if we're not building std this stage, the new sysroot must
  // still contain the third party objects needed by various targets.
  self.copy_extra_objects(builder, &build_compiler, target);
 
- builder.ensure(StdLink::from_std(self, compiler_to_use));
+ builder.ensure(StdLink::from_std(self, build_compiler_for_std_to_uplift));
  return;
  }
 
- trace!(
- ?compiler_to_use,
- ?build_compiler,
- "compiler == compiler_to_use, handling not-cross-compile scenario"
- );
-
  target_deps.extend(self.copy_extra_objects(builder, &build_compiler, target));
 
  // We build a sysroot for mir-opt tests using the same trick that Miri does: A check build
@@ -299,7 +300,7 @@ impl Step for Std {
  }
 
  fn metadata(&self) -> Option<StepMetadata> {
- Some(StepMetadata::build("std", self.target).built_by(self.compiler))
+ Some(StepMetadata::build("std", self.target).built_by(self.build_compiler))
  }
 }
 
@@ -665,6 +666,14 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, cargo: &mut Car
  cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)");
 }
 
+/// Link all libstd rlibs/dylibs into a sysroot of `target_compiler`.
+///
+/// Links those artifacts generated by `compiler` to the `stage` compiler's
+/// sysroot for the specified `host` and `target`.
+///
+/// Note that this assumes that `compiler` has already generated the libstd
+/// libraries for `target`, and this method will find them in the relevant
+/// output directory.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct StdLink {
  pub compiler: Compiler,
@@ -680,7 +689,7 @@ impl StdLink {
  pub fn from_std(std: Std, host_compiler: Compiler) -> Self {
  Self {
  compiler: host_compiler,
- target_compiler: std.compiler,
+ target_compiler: std.build_compiler,
  target: std.target,
  crates: std.crates,
  force_recompile: std.force_recompile,
@@ -951,14 +960,8 @@ impl Rustc {
 }
 
 impl Step for Rustc {
- /// We return the stage of the "actual" compiler (not the uplifted one).
- ///
- /// By "actual" we refer to the uplifting logic where we may not compile the requested stage;
- /// instead, we uplift it from the previous stages. Which can lead to bootstrap failures in
- /// specific situations where we request stage X from other steps. However we may end up
- /// uplifting it from stage Y, causing the other stage to fail when attempting to link with
- /// stage X which was never actually built.
- type Output = u32;
+ type Output = ();
+
  const IS_HOST: bool = true;
  const DEFAULT: bool = false;
 
@@ -997,7 +1000,7 @@ impl Step for Rustc {
  /// This will build the compiler for a particular stage of the build using
  /// the `build_compiler` targeting the `target` architecture. The artifacts
  /// created will also be linked into the sysroot directory.
- fn run(self, builder: &Builder<'_>) -> u32 {
+ fn run(self, builder: &Builder<'_>) {
  let build_compiler = self.build_compiler;
  let target = self.target;
 
@@ -1013,7 +1016,7 @@ impl Step for Rustc {
  &sysroot,
  builder.config.ci_rustc_dev_contents(),
  );
- return build_compiler.stage;
+ return;
  }
 
  // Build a standard library for `target` using the `build_compiler`.
@@ -1027,31 +1030,33 @@ impl Step for Rustc {
  builder.info("WARNING: Use `--keep-stage-std` if you want to rebuild the compiler when it changes");
  builder.ensure(RustcLink::from_rustc(self, build_compiler));
 
- return build_compiler.stage;
+ return;
  }
 
- let compiler_to_use =
- builder.compiler_for(build_compiler.stage, build_compiler.host, target);
- if compiler_to_use != build_compiler {
- builder.ensure(Rustc::new(compiler_to_use, target));
- let msg = if compiler_to_use.host == target {
- format!(
- "Uplifting rustc (stage{} -> stage{})",
- compiler_to_use.stage,
- build_compiler.stage + 1
- )
+ // The stage of the compiler that we're building
+ let stage = build_compiler.stage + 1;
+
+ // If we are building a stage3+ compiler, and full bootstrap is disabled, and we have a
+ // previous rustc available, we will uplift a compiler from a previous stage.
+ if build_compiler.stage >= 2
+ && !builder.config.full_bootstrap
+ && (target == builder.host_target || builder.hosts.contains(&target))
+ {
+ // If we're cross-compiling, the earliest rustc that we could have is stage 2.
+ // If we're not cross-compiling, then we should have rustc stage 1.
+ let stage_to_uplift = if target == builder.host_target { 1 } else { 2 };
+ let rustc_to_uplift = builder.compiler(stage_to_uplift, target);
+ let msg = if rustc_to_uplift.host == target {
+ format!("Uplifting rustc (stage{} -> stage{stage})", rustc_to_uplift.stage,)
  } else {
  format!(
- "Uplifting rustc (stage{}:{} -> stage{}:{})",
- compiler_to_use.stage,
- compiler_to_use.host,
- build_compiler.stage + 1,
- target
+ "Uplifting rustc (stage{}:{} -> stage{stage}:{target})",
+ rustc_to_uplift.stage, rustc_to_uplift.host,
  )
  };
  builder.info(&msg);
- builder.ensure(RustcLink::from_rustc(self, compiler_to_use));
- return compiler_to_use.stage;
+ builder.ensure(RustcLink::from_rustc(self, rustc_to_uplift));
+ return;
  }
 
  // Build a standard library for the current host target using the `build_compiler`.
@@ -1128,8 +1133,6 @@ impl Step for Rustc {
  self,
  builder.compiler(build_compiler.stage, builder.config.host_target),
  ));
-
- build_compiler.stage
  }
 
  fn metadata(&self) -> Option<StepMetadata> {
@@ -1909,12 +1912,18 @@ impl Step for Sysroot {
  }
 }
 
+/// Prepare a compiler sysroot.
+///
+/// The sysroot may contain various things useful for running the compiler, like linkers and
+/// linker wrappers (LLD, LLVM bitcode linker, etc.).
+///
+/// This will assemble a compiler in `build/$target/stage$stage`.
 #[derive(Debug, PartialOrd, Ord, Clone, PartialEq, Eq, Hash)]
 pub struct Assemble {
  /// The compiler which we will produce in this step. Assemble itself will
  /// take care of ensuring that the necessary prerequisites to do so exist,
- /// that is, this target can be a stage2 compiler and Assemble will build
- /// previous stages for you.
+ /// that is, this can be e.g. a stage2 compiler and Assemble will build
+ /// the previous stages for you.
  pub target_compiler: Compiler,
 }
 
@@ -1932,11 +1941,6 @@ impl Step for Assemble {
  });
  }
 
- /// Prepare a new compiler from the artifacts in `stage`
- ///
- /// This will assemble a compiler in `build/$host/stage$stage`. The compiler
- /// must have been previously produced by the `stage - 1` builder.build
- /// compiler.
  fn run(self, builder: &Builder<'_>) -> Compiler {
  let target_compiler = self.target_compiler;
 
@@ -2065,7 +2069,7 @@ impl Step for Assemble {
  target_compiler.stage - 1,
  builder.config.host_target,
  );
- let mut build_compiler =
+ let build_compiler =
  builder.compiler(target_compiler.stage - 1, builder.config.host_target);
 
  // Build enzyme
@@ -2089,24 +2093,13 @@ impl Step for Assemble {
  }
 
  // Build the libraries for this compiler to link to (i.e., the libraries
- // it uses at runtime). NOTE: Crates the target compiler compiles don't
- // link to these. (FIXME: Is that correct? It seems to be correct most
- // of the time but I think we do link to these for stage2/bin compilers
- // when not performing a full bootstrap).
+ // it uses at runtime).
  debug!(
  ?build_compiler,
  "target_compiler.host" = ?target_compiler.host,
  "building compiler libraries to link to"
  );
- let actual_stage = builder.ensure(Rustc::new(build_compiler, target_compiler.host));
- // Current build_compiler.stage might be uplifted instead of being built; so update it
- // to not fail while linking the artifacts.
- debug!(
- "(old) build_compiler.stage" = build_compiler.stage,
- "(adjusted) build_compiler.stage" = actual_stage,
- "temporarily adjusting `build_compiler.stage` to account for uplifted libraries"
- );
- build_compiler.stage = actual_stage;
+ builder.ensure(Rustc::new(build_compiler, target_compiler.host));
 
  let stage = target_compiler.stage;
  let host = target_compiler.host;