Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,7 @@ pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetS
if builder.config.llvm_enabled(target) {
let building_llvm_is_expensive = prebuilt_llvm_output(builder, target).is_none();

let skip_llvm = (builder.kind == Kind::Check) && building_llvm_is_expensive;
let skip_llvm = (cargo.kind() == Kind::Check) && building_llvm_is_expensive;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: hm, in some sense the "overall" (?) build kind (tied to the cli invocation) is not the same as the "local" build kind (in the case of using cargo, which check/build etc. is used). I.e. the over build kind matters only for the final "step", but it may be multiple local "build" kind steps leading up to the final step.

I wonder if there are ways to make this logic more clear but yeah.

if !skip_llvm {
rustc_llvm_env(builder, cargo, target)
}
Expand Down
18 changes: 17 additions & 1 deletion src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,13 @@ pub fn prebuilt_llvm_output(builder: &Builder<'_>, target: TargetSelection) -> O
kind: LlvmKind::External,
});
}
None

// If LLVM is not available from CI, not externally, it is still possible that it was already
// built locally before. In that case we still treat it as prebuilt config.
match get_locally_built_llvm_build_status(builder, target) {
LlvmBuildStatus::AlreadyBuilt(output) => Some(output),
LlvmBuildStatus::ShouldBuild(_) => None,
}
}

/// This returns whether we've already previously built LLVM.
Expand All @@ -186,6 +192,16 @@ pub fn get_llvm_build_status(builder: &Builder<'_>, target: TargetSelection) ->
// If submodules are disabled, this does nothing.
builder.config.update_submodule("src/llvm-project");

get_locally_built_llvm_build_status(builder, target)
}

/// Return build status of LLVM, considering only the (possibly) locally built LLVM.
///
/// Calling this function should never attempt to checkout the LLVM submodule.
fn get_locally_built_llvm_build_status(
builder: &Builder<'_>,
target: TargetSelection,
) -> LlvmBuildStatus {
let out_dir = builder.llvm_out(target);

let build_llvm_config = if let Some(build_llvm_config) = builder
Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ pub(crate) struct Cargo {
build_compiler_stage: u32,
extra_rustflags: Vec<String>,
profile: Option<&'static str>,
kind: Kind,
}

impl Cargo {
Expand Down Expand Up @@ -203,6 +204,10 @@ impl Cargo {
self.into()
}

pub(crate) fn kind(&self) -> Kind {
self.kind
}

/// Same as [`Cargo::new`] except this one doesn't configure the linker with
/// [`Cargo::configure_linker`].
#[track_caller]
Expand Down Expand Up @@ -1530,6 +1535,7 @@ impl Builder<'_> {
build_compiler_stage,
extra_rustflags,
profile,
kind: cmd_kind,
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/bootstrap/src/core/builder/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2704,8 +2704,7 @@ mod snapshot {
ctx.config("clippy")
.path("miri")
.stage(1)
.render_steps(), @r"
[build] llvm <host>
.render_steps(), @"
[check] rustc 0 <host> -> rustc 1 <host>
[clippy] rustc 0 <host> -> miri 1 <host>
");
Expand Down
Loading