Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ impl<'a> Builder<'a> {
let mut dylib_dirs = vec![self.rustc_libdir(compiler)];

// Ensure that the downloaded LLVM libraries can be found.
if self.config.llvm.from_ci {
if self.config.llvm_from_ci {
let ci_llvm_lib = self.out.join(&*compiler.host.triple).join("ci-llvm").join("lib");
dylib_dirs.push(ci_llvm_lib);
}
Expand Down
45 changes: 29 additions & 16 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl Display for DebuginfoLevel {
}
}

#[derive(Default, Clone)]
#[derive(Default, Clone, PartialEq, serde_derive::Serialize, serde_derive::Deserialize)]
pub struct LLVMConfig {
pub assertions: bool,
pub tests: bool,
Expand All @@ -111,9 +111,6 @@ pub struct LLVMConfig {
pub release_debuginfo: bool,
pub static_stdcpp: bool,
/// `None` if `llvm_from_ci` is true and we haven't yet downloaded llvm.
#[cfg(not(test))]
link_shared: Cell<Option<bool>>,
#[cfg(test)]
pub link_shared: Cell<Option<bool>>,
pub clang_cl: Option<String>,
pub targets: Option<String>,
Expand All @@ -125,7 +122,6 @@ pub struct LLVMConfig {
pub polly: bool,
pub clang: bool,
pub enable_warnings: bool,
pub from_ci: bool,
pub build_config: HashMap<String, String>,

pub use_lld: bool,
Expand Down Expand Up @@ -204,6 +200,7 @@ pub struct Config {
pub backtrace_on_ice: bool,

// llvm codegen options
pub llvm_from_ci: bool,
pub llvm: LLVMConfig,

// rust codegen options
Expand Down Expand Up @@ -1519,18 +1516,26 @@ impl Config {
config.llvm.build_config = llvm.build_config.clone().unwrap_or(Default::default());

let asserts = llvm_assertions.unwrap_or(false);
config.llvm.from_ci = match llvm.download_ci_llvm {
Some(StringOrBool::String(s)) => {
assert!(s == "if-available", "unknown option `{}` for download-ci-llvm", s);
crate::llvm::is_ci_llvm_available(&config, asserts)
}
let mut downloaded_config = false;
config.llvm_from_ci = match llvm.download_ci_llvm {
Some(StringOrBool::String(s)) => match s.as_str() {
"if-available" => crate::llvm::is_ci_llvm_available(&config, asserts),
"if-identical" => match crate::llvm::get_llvm_opts_from_ci(&config) {
Some(config_ci) => {
downloaded_config = true;
is_llvm_config_identical(&config_ci, &config.llvm)
}
None => false,
},
_ => panic!("unknown option `{s}` for download-ci-llvm"),
},
Some(StringOrBool::Bool(b)) => b,
None => {
config.channel == "dev" && crate::llvm::is_ci_llvm_available(&config, asserts)
}
};

if config.llvm.from_ci {
if config.llvm_from_ci && !downloaded_config {
// None of the LLVM options, except assertions, are supported
// when using downloaded LLVM. We could just ignore these but
// that's potentially confusing, so force them to not be
Expand Down Expand Up @@ -1561,14 +1566,14 @@ impl Config {
}

// NOTE: can never be hit when downloading from CI, since we call `check_ci_llvm!(thin_lto)` above.
if config.llvm.thin_lto && llvm.link_shared.is_none() {
if !downloaded_config && config.llvm.thin_lto && llvm.link_shared.is_none() {
// If we're building with ThinLTO on, by default we want to link
// to LLVM shared, to avoid re-doing ThinLTO (which happens in
// the link step) with each stage.
config.llvm.link_shared.set(Some(true));
}
} else {
config.llvm.from_ci =
config.llvm_from_ci =
config.channel == "dev" && crate::llvm::is_ci_llvm_available(&config, false);
}

Expand Down Expand Up @@ -1620,7 +1625,7 @@ impl Config {
}
}

if config.llvm.from_ci {
if config.llvm_from_ci {
let triple = &config.build.triple;
let ci_llvm_bin = config.ci_llvm_root().join("bin");
let build_target = config
Expand Down Expand Up @@ -1860,10 +1865,14 @@ impl Config {

/// The absolute path to the downloaded LLVM artifacts.
pub(crate) fn ci_llvm_root(&self) -> PathBuf {
assert!(self.llvm.from_ci);
assert!(self.llvm_from_ci);
self.out.join(&*self.build.triple).join("ci-llvm")
}

pub(crate) fn ci_llvm_root_opts(&self) -> PathBuf {
self.out.join(&*self.build.triple).join("ci-llvm-opts")
}

/// Directory where the extracted `rustc-dev` component is stored.
pub(crate) fn ci_rustc_dir(&self) -> PathBuf {
assert!(self.download_rustc());
Expand All @@ -1882,7 +1891,7 @@ impl Config {
}

let llvm_link_shared = *opt.get_or_insert_with(|| {
if self.llvm.from_ci {
if self.llvm_from_ci {
self.maybe_download_ci_llvm();
let ci_llvm = self.ci_llvm_root();
let link_type = t!(
Expand Down Expand Up @@ -2089,6 +2098,10 @@ impl Config {
}
}

fn is_llvm_config_identical(from_ci: &LLVMConfig, current: &LLVMConfig) -> bool {
from_ci == current
}

fn set<T>(field: &mut T, val: Option<T>) {
if let Some(v) = val {
*field = v;
Expand Down
6 changes: 3 additions & 3 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1968,7 +1968,7 @@ fn install_llvm_file(builder: &Builder<'_>, source: &Path, destination: &Path) {
/// Returns whether the files were actually copied.
fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir: &Path) -> bool {
if let Some(config) = builder.config.target_config.get(&target) {
if config.llvm_config.is_some() && !builder.config.llvm.from_ci {
if config.llvm_config.is_some() && !builder.config.llvm_from_ci {
// If the LLVM was externally provided, then we don't currently copy
// artifacts into the sysroot. This is not necessarily the right
// choice (in particular, it will require the LLVM dylib to be in
Expand Down Expand Up @@ -2090,7 +2090,7 @@ impl Step for BoltInstrument {
return self.file.clone();
}

if builder.build.config.llvm.from_ci {
if builder.build.config.llvm_from_ci {
println!("warning: trying to use BOLT with LLVM from CI, this will probably not work");
}

Expand Down Expand Up @@ -2142,7 +2142,7 @@ impl Step for BoltOptimize {
return self.file.clone();
}

if builder.build.config.llvm.from_ci {
if builder.build.config.llvm_from_ci {
println!("warning: trying to use BOLT with LLVM from CI, this will probably not work");
}

Expand Down
31 changes: 30 additions & 1 deletion src/bootstrap/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ download-rustc = false
}

pub(crate) fn maybe_download_ci_llvm(&self) {
if !self.llvm.from_ci {
if !self.llvm_from_ci {
return;
}
let llvm_root = self.ci_llvm_root();
Expand Down Expand Up @@ -674,4 +674,33 @@ download-rustc = false
let llvm_root = self.ci_llvm_root();
self.unpack(&tarball, &llvm_root, "rust-dev");
}

pub(crate) fn download_ci_llvm_opts(&self, llvm_sha: &str) {
let cache_prefix = format!("llvm-opts-{llvm_sha}");
let cache_dst = self.out.join("cache");
let rustc_cache = cache_dst.join(cache_prefix);
t!(fs::create_dir_all(&rustc_cache));
let base = if self.llvm.assertions {
&self.stage0_metadata.config.artifacts_with_llvm_assertions_server
} else {
&self.stage0_metadata.config.artifacts_server
};
let version = self.artifact_version_part(llvm_sha);
let filename = format!("rust-dev-config-{}-{}.tar.xz", version, self.build.triple);
let tarball = rustc_cache.join(&filename);
if !tarball.exists() {
let help_on_error = "error: failed to download llvm config from ci

help: old builds get deleted after a certain time
help: if trying to compile an old commit of rustc, disable `download-ci-llvm` in config.toml:

[llvm]
download-ci-llvm = false
";
self.download_file(&format!("{base}/{llvm_sha}/{filename}"), &tarball, help_on_error);
}

let llvm_root = self.ci_llvm_root_opts();
self.unpack(&tarball, &llvm_root, "rust-dev-config");
Comment on lines +678 to +704
Copy link
Member

Choose a reason for hiding this comment

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

i have a vague "this is duplicating a lot of code" feeling but nothing concrete - if you find some easy way to reduce the boilerplate here that would be nice, but no worries if it's tricky

Copy link
Member Author

Choose a reason for hiding this comment

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

I still wonder if it wouldn't be simpler to just always download the LLVM archive (when using the if-identical mode) and embed the LLVM config directly inside of it. I think that the "hitrate" of this cache should be high enough (src/llvm-project doesn't change often, nor do build flags in dist builders), so downloading the archive everytime should be fine.

}
}
2 changes: 1 addition & 1 deletion src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ impl Build {
Some(Target { llvm_config, .. }) => {
// If the user set llvm-config we assume Rust is not patched,
// but first check to see if it was configured by llvm-from-ci.
(self.config.llvm.from_ci && target == self.config.build) || llvm_config.is_none()
(self.config.llvm_from_ci && target == self.config.build) || llvm_config.is_none()
}
None => true,
}
Expand Down
19 changes: 18 additions & 1 deletion src/bootstrap/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::process::Command;

use crate::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::channel;
use crate::config::{Config, TargetSelection};
use crate::config::{Config, LLVMConfig, TargetSelection};
use crate::util::get_clang_cl_resource_dir;
use crate::util::{self, exe, output, t, up_to_date};
use crate::{CLang, GitRepo, Kind};
Expand Down Expand Up @@ -215,6 +215,23 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool {
true
}

pub(crate) fn get_llvm_opts_from_ci(config: &Config) -> Option<LLVMConfig> {
if config.dry_run() || is_ci_llvm_modified(config) {
return None;
}

let llvm_sha = detect_llvm_sha(config, config.rust_info.is_managed_git_subrepository());
config.download_ci_llvm_opts(&llvm_sha);

let config_path = config.ci_llvm_root_opts().join("llvm-opts.json");
let Ok(config) = serde_json::from_slice::<LLVMConfig>(&t!(std::fs::read(config_path))) else {
// The LLVM config has changed its format or is corrupted in some way
eprintln!("Cannot deserialize LLVM config from llvm-opts.json");
return None;
Comment on lines +228 to +230
Copy link
Member

Choose a reason for hiding this comment

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

hmm, when would this happen? continuing the build when we can't parse the format seems unfortunate, i worry it'll silence a real bug.

Copy link
Member Author

Choose a reason for hiding this comment

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

For example, if the struct is modified in a backwards incompatible way (e.g. new option added/removed), then it may fail to parse. This situation necessarily means that the config has changed somehow, and therefore we shouldn't reuse the LLVM from CI.

In theory, we could lose all caching if there was some bug and it was never deserialized properly, but the alternative is basically failing the build if the format changes, which we probably shouldn't do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or we could devise some versioning scheme for the config (Protobuf? 😆 ), but that seems like overkill.

};
Some(config)
}

/// Returns true if we're running in CI with modified LLVM (and thus can't download it)
pub(crate) fn is_ci_llvm_modified(config: &Config) -> bool {
CiEnv::is_ci() && config.rust_info.is_managed_git_subrepository() && {
Expand Down