From 1286d982781c4d5bb009e3e8189ccbe0bcbf0f4b Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 20 Dec 2022 16:02:15 +0100 Subject: [PATCH 1/2] Don't explicitly set C++ standard for lld LLVM does this itself since 606cb8548a1b7763e0c8489c5efe66803a7ede72, and 14 is no longer the correct standard when building lld 16, causing build failures. --- src/bootstrap/native.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index f6c453ebe10..8052bde6024 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -886,10 +886,6 @@ impl Step for Lld { ); } - // Explicitly set C++ standard, because upstream doesn't do so - // for standalone builds. - cfg.define("CMAKE_CXX_STANDARD", "14"); - cfg.build(); t!(File::create(&done_stamp)); From 59b3157c45048edca3cb94841d799d2ab1fe3c43 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 20 Dec 2022 17:07:04 +0100 Subject: [PATCH 2/2] Use LLVM_CMAKE_DIR for lld build LLVM_CONFIG_PATH is no longer supported as of LLVM 16, switch to using the cmake module instead. We separately return the llvm-config and cmake directory paths, because llvm-config always refers to the host binary, while the cmake directory is for the target triple. --- src/bootstrap/Cargo.toml | 5 -- src/bootstrap/bin/llvm-config-wrapper.rs | 24 ------ src/bootstrap/builder.rs | 2 +- src/bootstrap/compile.rs | 7 +- src/bootstrap/dist.rs | 7 +- src/bootstrap/native.rs | 105 ++++++++++------------- src/bootstrap/test.rs | 3 +- 7 files changed, 58 insertions(+), 95 deletions(-) delete mode 100644 src/bootstrap/bin/llvm-config-wrapper.rs diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index ccc7ec1fce9..fafe82a9c12 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -29,11 +29,6 @@ name = "sccache-plus-cl" path = "bin/sccache-plus-cl.rs" test = false -[[bin]] -name = "llvm-config-wrapper" -path = "bin/llvm-config-wrapper.rs" -test = false - [dependencies] cmake = "0.1.38" fd-lock = "3.0.8" diff --git a/src/bootstrap/bin/llvm-config-wrapper.rs b/src/bootstrap/bin/llvm-config-wrapper.rs deleted file mode 100644 index 89984bb55df..00000000000 --- a/src/bootstrap/bin/llvm-config-wrapper.rs +++ /dev/null @@ -1,24 +0,0 @@ -// The sheer existence of this file is an awful hack. See the comments in -// `src/bootstrap/native.rs` for why this is needed when compiling LLD. - -use std::env; -use std::io::{self, Write}; -use std::process::{self, Command, Stdio}; - -fn main() { - let real_llvm_config = env::var_os("LLVM_CONFIG_REAL").unwrap(); - let mut cmd = Command::new(real_llvm_config); - cmd.args(env::args().skip(1)).stderr(Stdio::piped()); - let output = cmd.output().expect("failed to spawn llvm-config"); - let mut stdout = String::from_utf8_lossy(&output.stdout); - - if let Ok(to_replace) = env::var("LLVM_CONFIG_SHIM_REPLACE") { - if let Ok(replace_with) = env::var("LLVM_CONFIG_SHIM_REPLACE_WITH") { - stdout = stdout.replace(&to_replace, &replace_with).into(); - } - } - - print!("{}", stdout.replace("\\", "/")); - io::stdout().flush().unwrap(); - process::exit(output.status.code().unwrap_or(1)); -} diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 1d37d68c1d4..b9d06a77966 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1068,7 +1068,7 @@ impl<'a> Builder<'a> { /// check build or dry-run, where there's no need to build all of LLVM. fn llvm_config(&self, target: TargetSelection) -> Option { if self.config.llvm_enabled() && self.kind != Kind::Check && !self.config.dry_run() { - let llvm_config = self.ensure(native::Llvm { target }); + let native::LlvmResult { llvm_config, .. } = self.ensure(native::Llvm { target }); if llvm_config.is_file() { return Some(llvm_config); } diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 0deed3f990d..b62e0bfe4e0 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -805,7 +805,7 @@ pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetS if builder.is_rust_llvm(target) { cargo.env("LLVM_RUSTLLVM", "1"); } - let llvm_config = builder.ensure(native::Llvm { target }); + let native::LlvmResult { llvm_config, .. } = builder.ensure(native::Llvm { target }); cargo.env("LLVM_CONFIG", &llvm_config); if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) { cargo.env("CFG_LLVM_ROOT", s); @@ -1341,9 +1341,10 @@ impl Step for Assemble { } if builder.config.rust_codegen_backends.contains(&INTERNER.intern_str("llvm")) { - let llvm_config_bin = builder.ensure(native::Llvm { target: target_compiler.host }); + let native::LlvmResult { llvm_config, .. } = + builder.ensure(native::Llvm { target: target_compiler.host }); if !builder.config.dry_run() { - let llvm_bin_dir = output(Command::new(llvm_config_bin).arg("--bindir")); + let llvm_bin_dir = output(Command::new(llvm_config).arg("--bindir")); let llvm_bin_dir = Path::new(llvm_bin_dir.trim()); // Since we've already built the LLVM tools, install them to the sysroot. diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index 3cb0eccd324..340aa78ebf9 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -24,6 +24,7 @@ use crate::channel; use crate::compile; use crate::config::TargetSelection; use crate::doc::DocumentationFormat; +use crate::native; use crate::tarball::{GeneratedTarball, OverlayKind, Tarball}; use crate::tool::{self, Tool}; use crate::util::{exe, is_dylib, output, t, timeit}; @@ -1927,7 +1928,9 @@ fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir builder.install(&llvm_dylib_path, dst_libdir, 0o644); } !builder.config.dry_run() - } else if let Ok(llvm_config) = crate::native::prebuilt_llvm_config(builder, target) { + } else if let Ok(native::LlvmResult { llvm_config, .. }) = + native::prebuilt_llvm_config(builder, target) + { let mut cmd = Command::new(llvm_config); cmd.arg("--libfiles"); builder.verbose(&format!("running {:?}", cmd)); @@ -2137,7 +2140,7 @@ impl Step for Bootstrap { let tarball = Tarball::new(builder, "bootstrap", &target.triple); let bootstrap_outdir = &builder.bootstrap_out; - for file in &["bootstrap", "llvm-config-wrapper", "rustc", "rustdoc", "sccache-plus-cl"] { + for file in &["bootstrap", "rustc", "rustdoc", "sccache-plus-cl"] { tarball.add_file(bootstrap_outdir.join(exe(file, target)), "bootstrap/bin", 0o755); } diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 8052bde6024..b944c8210be 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -24,9 +24,18 @@ use crate::util::get_clang_cl_resource_dir; use crate::util::{self, exe, output, t, up_to_date}; use crate::{CLang, GitRepo}; +#[derive(Clone)] +pub struct LlvmResult { + /// Path to llvm-config binary. + /// NB: This is always the host llvm-config! + pub llvm_config: PathBuf, + /// Path to LLVM cmake directory for the target. + pub llvm_cmake_dir: PathBuf, +} + pub struct Meta { stamp: HashStamp, - build_llvm_config: PathBuf, + res: LlvmResult, out_dir: PathBuf, root: String, } @@ -64,7 +73,7 @@ impl LdFlags { pub fn prebuilt_llvm_config( builder: &Builder<'_>, target: TargetSelection, -) -> Result { +) -> Result { builder.config.maybe_download_ci_llvm(); // If we're using a custom LLVM bail out here, but we can only use a @@ -72,7 +81,14 @@ pub fn prebuilt_llvm_config( if let Some(config) = builder.config.target_config.get(&target) { if let Some(ref s) = config.llvm_config { check_llvm_version(builder, s); - return Ok(s.to_path_buf()); + let llvm_config = s.to_path_buf(); + let mut llvm_cmake_dir = llvm_config.clone(); + llvm_cmake_dir.pop(); + llvm_cmake_dir.pop(); + llvm_cmake_dir.push("lib"); + llvm_cmake_dir.push("cmake"); + llvm_cmake_dir.push("llvm"); + return Ok(LlvmResult { llvm_config, llvm_cmake_dir }); } } @@ -84,8 +100,9 @@ pub fn prebuilt_llvm_config( llvm_config_ret_dir.push("build"); } llvm_config_ret_dir.push("bin"); - let build_llvm_config = llvm_config_ret_dir.join(exe("llvm-config", builder.config.build)); + let llvm_cmake_dir = out_dir.join("lib/cmake/llvm"); + let res = LlvmResult { llvm_config: build_llvm_config, llvm_cmake_dir }; let stamp = out_dir.join("llvm-finished-building"); let stamp = HashStamp::new(stamp, builder.in_tree_llvm_info.sha()); @@ -96,7 +113,7 @@ pub fn prebuilt_llvm_config( Using a potentially stale build of LLVM; \ This may not behave well.", ); - return Ok(build_llvm_config); + return Ok(res); } if stamp.is_done() { @@ -110,10 +127,10 @@ pub fn prebuilt_llvm_config( stamp.path.display() )); } - return Ok(build_llvm_config); + return Ok(res); } - Err(Meta { stamp, build_llvm_config, out_dir, root: root.into() }) + Err(Meta { stamp, res, out_dir, root: root.into() }) } /// This retrieves the LLVM sha we *want* to use, according to git history. @@ -223,7 +240,7 @@ pub struct Llvm { } impl Step for Llvm { - type Output = PathBuf; // path to llvm-config + type Output = LlvmResult; const ONLY_HOSTS: bool = true; @@ -236,7 +253,7 @@ impl Step for Llvm { } /// Compile LLVM for `target`. - fn run(self, builder: &Builder<'_>) -> PathBuf { + fn run(self, builder: &Builder<'_>) -> LlvmResult { let target = self.target; let target_native = if self.target.starts_with("riscv") { // RISC-V target triples in Rust is not named the same as C compiler target triples. @@ -252,11 +269,10 @@ impl Step for Llvm { target.to_string() }; - let Meta { stamp, build_llvm_config, out_dir, root } = - match prebuilt_llvm_config(builder, target) { - Ok(p) => return p, - Err(m) => m, - }; + let Meta { stamp, res, out_dir, root } = match prebuilt_llvm_config(builder, target) { + Ok(p) => return p, + Err(m) => m, + }; builder.update_submodule(&Path::new("src").join("llvm-project")); if builder.llvm_link_shared() && target.contains("windows") { @@ -430,7 +446,8 @@ impl Step for Llvm { // https://llvm.org/docs/HowToCrossCompileLLVM.html if target != builder.config.build { - let llvm_config = builder.ensure(Llvm { target: builder.config.build }); + let LlvmResult { llvm_config, .. } = + builder.ensure(Llvm { target: builder.config.build }); if !builder.config.dry_run() { let llvm_bindir = output(Command::new(&llvm_config).arg("--bindir")); let host_bin = Path::new(llvm_bindir.trim()); @@ -480,7 +497,7 @@ impl Step for Llvm { // tools and libs on all platforms. if builder.config.dry_run() { - return build_llvm_config; + return res; } cfg.build(); @@ -490,7 +507,7 @@ impl Step for Llvm { // for a versioned path like libLLVM-14.dylib. Manually create a symbolic // link to make llvm-config happy. if builder.llvm_link_shared() && target.contains("apple-darwin") { - let mut cmd = Command::new(&build_llvm_config); + let mut cmd = Command::new(&res.llvm_config); let version = output(cmd.arg("--version")); let major = version.split('.').next().unwrap(); let lib_name = match llvm_version_suffix { @@ -509,18 +526,18 @@ impl Step for Llvm { // LLVM after a configuration change, so to rebuild it the build files have to be removed, // which will also remove these modified files. if builder.config.llvm_bolt_profile_generate { - instrument_with_bolt_inplace(&get_built_llvm_lib_path(&build_llvm_config)); + instrument_with_bolt_inplace(&get_built_llvm_lib_path(&res.llvm_config)); } if let Some(path) = &builder.config.llvm_bolt_profile_use { optimize_library_with_bolt_inplace( - &get_built_llvm_lib_path(&build_llvm_config), + &get_built_llvm_lib_path(&res.llvm_config), &Path::new(path), ); } t!(stamp.write()); - build_llvm_config + res } } @@ -803,7 +820,8 @@ impl Step for Lld { } let target = self.target; - let llvm_config = builder.ensure(Llvm { target: self.target }); + let LlvmResult { llvm_config, llvm_cmake_dir } = + builder.ensure(Llvm { target: self.target }); let out_dir = builder.lld_out(target); let done_stamp = out_dir.join("lld-finished-building"); @@ -834,22 +852,6 @@ impl Step for Lld { configure_cmake(builder, target, &mut cfg, true, ldflags); configure_llvm(builder, target, &mut cfg); - // This is an awful, awful hack. Discovered when we migrated to using - // clang-cl to compile LLVM/LLD it turns out that LLD, when built out of - // tree, will execute `llvm-config --cmakedir` and then tell CMake about - // that directory for later processing. Unfortunately if this path has - // forward slashes in it (which it basically always does on Windows) - // then CMake will hit a syntax error later on as... something isn't - // escaped it seems? - // - // Instead of attempting to fix this problem in upstream CMake and/or - // LLVM/LLD we just hack around it here. This thin wrapper will take the - // output from llvm-config and replace all instances of `\` with `/` to - // ensure we don't hit the same bugs with escaping. It means that you - // can't build on a system where your paths require `\` on Windows, but - // there's probably a lot of reasons you can't do that other than this. - let llvm_config_shim = env::current_exe().unwrap().with_file_name("llvm-config-wrapper"); - // Re-use the same flags as llvm to control the level of debug information // generated for lld. let profile = match (builder.config.llvm_optimize, builder.config.llvm_release_debuginfo) { @@ -860,30 +862,15 @@ impl Step for Lld { cfg.out_dir(&out_dir) .profile(profile) - .env("LLVM_CONFIG_REAL", &llvm_config) - .define("LLVM_CONFIG_PATH", llvm_config_shim) + .define("LLVM_CMAKE_DIR", llvm_cmake_dir) .define("LLVM_INCLUDE_TESTS", "OFF"); - // While we're using this horrible workaround to shim the execution of - // llvm-config, let's just pile on more. I can't seem to figure out how - // to build LLD as a standalone project and also cross-compile it at the - // same time. It wants a natively executable `llvm-config` to learn - // about LLVM, but then it learns about all the host configuration of - // LLVM and tries to link to host LLVM libraries. - // - // To work around that we tell our shim to replace anything with the - // build target with the actual target instead. This'll break parts of - // LLD though which try to execute host tools, such as llvm-tblgen, so - // we specifically tell it where to find those. This is likely super - // brittle and will break over time. If anyone knows better how to - // cross-compile LLD it would be much appreciated to fix this! if target != builder.config.build { - cfg.env("LLVM_CONFIG_SHIM_REPLACE", &builder.config.build.triple) - .env("LLVM_CONFIG_SHIM_REPLACE_WITH", &target.triple) - .define( - "LLVM_TABLEGEN_EXE", - llvm_config.with_file_name("llvm-tblgen").with_extension(EXE_EXTENSION), - ); + // Use the host llvm-tblgen binary. + cfg.define( + "LLVM_TABLEGEN_EXE", + llvm_config.with_file_name("llvm-tblgen").with_extension(EXE_EXTENSION), + ); } cfg.build(); @@ -987,7 +974,7 @@ impl Step for Sanitizers { return runtimes; } - let llvm_config = builder.ensure(Llvm { target: builder.config.build }); + let LlvmResult { llvm_config, .. } = builder.ensure(Llvm { target: builder.config.build }); if builder.config.dry_run() { return runtimes; } diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 39cedfdac5f..0d9c22e210f 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1575,7 +1575,8 @@ note: if you're sure you want to do this, please open an issue as to why. In the let mut llvm_components_passed = false; let mut copts_passed = false; if builder.config.llvm_enabled() { - let llvm_config = builder.ensure(native::Llvm { target: builder.config.build }); + let native::LlvmResult { llvm_config, .. } = + builder.ensure(native::Llvm { target: builder.config.build }); if !builder.config.dry_run() { let llvm_version = output(Command::new(&llvm_config).arg("--version")); let llvm_components = output(Command::new(&llvm_config).arg("--components"));