Add ToolTarget to bootstrap
#143641
Add ToolTarget to bootstrap
#143641
Conversation
rustbot
added
A-testsuite
rustbot
added
the
S-waiting-on-review
|
#142357 was merged, so undrafting. |
| @@ -365,6 +365,17 @@ pub(crate) fn get_tool_rustc_compiler( | |||
| builder.compiler(target_compiler.stage.saturating_sub(1), builder.config.host_target) | |||
| } | |||
|
|
|||
| /// Returns a compiler that is able to compile a `ToolTarget` tool for the given `target`. | |||
| pub(crate) fn get_tool_target_compiler(builder: &Builder<'_>, target: TargetSelection) -> Compiler { | |||
| todo!("FIX"); | |||
There was a problem hiding this comment.
Problem [STAGING 0/2]: leftover todo!()? 🤔
| // FIXME: should this be builder.top_stage to avoid rebuilds? | ||
| builder.compiler(1, target) |
There was a problem hiding this comment.
Discussion: hm, for dist tools (cross-compiled for target platform), does the final distributed tool need to be built by the stage 2 compiler? Though I imagine the differences of tools produced by
| if builder.host_target == target { | ||
| builder.compiler(0, builder.host_target) |
There was a problem hiding this comment.
Question [STAGING 1/2]: wait, does this actually agree with the comment
While we could compile them using the stage0 compiler when not cross-compiling, we instead
use the in-tree compiler (and std) to build them, so that we can ship e.g. std security
fixes and avoid depending fully on stage0 for the artifacts that we ship.
are we not using the stage 0 compiler (not in-tree!) when not cross-compiling?
| /// Returns the smallest stage compiler that is able to compile code for the given `target`. | ||
| pub(crate) fn get_compiler_for_target(builder: &Builder<'_>, target: TargetSelection) -> Compiler { | ||
| let compiler = if builder.host_target == target { | ||
| builder.compiler(0, builder.host_target) | ||
| } else { | ||
| // FIXME: should this be builder.top_stage to avoid rebuilds? | ||
| builder.compiler(1, target) | ||
| } | ||
| builder.compiler(1, builder.host_target) | ||
| }; | ||
| builder.std(compiler, target); | ||
| compiler |
There was a problem hiding this comment.
Question [STAGING 2/2]: oh, it's changed in this commit. But wait, doesn't this still disagree with the comment re. building ToolTarget tools with the in-tree compiler and std, even when not cross-compiling?
| /// Determines how to build a `ToolTarget`, i.e. which compiler should be used to compile it. | ||
| /// The compiler stage is automatically auto-bumped if we need to cross-compile a stage 1 tool. | ||
| pub enum ToolTargetBuildMode { | ||
| /// Build the tool using rustc that corresponds to the selected CLI stage. | ||
| Build(TargetSelection), | ||
| /// Build the tool so that it can be attached to the sysroot of the passed compiler. | ||
| /// Since we always dist stage 2+, the compiler that builds the tool in this case has to be | ||
| /// stage 1+. | ||
| Dist(Compiler), | ||
| } |
There was a problem hiding this comment.
Discussion [STAGING 3/2]: oh I see, this is exactly what I was curious about.
| let (target, min_build_compiler_stage) = match mode { | ||
| ToolTargetBuildMode::Build(target) => { | ||
| assert!(builder.top_stage > 0); | ||
| (target, builder.top_stage - 1) | ||
| } | ||
| ToolTargetBuildMode::Dist(target_compiler) => { | ||
| assert!(target_compiler.stage > 0); | ||
| (target_compiler.host, target_compiler.stage - 1) | ||
| } | ||
| }; | ||
| let compiler = if builder.host_target == target { | ||
| builder.compiler(0, builder.host_target) | ||
| builder.compiler(min_build_compiler_stage, builder.host_target) | ||
| } else { | ||
| builder.compiler(1, builder.host_target) | ||
| builder.compiler(min_build_compiler_stage.max(1), builder.host_target) |
There was a problem hiding this comment.
Question [STAGING 4/2]: I have to admit this logic is a bit confusing to me, especially the stage adjustments and also the stage cap. I need to think about this a bit.
| path: "src/tools/wasm-component-ld", | ||
| source_type: SourceType::InTree, | ||
| extra_features: vec![], | ||
| allow_features: "min-specialization", |
There was a problem hiding this comment.
Question: does wasm-component-ld actually need min-specialization? At least, I'm not seeing it used at all.
| /// Returns `LlvmBitcodeLinker` that should be **used** by the passed compiler. | ||
| pub fn for_compiler(builder: &Builder<'_>, target_compiler: Compiler) -> Self { |
There was a problem hiding this comment.
Suggestion: for_use_by_compiler?
Oh, you thought I'm done with refactoring bootstrap tools? Na-ah, think again! After the failure of #143581,
ToolTargetis back with a vengeance. This time, I implemented the test changes and tool cleanups without forcing these tools to be built with the stage0 compiler.There are still some small wins though,
LlvmBitcodeLinkernow starts at stage 1, and not stage 2. Cargo should also be ported to this new mode, but I'm leaving that for a follow-up PR.Hopefully X-th time's the charm 🤞
r? @jieyouxu