Skip to content

Assorted bootstrap LLVM refactors (part 4/N) #161290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Kobzol wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Assorted bootstrap LLVM refactors (part 4/N) #161290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
794790d
Reduce usage of the `download_from_ci` function and rename it to `req…
Kobzol Aug 18, 2026
23ed00f
Move `download-ci-llvm` `if-unchanged` detection logic into the `Llvm…
Kobzol Aug 18, 2026
9cc2384
Allow downloading LLVM for arbitrary targets
Kobzol Aug 18, 2026
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
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/build_steps/compile.rs
View file
Open in desktop

@jieyouxu jieyouxu Aug 26, 2026

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.

There is one annoyance related to that, and that is that download-ci-llvm now applies to all targets for which you try to build LLVM (d'uh), but that also means that if (for whatever reason) LLVM fails to be downloaded from CI, the build will fail. So if you build for target T2 from target T1:

  • If you want to download T1, but build T2, that's not possible to express.
  • If T2 fails to be downloaded, the build fails, even if it could be built locally.

I think that we mostly have four options how to deal with this:

  1. Just ignore it and wait to see if someone complains.
  2. Revert the change and always download only for the host target. Worked so far. However, downloading LLVM for non-host targets would be quite useful for further bootstrap improvements and refactorings, because the current logic around sysroots and libdirs is.. convoluted, to say the last, and making cross-compilation easier would help with that a lot.
  3. Allow specifying download-ci-llvm per target in the target config section. So that you can say that you want to download for T1, but build for T2.
  4. Make download failures non-fatal, and cause them to trigger a local build. This would also help with removing the hacky is_ci_llvm_available_for_target logic, which hard-codes a bunch of targets to "know" which ones offer LLVM and which don't. We could just try to download, and if the result is 404, then we print a warning and continue with building (but this is slightly orthogonal, we can do this even if we don't make LLVM build failures non-fatal).

I think that 3 or 4 would be the best solution, perhaps slightly opting for 4. If we get a 404, there's no way we can download, so we build instead. If we get a different error, we still make the failed download fail the build. And only if someone has a use-case for 3, we'd add the new config.

I'm on board with option (4) but with a caveat:

  • If user did not specify their preference (so getting a cascading default to download-ci-llvm = "if-unchanged") or if they explicitly specified download-ci-llvm = "if-unchanged", then "try to download, build if missing" fallback feels reasonable.
  • If user specified download-ci-llvm = true, we should fail the build if no CI LLVM is available.

I think this matches what currently download-ci-llvm = "if-unchanged" for host targets do?

To clarify, I think what you mean here is for download-ci-llvm = "if-unchanged"?

@Kobzol Kobzol Aug 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this matches what currently download-ci-llvm = "if-unchanged" for host targets do?

Actualy, it doesn't :) true and if-unchanged are not different in this behavior currently. If you use either of them, and the given target is unavailable on CI, bootstrap will just build LLVM locally. It would seem surprising to me if those two modes had a different behavior in this specific aspect.

(By the way, I'm considering merging those two modes together, same as we do for gcc. So that you only say true, and if there are local changes, you would build. But if the LLVM submodule is checked out, you wouldn't check git changes, ofc.)

@jieyouxu jieyouxu Aug 27, 2026

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.

Hm... I guess that sometimes I do want to download-ci-llvm to fail the build if CI LLVM cannot be used for whatever reason and never checkout the llvm submodule. (In that, for specific cases like testing in-tree tools I prefer to fail the build rather than trying to recover.)

But yeah if this matches existing behavior then we can consider revisiting this behavior later.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use tracing::span;

