From b3c9ffdc775b73cc9ab6bf36375a7ea941343b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 29 Sep 2023 18:16:17 +0200 Subject: [PATCH 1/9] Add `is_msvc` function --- src/bootstrap/src/core/build_steps/compile.rs | 6 +++--- src/bootstrap/src/core/build_steps/llvm.rs | 14 +++++++------- src/bootstrap/src/core/build_steps/test.rs | 4 ++-- src/bootstrap/src/core/build_steps/tool.rs | 2 +- src/bootstrap/src/core/builder.rs | 5 ++--- src/bootstrap/src/core/config/config.rs | 4 ++++ src/bootstrap/src/core/sanity.rs | 2 +- src/bootstrap/src/lib.rs | 8 ++++---- src/bootstrap/src/utils/cc_detect.rs | 4 ++-- 9 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 1ab488ef656d9..89abf976776c1 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -870,7 +870,7 @@ impl Step for Rustc { // is already on by default in MSVC optimized builds, which is interpreted as --icf=all: // https://github.com/llvm/llvm-project/blob/3329cec2f79185bafd678f310fafadba2a8c76d2/lld/COFF/Driver.cpp#L1746 // https://github.com/rust-lang/rust/blob/f22819bcce4abaff7d1246a56eec493418f9f4ee/compiler/rustc_codegen_ssa/src/back/linker.rs#L827 - if builder.config.use_lld && !compiler.host.contains("msvc") { + if builder.config.use_lld && !compiler.host.is_msvc() { cargo.rustflag("-Clink-args=-Wl,--icf=all"); } @@ -1089,7 +1089,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect // found. This is to avoid the linker errors about undefined references to // `__llvm_profile_instrument_memop` when linking `rustc_driver`. let mut llvm_linker_flags = String::new(); - if builder.config.llvm_profile_generate && target.contains("msvc") { + if builder.config.llvm_profile_generate && target.is_msvc() { if let Some(ref clang_cl_path) = builder.config.llvm_clang_cl { // Add clang's runtime library directory to the search path let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path); @@ -1114,7 +1114,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect // not for MSVC or macOS if builder.config.llvm_static_stdcpp && !target.contains("freebsd") - && !target.contains("msvc") + && !target.is_msvc() && !target.contains("apple") && !target.contains("solaris") { diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 7c119b0273b7c..8de208cd09c3c 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -98,7 +98,7 @@ pub fn prebuilt_llvm_config( let out_dir = builder.llvm_out(target); let mut llvm_config_ret_dir = builder.llvm_out(builder.config.build); - if !builder.config.build.contains("msvc") || builder.ninja() { + if !builder.config.build.is_msvc() || builder.ninja() { llvm_config_ret_dir.push("build"); } llvm_config_ret_dir.push("bin"); @@ -411,7 +411,7 @@ impl Step for Llvm { ldflags.shared.push(" -latomic"); } - if target.contains("msvc") { + if target.is_msvc() { cfg.define("LLVM_USE_CRT_DEBUG", "MT"); cfg.define("LLVM_USE_CRT_RELEASE", "MT"); cfg.define("LLVM_USE_CRT_RELWITHDEBINFO", "MT"); @@ -644,7 +644,7 @@ fn configure_cmake( } let sanitize_cc = |cc: &Path| { - if target.contains("msvc") { + if target.is_msvc() { OsString::from(cc.to_str().unwrap().replace("\\", "/")) } else { cc.as_os_str().to_owned() @@ -654,7 +654,7 @@ fn configure_cmake( // MSVC with CMake uses msbuild by default which doesn't respect these // vars that we'd otherwise configure. In that case we just skip this // entirely. - if target.contains("msvc") && !builder.ninja() { + if target.is_msvc() && !builder.ninja() { return; } @@ -664,7 +664,7 @@ fn configure_cmake( }; // Handle msvc + ninja + ccache specially (this is what the bots use) - if target.contains("msvc") && builder.ninja() && builder.config.ccache.is_some() { + if target.is_msvc() && builder.ninja() && builder.config.ccache.is_some() { let mut wrap_cc = env::current_exe().expect("failed to get cwd"); wrap_cc.set_file_name("sccache-plus-cl.exe"); @@ -768,7 +768,7 @@ fn configure_cmake( // For distribution we want the LLVM tools to be *statically* linked to libstdc++. // We also do this if the user explicitly requested static libstdc++. if builder.config.llvm_static_stdcpp - && !target.contains("msvc") + && !target.is_msvc() && !target.contains("netbsd") && !target.contains("solaris") { @@ -874,7 +874,7 @@ impl Step for Lld { // when doing PGO on CI, cmake or clang-cl don't automatically link clang's // profiler runtime in. In that case, we need to manually ask cmake to do it, to avoid // linking errors, much like LLVM's cmake setup does in that situation. - if builder.config.llvm_profile_generate && target.contains("msvc") { + if builder.config.llvm_profile_generate && target.is_msvc() { if let Some(clang_cl_path) = builder.config.llvm_clang_cl.as_ref() { // Find clang's runtime library directory and push that as a search path to the // cmake linker flags. diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 77a4f2c4cb201..d36fcddde0d64 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1931,7 +1931,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // // Note that if we encounter `PATH` we make sure to append to our own `PATH` // rather than stomp over it. - if !builder.config.dry_run() && target.contains("msvc") { + if !builder.config.dry_run() && target.is_msvc() { for &(ref k, ref v) in builder.cc.borrow()[&target].env() { if k != "PATH" { cmd.env(k, v); @@ -3070,7 +3070,7 @@ impl Step for TestHelpers { // We may have found various cross-compilers a little differently due to our // extra configuration, so inform cc of these compilers. Note, though, that // on MSVC we still need cc's detection of env vars (ugh). - if !target.contains("msvc") { + if !target.is_msvc() { if let Some(ar) = builder.ar(target) { cfg.archiver(ar); } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 37cd96e7c63ec..9942f00a0562b 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -833,7 +833,7 @@ impl<'a> Builder<'a> { // On MSVC a tool may invoke a C compiler (e.g., compiletest in run-make // mode) and that C compiler may need some extra PATH modification. Do // so here. - if compiler.host.contains("msvc") { + if compiler.host.is_msvc() { let curpaths = env::var_os("PATH").unwrap_or_default(); let curpaths = env::split_paths(&curpaths).collect::>(); for &(ref k, ref v) in self.cc.borrow()[&compiler.host].env() { diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index d018be5f8e1d7..28bfe2949a05a 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1729,8 +1729,7 @@ impl<'a> Builder<'a> { let split_debuginfo_is_stable = target.contains("linux") || target.contains("apple") - || (target.contains("msvc") - && self.config.rust_split_debuginfo == SplitDebuginfo::Packed) + || (target.is_msvc() && self.config.rust_split_debuginfo == SplitDebuginfo::Packed) || (target.contains("windows") && self.config.rust_split_debuginfo == SplitDebuginfo::Off); @@ -1909,7 +1908,7 @@ impl<'a> Builder<'a> { // the options through environment variables that are fetched and understood by both. // // FIXME: the guard against msvc shouldn't need to be here - if target.contains("msvc") { + if target.is_msvc() { if let Some(ref cl) = self.config.llvm_clang_cl { cargo.env("CC", cl).env("CXX", cl); } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index b5291817e7920..23a9b6a2f1189 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -489,6 +489,10 @@ impl TargetSelection { pub fn is_synthetic(&self) -> bool { self.synthetic } + + pub fn is_msvc(&self) -> bool { + self.contains("msvc") + } } impl fmt::Display for TargetSelection { diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index eec3be66a1281..9101d94ea881e 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -237,7 +237,7 @@ than building it. } } - if need_cmake && target.contains("msvc") { + if need_cmake && target.is_msvc() { // There are three builds of cmake on windows: MSVC, MinGW, and // Cygwin. The Cygwin build does not have generators for Visual // Studio, so detect that here and error. diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 13391b1faa193..e0268fdbc882f 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -862,7 +862,7 @@ impl Build { } } else { let base = self.llvm_out(target).join("build"); - let base = if !self.ninja() && target.contains("msvc") { + let base = if !self.ninja() && target.is_msvc() { if self.config.llvm_optimize { if self.config.llvm_release_debuginfo { base.join("RelWithDebInfo") @@ -1255,7 +1255,7 @@ impl Build { Some(self.cxx.borrow()[&target].path().into()) } else if target != self.config.build && helpers::use_host_linker(target) - && !target.contains("msvc") + && !target.is_msvc() { Some(self.cc(target)) } else if self.config.use_lld && !self.is_fuse_ld_lld(target) && self.build == target { @@ -1268,7 +1268,7 @@ impl Build { // LLD is used through `-fuse-ld=lld` rather than directly. // Only MSVC targets use LLD directly at the moment. fn is_fuse_ld_lld(&self, target: TargetSelection) -> bool { - self.config.use_lld && !target.contains("msvc") + self.config.use_lld && !target.is_msvc() } fn lld_flags(&self, target: TargetSelection) -> impl Iterator { @@ -1764,7 +1764,7 @@ to download LLVM rather than building it. // In these cases we automatically enable Ninja if we find it in the // environment. if !self.config.ninja_in_file - && self.config.build.contains("msvc") + && self.config.build.is_msvc() && cmd_finder.maybe_have("ninja").is_some() { return true; diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 52b36ce75f343..fb5b9d8c88f7d 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -39,7 +39,7 @@ fn cc2ar(cc: &Path, target: TargetSelection) -> Option { Some(PathBuf::from(ar)) } else if let Some(ar) = env::var_os("AR") { Some(PathBuf::from(ar)) - } else if target.contains("msvc") { + } else if target.is_msvc() { None } else if target.contains("musl") { Some(PathBuf::from("ar")) @@ -78,7 +78,7 @@ fn new_cc_build(build: &Build, target: TargetSelection) -> cc::Build { cfg.static_crt(a); } None => { - if target.contains("msvc") { + if target.is_msvc() { cfg.static_crt(true); } if target.contains("musl") { From d9f9e67bc19002d553bc3ca294c24c000d29f4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 2 Dec 2023 23:21:23 +0100 Subject: [PATCH 2/9] Refactor rust(do)c linker flags --- src/bootstrap/src/core/build_steps/test.rs | 14 ++--- src/bootstrap/src/core/builder.rs | 25 ++++----- src/bootstrap/src/lib.rs | 15 ------ src/bootstrap/src/utils/helpers.rs | 62 +++++++++++----------- 4 files changed, 51 insertions(+), 65 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index d36fcddde0d64..2184dcbf58989 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -29,8 +29,8 @@ use crate::utils; use crate::utils::cache::{Interned, INTERNER}; use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{ - self, add_link_lib_path, add_rustdoc_cargo_lld_flags, add_rustdoc_lld_flags, dylib_path, - dylib_path_var, output, t, target_supports_cranelift_backend, up_to_date, LldThreads, + self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var, + linker_args, output, t, target_supports_cranelift_backend, up_to_date, LldThreads, }; use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests}; use crate::{envify, CLang, DocTests, GitRepo, Mode}; @@ -277,7 +277,7 @@ impl Step for Cargotest { .args(builder.config.test_args()) .env("RUSTC", builder.rustc(compiler)) .env("RUSTDOC", builder.rustdoc(compiler)); - add_rustdoc_cargo_lld_flags(&mut cmd, builder, compiler.host, LldThreads::No); + add_rustdoc_cargo_linker_args(&mut cmd, builder, compiler.host, LldThreads::No); builder.run_delaying_failure(cmd); } } @@ -863,7 +863,7 @@ impl Step for RustdocTheme { .env("CFG_RELEASE_CHANNEL", &builder.config.channel) .env("RUSTDOC_REAL", builder.rustdoc(self.compiler)) .env("RUSTC_BOOTSTRAP", "1"); - add_rustdoc_lld_flags(&mut cmd, builder, self.compiler.host, LldThreads::No); + cmd.args(linker_args(builder, self.compiler.host, LldThreads::No)); builder.run_delaying_failure(&mut cmd); } @@ -1039,7 +1039,7 @@ impl Step for RustdocGUI { cmd.env("RUSTDOC", builder.rustdoc(self.compiler)) .env("RUSTC", builder.rustc(self.compiler)); - add_rustdoc_cargo_lld_flags(&mut cmd, builder, self.compiler.host, LldThreads::No); + add_rustdoc_cargo_linker_args(&mut cmd, builder, self.compiler.host, LldThreads::No); for path in &builder.paths { if let Some(p) = helpers::is_valid_test_suite_arg(path, "tests/rustdoc-gui", builder) { @@ -1746,14 +1746,14 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let mut hostflags = flags.clone(); hostflags.push(format!("-Lnative={}", builder.test_helpers_out(compiler.host).display())); - hostflags.extend(builder.lld_flags(compiler.host)); + hostflags.extend(linker_args(builder, compiler.host, LldThreads::No)); for flag in hostflags { cmd.arg("--host-rustcflags").arg(flag); } let mut targetflags = flags; targetflags.push(format!("-Lnative={}", builder.test_helpers_out(target).display())); - targetflags.extend(builder.lld_flags(target)); + targetflags.extend(linker_args(builder, compiler.host, LldThreads::No)); for flag in targetflags { cmd.arg("--target-rustcflags").arg(flag); } diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 28bfe2949a05a..75c06dd5510ac 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -19,7 +19,9 @@ use crate::core::build_steps::{check, clean, compile, dist, doc, install, run, s use crate::core::config::flags::{Color, Subcommand}; use crate::core::config::{DryRun, SplitDebuginfo, TargetSelection}; use crate::utils::cache::{Cache, Interned, INTERNER}; -use crate::utils::helpers::{self, add_dylib_path, add_link_lib_path, add_rustdoc_lld_flags, exe}; +use crate::utils::helpers::{ + self, add_dylib_path, add_link_lib_path, exe, linker_args, linker_flags, +}; use crate::utils::helpers::{libdir, output, t, LldThreads}; use crate::Crate; use crate::EXTRA_CHECK_CFGS; @@ -1174,7 +1176,7 @@ impl<'a> Builder<'a> { cmd.env_remove("MAKEFLAGS"); cmd.env_remove("MFLAGS"); - add_rustdoc_lld_flags(&mut cmd, self, compiler.host, LldThreads::Yes); + cmd.args(linker_args(self, compiler.host, LldThreads::Yes)); cmd } @@ -1673,21 +1675,20 @@ impl<'a> Builder<'a> { rustflags.arg(&format!("-Zstack-protector={stack_protector}")); } - if let Some(host_linker) = self.linker(compiler.host) { - hostflags.arg(format!("-Clinker={}", host_linker.display())); - } - if self.is_fuse_ld_lld(compiler.host) { - hostflags.arg("-Clink-args=-fuse-ld=lld"); - } + linker_args(self, compiler.host, LldThreads::Yes).into_iter().for_each(|flag| { + hostflags.arg(flag); + }); if let Some(target_linker) = self.linker(target) { let target = crate::envify(&target.triple); cargo.env(&format!("CARGO_TARGET_{target}_LINKER"), target_linker); } - if self.is_fuse_ld_lld(target) { - rustflags.arg("-Clink-args=-fuse-ld=lld"); - } - self.lld_flags(target).for_each(|flag| { + // We want to set -Clinker using Cargo, therefore we only call `linker_flags` and not + // `linker_args` here. + linker_flags(self, target, LldThreads::Yes).into_iter().for_each(|flag| { + rustflags.arg(&flag); + }); + linker_args(self, target, LldThreads::Yes).into_iter().for_each(|flag| { rustdocflags.arg(&flag); }); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index e0268fdbc882f..5442ce97a8d47 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1271,21 +1271,6 @@ impl Build { self.config.use_lld && !target.is_msvc() } - fn lld_flags(&self, target: TargetSelection) -> impl Iterator { - let mut options = [None, None]; - - if self.config.use_lld { - if self.is_fuse_ld_lld(target) { - options[0] = Some("-Clink-arg=-fuse-ld=lld".to_string()); - } - - let no_threads = helpers::lld_flag_no_threads(target.contains("windows")); - options[1] = Some(format!("-Clink-arg=-Wl,{no_threads}")); - } - - IntoIterator::into_iter(options).flatten() - } - /// Returns if this target should statically link the C runtime, if specified fn crt_static(&self, target: TargetSelection) -> Option { if target.contains("pc-windows-msvc") { diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index f878d6347435e..eeed9c8743802 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -5,7 +5,7 @@ use build_helper::util::fail; use std::env; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsStr; use std::fs; use std::io; use std::path::{Path, PathBuf}; @@ -476,22 +476,46 @@ pub enum LldThreads { No, } -pub fn add_rustdoc_lld_flags( - cmd: &mut Command, +/// Returns the linker arguments for rustc/rustdoc for the given builder and target. +pub fn linker_args( builder: &Builder<'_>, target: TargetSelection, lld_threads: LldThreads, -) { - cmd.args(build_rustdoc_lld_flags(builder, target, lld_threads)); +) -> Vec { + let mut args = linker_flags(builder, target, lld_threads); + + if let Some(linker) = builder.linker(target) { + args.push(format!("-Clinker={}", linker.display())); + } + + args } -pub fn add_rustdoc_cargo_lld_flags( +/// Returns the linker arguments for rustc/rustdoc for the given builder and target, without the +/// -Clinker flag. +pub fn linker_flags( + builder: &Builder<'_>, + target: TargetSelection, + lld_threads: LldThreads, +) -> Vec { + let mut args = vec![]; + if builder.is_fuse_ld_lld(target) { + args.push(String::from("-Clink-arg=-fuse-ld=lld")); + + if matches!(lld_threads, LldThreads::No) { + args.push(format!("-Clink-arg=-Wl,{}", lld_flag_no_threads(target.is_msvc()))); + } + } + args +} + +pub fn add_rustdoc_cargo_linker_args( cmd: &mut Command, builder: &Builder<'_>, target: TargetSelection, lld_threads: LldThreads, ) { - let args = build_rustdoc_lld_flags(builder, target, lld_threads); + let args = linker_args(builder, target, lld_threads); let mut flags = cmd .get_envs() .find_map(|(k, v)| if k == OsStr::new("RUSTDOCFLAGS") { v } else { None }) @@ -507,27 +531,3 @@ pub fn add_rustdoc_cargo_lld_flags( cmd.env("RUSTDOCFLAGS", flags); } } - -fn build_rustdoc_lld_flags( - builder: &Builder<'_>, - target: TargetSelection, - lld_threads: LldThreads, -) -> Vec { - let mut args = vec![]; - - if let Some(linker) = builder.linker(target) { - let mut flag = std::ffi::OsString::from("-Clinker="); - flag.push(linker); - args.push(flag); - } - if builder.is_fuse_ld_lld(target) { - args.push(OsString::from("-Clink-arg=-fuse-ld=lld")); - if matches!(lld_threads, LldThreads::No) { - args.push(OsString::from(format!( - "-Clink-arg=-Wl,{}", - lld_flag_no_threads(target.contains("windows")) - ))); - } - } - args -} From 48c1607bc6742f964d291cd1fb6c3284f0d5a61d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 29 Sep 2023 18:08:07 +0200 Subject: [PATCH 3/9] Introduce `LldMode` and generalize parsing of `use-lld` --- src/bootstrap/src/core/build_steps/compile.rs | 2 +- src/bootstrap/src/core/config/config.rs | 77 ++++++++++++++++++- src/bootstrap/src/lib.rs | 19 +++-- src/bootstrap/src/tests/config.rs | 11 ++- src/bootstrap/src/utils/helpers.rs | 2 +- 5 files changed, 99 insertions(+), 12 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 89abf976776c1..11e789a996af0 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -870,7 +870,7 @@ impl Step for Rustc { // is already on by default in MSVC optimized builds, which is interpreted as --icf=all: // https://github.com/llvm/llvm-project/blob/3329cec2f79185bafd678f310fafadba2a8c76d2/lld/COFF/Driver.cpp#L1746 // https://github.com/rust-lang/rust/blob/f22819bcce4abaff7d1246a56eec493418f9f4ee/compiler/rustc_codegen_ssa/src/back/linker.rs#L827 - if builder.config.use_lld && !compiler.host.is_msvc() { + if builder.config.lld_mode.is_used() && !compiler.host.is_msvc() { cargo.rustflag("-Clink-args=-Wl,--icf=all"); } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 23a9b6a2f1189..ade7d83f8b2d2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -105,6 +105,39 @@ impl Display for DebuginfoLevel { } } +/// LLD in bootstrap works like this: +/// - Self-contained lld: use `rust-lld` from the compiler's sysroot +/// - External: use an external `lld` binary +/// +/// It is configured depending on the target: +/// 1) Everything except MSVC +/// - Self-contained: -Clinker-flavor=gnu-lld-cc -Clink-self-contained=+linker +/// - External: -Clinker-flavor=gnu-lld-cc +/// 2) MSVC +/// - Self-contained: -Clinker= +/// - External: -Clinker=lld +#[derive(Default, Clone)] +pub enum LldMode { + /// Do not use LLD + #[default] + Unused, + /// Use `rust-lld` from the compiler's sysroot + SelfContained, + /// Use an externally provided `lld` binary. + /// Note that the linker name cannot be overridden, the binary has to be named `lld` and it has + /// to be in $PATH. + External, +} + +impl LldMode { + pub fn is_used(&self) -> bool { + match self { + LldMode::SelfContained | LldMode::External => true, + LldMode::Unused => false, + } + } +} + /// Global configuration for the entire build and/or bootstrap. /// /// This structure is parsed from `config.toml`, and some of the fields are inferred from `git` or build-time parameters. @@ -199,7 +232,7 @@ pub struct Config { pub llvm_from_ci: bool, pub llvm_build_config: HashMap, - pub use_lld: bool, + pub lld_mode: LldMode, pub lld_enabled: bool, pub llvm_tools_enabled: bool, @@ -981,6 +1014,44 @@ enum StringOrInt<'a> { String(&'a str), Int(i64), } + +impl<'de> Deserialize<'de> for LldMode { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct LldModeVisitor; + + impl<'de> serde::de::Visitor<'de> for LldModeVisitor { + type Value = LldMode; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("one of true, 'self-contained' or 'external'") + } + + fn visit_bool(self, v: bool) -> Result + where + E: serde::de::Error, + { + Ok(if v { LldMode::External } else { LldMode::Unused }) + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + match v { + "external" => Ok(LldMode::External), + "self-contained" => Ok(LldMode::SelfContained), + _ => Err(E::custom("unknown mode {v}")), + } + } + } + + deserializer.deserialize_any(LldModeVisitor) + } +} + define_config! { /// TOML representation of how the Rust build is configured. struct Rust { @@ -1018,7 +1089,7 @@ define_config! { save_toolstates: Option = "save-toolstates", codegen_backends: Option> = "codegen-backends", lld: Option = "lld", - use_lld: Option = "use-lld", + lld_mode: Option = "use-lld", llvm_tools: Option = "llvm-tools", deny_warnings: Option = "deny-warnings", backtrace_on_ice: Option = "backtrace-on-ice", @@ -1446,7 +1517,7 @@ impl Config { if let Some(true) = rust.incremental { config.incremental = true; } - set(&mut config.use_lld, rust.use_lld); + set(&mut config.lld_mode, rust.lld_mode); set(&mut config.lld_enabled, rust.lld); set(&mut config.llvm_tools_enabled, rust.llvm_tools); config.rustc_parallel = rust diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 5442ce97a8d47..fd0b61eae8d7c 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -37,7 +37,7 @@ use utils::channel::GitInfo; use crate::core::builder; use crate::core::builder::Kind; -use crate::core::config::flags; +use crate::core::config::{flags, LldMode}; use crate::core::config::{DryRun, Target}; use crate::core::config::{LlvmLibunwind, TargetSelection}; use crate::utils::cache::{Interned, INTERNER}; @@ -1258,17 +1258,24 @@ impl Build { && !target.is_msvc() { Some(self.cc(target)) - } else if self.config.use_lld && !self.is_fuse_ld_lld(target) && self.build == target { - Some(self.initial_lld.clone()) + } else if self.config.lld_mode.is_used() + && self.is_lld_direct_linker(target) + && self.build == target + { + match self.config.lld_mode { + LldMode::SelfContained => Some(self.initial_lld.clone()), + LldMode::External => Some("lld".into()), + LldMode::Unused => None, + } } else { None } } - // LLD is used through `-fuse-ld=lld` rather than directly. + // Is LLD configured directly through `-Clinker`? // Only MSVC targets use LLD directly at the moment. - fn is_fuse_ld_lld(&self, target: TargetSelection) -> bool { - self.config.use_lld && !target.is_msvc() + fn is_lld_direct_linker(&self, target: TargetSelection) -> bool { + target.is_msvc() } /// Returns if this target should statically link the C runtime, if specified diff --git a/src/bootstrap/src/tests/config.rs b/src/bootstrap/src/tests/config.rs index a8106ca8ff923..c24d57fb8f82c 100644 --- a/src/bootstrap/src/tests/config.rs +++ b/src/bootstrap/src/tests/config.rs @@ -1,5 +1,5 @@ -use crate::core::config::TomlConfig; use super::{Config, Flags}; +use crate::core::config::{LldMode, TomlConfig}; use clap::CommandFactory; use serde::Deserialize; @@ -217,3 +217,12 @@ fn verify_file_integrity() { remove_file(tempfile).unwrap(); } + +#[test] +fn rust_lld() { + assert!(matches!(parse("").lld_mode, LldMode::Unused)); + assert!(matches!(parse("rust.use-lld = \"self-contained\"").lld_mode, LldMode::SelfContained)); + assert!(matches!(parse("rust.use-lld = \"external\"").lld_mode, LldMode::External)); + assert!(matches!(parse("rust.use-lld = true").lld_mode, LldMode::External)); + assert!(matches!(parse("rust.use-lld = false").lld_mode, LldMode::Unused)); +} diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index eeed9c8743802..2da556f0a2de4 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -499,7 +499,7 @@ pub fn linker_flags( lld_threads: LldThreads, ) -> Vec { let mut args = vec![]; - if builder.is_fuse_ld_lld(target) { + if !builder.is_lld_direct_linker(target) && builder.config.lld_mode.is_used() { args.push(String::from("-Clink-arg=-fuse-ld=lld")); if matches!(lld_threads, LldThreads::No) { From 40c3d351ada5d991f7474ecc4bc6c9b976c656cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 2 Dec 2023 23:49:01 +0100 Subject: [PATCH 4/9] Use MCP510 --- src/bootstrap/src/utils/helpers.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 2da556f0a2de4..b2aa37d069177 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -15,7 +15,7 @@ use std::sync::OnceLock; use std::time::{Instant, SystemTime, UNIX_EPOCH}; use crate::core::builder::Builder; -use crate::core::config::{Config, TargetSelection}; +use crate::core::config::{Config, LldMode, TargetSelection}; pub use crate::utils::dylib::{dylib_path, dylib_path_var}; @@ -500,7 +500,16 @@ pub fn linker_flags( ) -> Vec { let mut args = vec![]; if !builder.is_lld_direct_linker(target) && builder.config.lld_mode.is_used() { - args.push(String::from("-Clink-arg=-fuse-ld=lld")); + match builder.config.lld_mode { + LldMode::External => { + args.push("-Clinker-flavor=gnu-lld-cc".to_string()); + } + LldMode::SelfContained => { + args.push("-Clinker-flavor=gnu-lld-cc".to_string()); + args.push("-Clink-self-contained=+linker".to_string()); + } + LldMode::Unused => {} + } if matches!(lld_threads, LldThreads::No) { args.push(format!("-Clink-arg=-Wl,{}", lld_flag_no_threads(target.is_msvc()))); From 50865745e1aa3b6af6fe98d121ba68516fcce669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 2 Dec 2023 23:51:22 +0100 Subject: [PATCH 5/9] Update `config.example.toml` --- config.example.toml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/config.example.toml b/config.example.toml index 49c4ad4c958ed..b103e9890d20d 100644 --- a/config.example.toml +++ b/config.example.toml @@ -648,10 +648,12 @@ change-id = 117813 #lld = false # Indicates whether LLD will be used to link Rust crates during bootstrap on -# supported platforms. The LLD from the bootstrap distribution will be used -# and not the LLD compiled during the bootstrap. +# supported platforms. +# If set to `true` or `"external"`, a global `lld` binary that has to be in $PATH +# will be used. +# If set to `"self-contained"`, rust-lld from the snapshot compiler will be used. # -# LLD will not be used if we're cross linking. +# On MSVC, LLD will not be used if we're cross linking. # # Explicitly setting the linker for a target will override this option when targeting MSVC. #use-lld = false From cbfe6327a1e1d4abc4df9c3394b0b96c8a9c95e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 3 Dec 2023 10:23:54 +0100 Subject: [PATCH 6/9] Do not invoke external lld to figure out thread flags in self-contained mode --- src/bootstrap/src/utils/helpers.rs | 31 ++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index b2aa37d069177..8a818fb87ffde 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -443,17 +443,29 @@ pub fn get_clang_cl_resource_dir(clang_cl_path: &str) -> PathBuf { clang_rt_dir.to_path_buf() } -pub fn lld_flag_no_threads(is_windows: bool) -> &'static str { +/// Returns a flag that configures LLD to use only a single thread. +/// If we use an external LLD, we need to find out which version is it to know which flag should we +/// pass to it (LLD older than version 10 had a different flag). +fn lld_flag_no_threads(lld_mode: LldMode, is_windows: bool) -> &'static str { static LLD_NO_THREADS: OnceLock<(&'static str, &'static str)> = OnceLock::new(); - let (windows, other) = LLD_NO_THREADS.get_or_init(|| { - let out = output(Command::new("lld").arg("-flavor").arg("ld").arg("--version")); - let newer = match (out.find(char::is_numeric), out.find('.')) { - (Some(b), Some(e)) => out.as_str()[b..e].parse::().ok().unwrap_or(14) > 10, + + let new_flags = ("/threads:1", "--threads=1"); + let old_flags = ("/no-threads", "--no-threads"); + + let (windows_flag, other_flag) = LLD_NO_THREADS.get_or_init(|| { + let newer_version = match lld_mode { + LldMode::External => { + let out = output(Command::new("lld").arg("-flavor").arg("ld").arg("--version")); + match (out.find(char::is_numeric), out.find('.')) { + (Some(b), Some(e)) => out.as_str()[b..e].parse::().ok().unwrap_or(14) > 10, + _ => true, + } + } _ => true, }; - if newer { ("/threads:1", "--threads=1") } else { ("/no-threads", "--no-threads") } + if newer_version { new_flags } else { old_flags } }); - if is_windows { windows } else { other } + if is_windows { windows_flag } else { other_flag } } pub fn dir_is_empty(dir: &Path) -> bool { @@ -512,7 +524,10 @@ pub fn linker_flags( } if matches!(lld_threads, LldThreads::No) { - args.push(format!("-Clink-arg=-Wl,{}", lld_flag_no_threads(target.is_msvc()))); + args.push(format!( + "-Clink-arg=-Wl,{}", + lld_flag_no_threads(builder.config.lld_mode.clone(), target.is_msvc()) + )); } } args From ea769dbeb7d85e657148445447e1acaf96fe654d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 3 Dec 2023 10:29:30 +0100 Subject: [PATCH 7/9] Add change tracker entry --- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 887b596b7868a..af3b238dfc36a 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -86,4 +86,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Info, summary: "Use of the `if-available` value for `download-ci-llvm` is deprecated; prefer using the new `if-unchanged` value.", }, + ChangeInfo { + change_id: 116278, + severity: ChangeSeverity::Info, + summary: "The `rust.use-lld` configuration now has different options ('external'/true or 'self-contained'), and its behaviour has changed.", + }, ]; From 4750e9de4790672481f6709af015efd17dbf18f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 7 Dec 2023 12:34:54 +0100 Subject: [PATCH 8/9] Produce an explicit error when using a self-contained lld, but we don't add it to sysroot --- src/bootstrap/src/core/config/config.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index ade7d83f8b2d2..105b7d74dfc1e 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1519,6 +1519,16 @@ impl Config { } set(&mut config.lld_mode, rust.lld_mode); set(&mut config.lld_enabled, rust.lld); + + if matches!(config.lld_mode, LldMode::SelfContained) + && !config.lld_enabled + && flags.stage.unwrap_or(0) > 0 + { + panic!( + "Trying to use self-contained lld as a linker, but LLD is not being added to the sysroot. Enable it with rust.lld = true." + ); + } + set(&mut config.llvm_tools_enabled, rust.llvm_tools); config.rustc_parallel = rust .parallel_compiler From 53031b264ed51f1e828ddb91f6614f61840f6b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 8 Dec 2023 17:32:56 +0100 Subject: [PATCH 9/9] Review fixes --- src/bootstrap/src/core/build_steps/test.rs | 7 ++++--- src/bootstrap/src/core/builder.rs | 22 ++++++++++------------ src/bootstrap/src/core/config/config.rs | 10 +++++----- src/bootstrap/src/utils/helpers.rs | 3 ++- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 2184dcbf58989..d0f36f99342fe 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -30,7 +30,8 @@ use crate::utils::cache::{Interned, INTERNER}; use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{ self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var, - linker_args, output, t, target_supports_cranelift_backend, up_to_date, LldThreads, + linker_args, linker_flags, output, t, target_supports_cranelift_backend, up_to_date, + LldThreads, }; use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests}; use crate::{envify, CLang, DocTests, GitRepo, Mode}; @@ -1746,14 +1747,14 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let mut hostflags = flags.clone(); hostflags.push(format!("-Lnative={}", builder.test_helpers_out(compiler.host).display())); - hostflags.extend(linker_args(builder, compiler.host, LldThreads::No)); + hostflags.extend(linker_flags(builder, compiler.host, LldThreads::No)); for flag in hostflags { cmd.arg("--host-rustcflags").arg(flag); } let mut targetflags = flags; targetflags.push(format!("-Lnative={}", builder.test_helpers_out(target).display())); - targetflags.extend(linker_args(builder, compiler.host, LldThreads::No)); + targetflags.extend(linker_flags(builder, compiler.host, LldThreads::No)); for flag in targetflags { cmd.arg("--target-rustcflags").arg(flag); } diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 75c06dd5510ac..2356565190bdc 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -19,10 +19,8 @@ use crate::core::build_steps::{check, clean, compile, dist, doc, install, run, s use crate::core::config::flags::{Color, Subcommand}; use crate::core::config::{DryRun, SplitDebuginfo, TargetSelection}; use crate::utils::cache::{Cache, Interned, INTERNER}; -use crate::utils::helpers::{ - self, add_dylib_path, add_link_lib_path, exe, linker_args, linker_flags, -}; -use crate::utils::helpers::{libdir, output, t, LldThreads}; +use crate::utils::helpers::{self, add_dylib_path, add_link_lib_path, exe, linker_args}; +use crate::utils::helpers::{libdir, linker_flags, output, t, LldThreads}; use crate::Crate; use crate::EXTRA_CHECK_CFGS; use crate::{Build, CLang, DocTests, GitRepo, Mode}; @@ -1675,9 +1673,9 @@ impl<'a> Builder<'a> { rustflags.arg(&format!("-Zstack-protector={stack_protector}")); } - linker_args(self, compiler.host, LldThreads::Yes).into_iter().for_each(|flag| { - hostflags.arg(flag); - }); + for arg in linker_args(self, compiler.host, LldThreads::Yes) { + hostflags.arg(&arg); + } if let Some(target_linker) = self.linker(target) { let target = crate::envify(&target.triple); @@ -1685,12 +1683,12 @@ impl<'a> Builder<'a> { } // We want to set -Clinker using Cargo, therefore we only call `linker_flags` and not // `linker_args` here. - linker_flags(self, target, LldThreads::Yes).into_iter().for_each(|flag| { + for flag in linker_flags(self, target, LldThreads::Yes) { rustflags.arg(&flag); - }); - linker_args(self, target, LldThreads::Yes).into_iter().for_each(|flag| { - rustdocflags.arg(&flag); - }); + } + for arg in linker_args(self, target, LldThreads::Yes) { + rustdocflags.arg(&arg); + } if !(["build", "check", "clippy", "fix", "rustc"].contains(&cmd)) && want_rustdoc { cargo.env("RUSTDOC_LIBDIR", self.rustc_libdir(compiler)); diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 105b7d74dfc1e..3eaeb73eecbc2 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -111,12 +111,12 @@ impl Display for DebuginfoLevel { /// /// It is configured depending on the target: /// 1) Everything except MSVC -/// - Self-contained: -Clinker-flavor=gnu-lld-cc -Clink-self-contained=+linker -/// - External: -Clinker-flavor=gnu-lld-cc +/// - Self-contained: `-Clinker-flavor=gnu-lld-cc -Clink-self-contained=+linker` +/// - External: `-Clinker-flavor=gnu-lld-cc` /// 2) MSVC -/// - Self-contained: -Clinker= -/// - External: -Clinker=lld -#[derive(Default, Clone)] +/// - Self-contained: `-Clinker=` +/// - External: `-Clinker=lld` +#[derive(Default, Copy, Clone)] pub enum LldMode { /// Do not use LLD #[default] diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 8a818fb87ffde..e563a83c5c478 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -378,6 +378,7 @@ fn absolute_unix(path: &Path) -> io::Result { #[cfg(windows)] fn absolute_windows(path: &std::path::Path) -> std::io::Result { + use std::ffi::OsString; use std::io::Error; use std::os::windows::ffi::{OsStrExt, OsStringExt}; use std::ptr::null_mut; @@ -526,7 +527,7 @@ pub fn linker_flags( if matches!(lld_threads, LldThreads::No) { args.push(format!( "-Clink-arg=-Wl,{}", - lld_flag_no_threads(builder.config.lld_mode.clone(), target.is_msvc()) + lld_flag_no_threads(builder.config.lld_mode, target.is_msvc()) )); } }