Skip to content

Disable combining LLD with external llvm-config #139853

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.

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

Merged
merged 3 commits into from
Apr 18, 2025
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
12 changes: 7 additions & 5 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl Step for Std {

// When using `download-rustc`, we already have artifacts for the host available. Don't
// recompile them.
if builder.download_rustc() && builder.is_builder_target(target)
if builder.download_rustc() && builder.config.is_host_target(target)
// NOTE: the beta compiler may generate different artifacts than the downloaded compiler, so
// its artifacts can't be reused.
&& compiler.stage != 0
Expand Down Expand Up @@ -229,7 +229,7 @@ impl Step for Std {
// The LLD wrappers and `rust-lld` are self-contained linking components that can be
// necessary to link the stdlib on some targets. We'll also need to copy these binaries to
// the `stage0-sysroot` to ensure the linker is found when bootstrapping on such a target.
if compiler.stage == 0 && builder.is_builder_target(compiler.host) {
if compiler.stage == 0 && builder.config.is_host_target(compiler.host) {
trace!(
"(build == host) copying linking components to `stage0-sysroot` for bootstrapping"
);
Expand Down Expand Up @@ -1374,7 +1374,7 @@ pub fn rustc_cargo_env(
/// Pass down configuration from the LLVM build into the build of
/// rustc_llvm and rustc_codegen_llvm.
fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) {
if builder.is_rust_llvm(target) {
if builder.config.is_rust_llvm(target) {
cargo.env("LLVM_RUSTLLVM", "1");
}
if builder.config.llvm_enzyme {
Expand Down Expand Up @@ -2182,7 +2182,7 @@ impl Step for Assemble {
debug!("copying codegen backends to sysroot");
copy_codegen_backends_to_sysroot(builder, build_compiler, target_compiler);

if builder.config.lld_enabled {
if builder.config.lld_enabled && !builder.config.is_system_llvm(target_compiler.host) {
builder.ensure(crate::core::build_steps::tool::LldWrapper {
build_compiler,
target_compiler,
Expand Down Expand Up @@ -2532,7 +2532,9 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path)
// FIXME: to make things simpler for now, limit this to the host and target where we know
// `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not
// cross-compiling. Expand this to other appropriate targets in the future.
if target != "x86_64-unknown-linux-gnu" || !builder.is_builder_target(target) || !path.exists()
if target != "x86_64-unknown-linux-gnu"
|| !builder.config.is_host_target(target)
|| !path.exists()
{
return;
}
Expand Down
9 changes: 5 additions & 4 deletions src/bootstrap/src/core/build_steps/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ impl Step for DebuggerScripts {
fn skip_host_target_lib(builder: &Builder<'_>, compiler: Compiler) -> bool {
// The only true set of target libraries came from the build triple, so
// let's reduce redundant work by only producing archives from that host.
if !builder.is_builder_target(compiler.host) {
if !builder.config.is_host_target(compiler.host) {
builder.info("\tskipping, not a build host");
true
} else {
Expand Down Expand Up @@ -671,7 +671,8 @@ fn copy_target_libs(
&self_contained_dst.join(path.file_name().unwrap()),
FileType::NativeLibrary,
);
} else if dependency_type == DependencyType::Target || builder.is_builder_target(target) {
} else if dependency_type == DependencyType::Target || builder.config.is_host_target(target)
{
builder.copy_link(&path, &dst.join(path.file_name().unwrap()), FileType::NativeLibrary);
}
}
Expand Down Expand Up @@ -824,7 +825,7 @@ impl Step for Analysis {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
if !builder.is_builder_target(compiler.host) {
if !builder.config.is_host_target(compiler.host) {
return None;
}

Expand Down Expand Up @@ -2118,7 +2119,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.
if builder.is_system_llvm(target) {
if builder.config.is_system_llvm(target) {
trace!("system LLVM requested, no install");
return false;
}
Expand Down
6 changes: 3 additions & 3 deletions src/bootstrap/src/core/build_steps/llvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ impl Step for Llvm {
}

// https://llvm.org/docs/HowToCrossCompileLLVM.html
if !builder.is_builder_target(target) {
if !builder.config.is_host_target(target) {
let LlvmResult { llvm_config, .. } =
builder.ensure(Llvm { target: builder.config.build });
if !builder.config.dry_run() {
Expand Down Expand Up @@ -637,7 +637,7 @@ fn configure_cmake(
}
cfg.target(&target.triple).host(&builder.config.build.triple);

if !builder.is_builder_target(target) {
if !builder.config.is_host_target(target) {
cfg.define("CMAKE_CROSSCOMPILING", "True");

// NOTE: Ideally, we wouldn't have to do this, and `cmake-rs` would just handle it for us.
Expand Down Expand Up @@ -1098,7 +1098,7 @@ impl Step for Lld {
.define("LLVM_CMAKE_DIR", llvm_cmake_dir)
.define("LLVM_INCLUDE_TESTS", "OFF");

if !builder.is_builder_target(target) {
if !builder.config.is_host_target(target) {
// Use the host llvm-tblgen binary.
cfg.define(
"LLVM_TABLEGEN_EXE",
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1894,7 +1894,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
.arg(llvm_components.trim());
llvm_components_passed = true;
}
if !builder.is_rust_llvm(target) {
if !builder.config.is_rust_llvm(target) {
cmd.arg("--system-llvm");
}

Expand Down Expand Up @@ -2668,7 +2668,7 @@ impl Step for Crate {
cargo
} else {
// Also prepare a sysroot for the target.
if !builder.is_builder_target(target) {
if !builder.config.is_host_target(target) {
builder.ensure(compile::Std::new(compiler, target).force_recompile(true));
builder.ensure(RemoteCopyLibs { compiler, target });
}
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/src/core/builder/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,8 +1107,8 @@ fn test_is_builder_target() {
let build = Build::new(config);
let builder = Builder::new(&build);

assert!(builder.is_builder_target(target1));
assert!(!builder.is_builder_target(target2));
assert!(builder.config.is_host_target(target1));
assert!(!builder.config.is_host_target(target2));
}
}

Expand Down
42 changes: 42 additions & 0 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2397,6 +2397,12 @@ impl Config {
);
}

if config.lld_enabled && config.is_system_llvm(config.build) {
eprintln!(
"Warning: LLD is enabled when using external llvm-config. LLD will not be built and copied to the sysroot."
);
}

let default_std_features = BTreeSet::from([String::from("panic-unwind")]);
config.rust_std_features = std_features.unwrap_or(default_std_features);

Expand Down Expand Up @@ -3233,6 +3239,42 @@ impl Config {

Some(commit.to_string())
}

/// Checks if the given target is the same as the host target.
pub fn is_host_target(&self, target: TargetSelection) -> bool {
self.build == target
}

/// Returns `true` if this is an external version of LLVM not managed by bootstrap.
/// In particular, we expect llvm sources to be available when this is false.
///
/// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set.
pub fn is_system_llvm(&self, target: TargetSelection) -> bool {
match self.target_config.get(&target) {
Some(Target { llvm_config: Some(_), .. }) => {
let ci_llvm = self.llvm_from_ci && self.is_host_target(target);
!ci_llvm
}
// We're building from the in-tree src/llvm-project sources.
Some(Target { llvm_config: None, .. }) => false,
None => false,
}
}

/// Returns `true` if this is our custom, patched, version of LLVM.
///
/// This does not necessarily imply that we're managing the `llvm-project` submodule.
pub fn is_rust_llvm(&self, target: TargetSelection) -> bool {
match self.target_config.get(&target) {
// We're using a user-controlled version of LLVM. The user has explicitly told us whether the version has our patches.
// (They might be wrong, but that's not a supported use-case.)
// In particular, this tries to support `submodules = false` and `patches = false`, for using a newer version of LLVM that's not through `rust-lang/llvm-project`.
Some(Target { llvm_has_rust_patches: Some(patched), .. }) => *patched,
// The user hasn't promised the patches match.
// This only has our patches if it's downloaded from CI or built from source.
_ => !self.is_system_llvm(target),
}
}
}

/// Compares the current `Llvm` options against those in the CI LLVM builder and detects any incompatible options.
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ than building it.
if target.contains("musl") && !target.contains("unikraft") {
// If this is a native target (host is also musl) and no musl-root is given,
// fall back to the system toolchain in /usr before giving up
if build.musl_root(*target).is_none() && build.is_builder_target(*target) {
if build.musl_root(*target).is_none() && build.config.is_host_target(*target) {
let target = build.config.target_config.entry(*target).or_default();
target.musl_root = Some("/usr".into());
}
Expand Down
42 changes: 3 additions & 39 deletions src/bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use utils::channel::GitInfo;

use crate::core::builder;
use crate::core::builder::Kind;
use crate::core::config::{DryRun, LldMode, LlvmLibunwind, Target, TargetSelection, flags};
use crate::core::config::{DryRun, LldMode, LlvmLibunwind, TargetSelection, flags};
use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode, command};
use crate::utils::helpers::{
self, dir_is_empty, exe, libdir, output, set_file_times, split_debuginfo, symlink_dir,
Expand Down Expand Up @@ -803,7 +803,7 @@ impl Build {
/// Note that if LLVM is configured externally then the directory returned
/// will likely be empty.
fn llvm_out(&self, target: TargetSelection) -> PathBuf {
if self.config.llvm_from_ci && self.is_builder_target(target) {
if self.config.llvm_from_ci && self.config.is_host_target(target) {
self.config.ci_llvm_root()
} else {
self.out.join(target).join("llvm")
Expand Down Expand Up @@ -851,37 +851,6 @@ impl Build {
if self.config.vendor { Some(self.src.join(VENDOR_DIR)) } else { None }
}

/// Returns `true` if this is an external version of LLVM not managed by bootstrap.
/// In particular, we expect llvm sources to be available when this is false.
///
/// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set.
fn is_system_llvm(&self, target: TargetSelection) -> bool {
match self.config.target_config.get(&target) {
Some(Target { llvm_config: Some(_), .. }) => {
let ci_llvm = self.config.llvm_from_ci && self.is_builder_target(target);
!ci_llvm
}
// We're building from the in-tree src/llvm-project sources.
Some(Target { llvm_config: None, .. }) => false,
None => false,
}
}

/// Returns `true` if this is our custom, patched, version of LLVM.
///
/// This does not necessarily imply that we're managing the `llvm-project` submodule.
fn is_rust_llvm(&self, target: TargetSelection) -> bool {
match self.config.target_config.get(&target) {
// We're using a user-controlled version of LLVM. The user has explicitly told us whether the version has our patches.
// (They might be wrong, but that's not a supported use-case.)
// In particular, this tries to support `submodules = false` and `patches = false`, for using a newer version of LLVM that's not through `rust-lang/llvm-project`.
Some(Target { llvm_has_rust_patches: Some(patched), .. }) => *patched,
// The user hasn't promised the patches match.
// This only has our patches if it's downloaded from CI or built from source.
_ => !self.is_system_llvm(target),
}
}

/// Returns the path to `FileCheck` binary for the specified target
fn llvm_filecheck(&self, target: TargetSelection) -> PathBuf {
let target_config = self.config.target_config.get(&target);
Expand Down Expand Up @@ -1356,7 +1325,7 @@ Executed at: {executed_at}"#,
// need to use CXX compiler as linker to resolve the exception functions
// that are only existed in CXX libraries
Some(self.cxx.borrow()[&target].path().into())
} else if !self.is_builder_target(target)
} else if !self.config.is_host_target(target)
&& helpers::use_host_linker(target)
&& !target.is_msvc()
{
Expand Down Expand Up @@ -2025,11 +1994,6 @@ to download LLVM rather than building it.
stream.reset().unwrap();
result
}

/// Checks if the given target is the same as the builder target.
fn is_builder_target(&self, target: TargetSelection) -> bool {
self.config.build == target
}
}

#[cfg(unix)]
Expand Down
Loading