use crate::core::backend::CodegenBackendKind;
use crate::core::build_steps::gcc::{Gcc, GccOutput, GccTargetPair};
use crate::core::build_steps::llvm::{LlvmFromCi, prebuilt_llvm_output};
use crate::core::build_steps::llvm::{LlvmFromCi, LlvmKind, prebuilt_llvm_output};
use crate::core::build_steps::tool::{RustcPrivateCompilers, SourceType, copy_lld_artifacts};
use crate::core::build_steps::{dist, llvm};
use crate::core::builder::{
Expand Down Expand Up @@ -2200,7 +2200,7 @@ impl CommandLineStep for Assemble {
let src_path = llvm_bin_dir.join(&tool_exe);

// When using `download-ci-llvm`, some of the tools may not exist, so skip trying to copy them.
if !src_path.exists() && builder.config.llvm_ci_mode.download_from_ci() {
if !src_path.exists() && llvm_output.kind() == LlvmKind::DownloadedFromCi {
eprintln!("{} does not exist; skipping copy", src_path.display());
continue;
}
Expand Down
20 changes: 6 additions & 14 deletions src/bootstrap/src/core/build_steps/dist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::core::build_steps::compile::{
use crate::core::build_steps::doc::DocumentationFormat;
use crate::core::build_steps::gcc::GccTargetPair;
use crate::core::build_steps::llvm::{
LLVM_CI_LINK_TYPE_PATH, LlvmBuildStatus, get_llvm_build_status,
LLVM_CI_LINK_TYPE_PATH, LlvmBuildStatus, LlvmKind, get_llvm_build_status,
};
use crate::core::build_steps::tool::{
self, RustcPrivateCompilers, ToolTargetBuildMode, get_tool_target_compiler,
Expand Down Expand Up @@ -2525,12 +2525,7 @@ fn maybe_install_llvm(
// If the LLVM is coming from ourselves (just from CI) though, we
// still want to install it, as it otherwise won't be available.

// FIXME: this should be simplified once we stop pre-setting LLVM CI llvm-config during
// config parsing.
let is_system_llvm =
builder.config.target_config.get(&target).and_then(|t| t.llvm_config.as_ref()).is_some()
&& !(builder.config.llvm_ci_mode.download_from_ci()
&& builder.config.is_host_target(target));
let is_system_llvm = llvm.llvm_output().kind() == LlvmKind::External;
if is_system_llvm {
trace!("system LLVM requested, no install");
return false;
Expand Down Expand Up @@ -2712,11 +2707,10 @@ impl CommandLineStep for LlvmTools {

let target = self.target;

let llvm_output = builder.ensure(crate::core::build_steps::llvm::Llvm { target });

// Run only if a custom llvm-config is not used
if let Some(config) = builder.config.target_config.get(&target)
&& !builder.config.llvm_ci_mode.download_from_ci()
&& config.llvm_config.is_some()
{
if llvm_output.kind() == LlvmKind::External {
builder.info(&format!("Skipping LlvmTools ({target}): external LLVM"));
return None;
}
Expand All @@ -2725,8 +2719,6 @@ impl CommandLineStep for LlvmTools {
builder.require_submodule("src/llvm-project", None);
}

let llvm_output = builder.ensure(crate::core::build_steps::llvm::Llvm { target });

let mut tarball = Tarball::new(builder, "llvm-tools", &target.triple);
tarball.set_overlay(OverlayKind::Llvm);
tarball.is_preview(true);
Expand All @@ -2738,7 +2730,7 @@ impl CommandLineStep for LlvmTools {
for tool in tools_to_install(&builder.paths) {
let exe = src_bindir.join(exe(tool, target));
// When using `download-ci-llvm`, some of the tools may not exist, so skip trying to copy them.
if !exe.exists() && builder.config.llvm_ci_mode.download_from_ci() {
if !exe.exists() && llvm_output.kind() == LlvmKind::DownloadedFromCi {
eprintln!("{} does not exist; skipping copy", exe.display());
continue;
}
Expand Down
51 changes: 37 additions & 14 deletions src/bootstrap/src/core/build_steps/llvm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::utils::helpers::{
/// Path where a file containing the link type (dynamic or static) is stored in the LLVM CI tarball.
pub const LLVM_CI_LINK_TYPE_PATH: &str = "link-type.txt";

#[derive(Copy, Clone)]
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum LlvmKind {
/// The LLVM was built from in-tree sources
BuiltLocally,
Expand Down Expand Up @@ -256,26 +256,45 @@ fn llvm_output_dir(builder: &Builder<'_>, target: TargetSelection) -> PathBuf {
}

fn try_download_ci_llvm(builder: &Builder<'_>, target: TargetSelection) -> Option<DownloadedLlvm> {
match builder.config.llvm_ci_mode {
LlvmCiMode::BuildLocally => return None,
LlvmCiMode::DownloadFromCi => {}
if builder.config.llvm_ci_mode.requests_download_from_ci()
&& let Some(config) = builder.config.target_config.get(&target)
{
if config.llvm_config.is_some() {
panic!(
"Cannot configure `llvm-config` for {target} when using `llvm.download-ci-llvm`",
);
}
if config.llvm_filecheck.is_some() {
panic!(
"Cannot configure `llvm-filecheck` for {target} when using `llvm.download-ci-llvm`"
);
}
}
Comment on lines +259 to +272

@jieyouxu jieyouxu Aug 29, 2026 •
edited
Loading

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.

Without per-target download-ci-llvm, with this logic here, we can't mix:

  • I want to download CI LLVM for host, where it is available
  • I want to provide an external LLVM for the target, where CI LLVM is unavailable

Previously, did we just use external LLVM for the target when host != target, when CI LLVM was available for host and requested? Or did this just never work?

This logic seems a bit fishy, won't in-tree default profiles (compiler/tools/libs) also hit this with if-changed if they try to cross-compile?

@Kobzol Kobzol Aug 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Previously this check was only done for the host, so the case you described worked. What about this:

  • We invert the priority, so that if you set llvm-config for a given target, it will take priority from download-ci-llvm, and we will use the external LLVM for that target before trying to download it from CI.
  • We remove the llvm-config error, and modify the filecheck error so that it simply errors our if you set filecheck, but don't set llvm-config (then it can be moved to config parsing).

@jieyouxu jieyouxu Aug 30, 2026

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.

This seems more intuitive to me yeah. 👍


// FIXME: this should eventually be relaxed
if target != builder.host_target {
crate::debug!("LLVM not available on CI for non-host target {target}");
return None;
match builder.config.llvm_ci_mode {
LlvmCiMode::BuildLocally => return None,
LlvmCiMode::Download => {}
LlvmCiMode::DownloadIfUnchanged => {
builder.config.update_submodule("src/llvm-project");

// Check for untracked changes in `src/llvm-project` and other important places.
let has_changes = builder.config.has_changes_from_upstream(LLVM_INVALIDATION_PATHS);
if has_changes {
builder.info("Warning: LLVM will not be downloaded because of local changes");
return None;
}
}
}

if !is_ci_llvm_available_for_target(&target, builder.config.llvm_assertions) {
crate::debug!(
"LLVM not available on CI for target={target} and assertions={}",
builder.info(&format!(
"Warning: LLVM not available on CI for target={target} and assertions={}",
builder.config.llvm_assertions
);
));
return None;
}

let ci_llvm = builder.config.maybe_download_host_ci_llvm()?;
let ci_llvm = builder.config.maybe_download_ci_llvm(target)?;
let link_shared = if !builder.config.dry_run() {
let link_type = t!(
std::fs::read_to_string(ci_llvm.join(LLVM_CI_LINK_TYPE_PATH)),
Expand All @@ -288,7 +307,7 @@ fn try_download_ci_llvm(builder: &Builder<'_>, target: TargetSelection) -> Optio

Some(DownloadedLlvm {
output: LlvmOutput {
llvm_config: ci_llvm.join("bin").join(exe("llvm-config", builder.host_target)),
llvm_config: ci_llvm.join("bin").join(exe("llvm-config", target)),
link_shared,
llvm_root_dir: ci_llvm,
kind: LlvmKind::DownloadedFromCi,
Expand Down Expand Up @@ -405,8 +424,12 @@ impl Step for LlvmFromCi {

fn run(self, builder: &Builder<'_>) -> Self::Output {
let llvm_ci = try_download_ci_llvm(builder, self.target)?;

// Sanity check
check_llvm_version(builder, llvm_ci.output.llvm_config());
if builder.host_target == self.target {
check_llvm_version(builder, llvm_ci.output.llvm_config());
}
Comment thread
Kobzol marked this conversation as resolved.

Some(llvm_ci)
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/bootstrap/src/core/builder/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use tracing::instrument;

pub(crate) use self::cargo::{Cargo, apply_pgo, cargo_profile_var};
use crate::core::build_steps::compile::{Std, StdLink, looks_like_codegen_backend};
use crate::core::build_steps::llvm::{LlvmKind, get_llvm_build_status};
use crate::core::build_steps::tool::RustcPrivateCompilers;
use crate::core::build_steps::{
check, clean, clippy, compile, dist, doc, gcc, install, llvm, run, setup, test, tool, vendor,
Expand Down Expand Up @@ -1399,7 +1400,10 @@ Alternatively, you can set `build.local-rebuild=true` and use a stage0 compiler
let mut dylib_dirs = vec![self.rustc_libdir(compiler)];

// Ensure that the downloaded LLVM libraries can be found.
if self.config.llvm_ci_mode.download_from_ci() {
// FIXME: the libraries should be added elsewhere, not in this function...
if get_llvm_build_status(self, compiler.host).llvm_output().kind()
== LlvmKind::DownloadedFromCi
{
let ci_llvm_lib = self.out.join(compiler.host).join("ci-llvm").join("lib");
dylib_dirs.push(ci_llvm_lib);
}
Expand Down
16 changes: 8 additions & 8 deletions src/bootstrap/src/core/builder/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use build_helper::stage0_parser::parse_stage0_file;
use llvm::get_llvm_build_status;

use super::*;
use crate::core::build_steps::llvm::LlvmKind;
use crate::core::config::Config;
use crate::utils::cache::ExecutedStep;
use crate::utils::helpers::get_host_target;
Expand Down Expand Up @@ -293,15 +294,14 @@ fn test_prebuilt_llvm_config_path_resolution() {
"#,
);

// CI-LLVM isn't always available; check if it's enabled before testing.
if config.llvm_ci_mode.download_from_ci() {
let sess = Session::new(config.clone());
let builder = Builder::new(&sess);
let sess = Session::new(config.clone());
let builder = Builder::new(&sess);

let actual = get_llvm_build_status(&builder, builder.config.host_target)
.llvm_output()
.llvm_config()
.to_path_buf();
let llvm = get_llvm_build_status(&builder, builder.config.host_target);
let llvm = llvm.llvm_output();
// CI-LLVM isn't always available; check if it's enabled before testing.
if llvm.kind() == LlvmKind::DownloadedFromCi {
let actual = llvm.llvm_config().to_path_buf();
let expected = builder
.out
.join(builder.config.host_target)
Expand Down
Loading
Loading