diff --git a/compiler/rustc_mir_transform/src/cross_crate_inline.rs b/compiler/rustc_mir_transform/src/cross_crate_inline.rs index 8fce856687cb8..9e5d4a436584d 100644 --- a/compiler/rustc_mir_transform/src/cross_crate_inline.rs +++ b/compiler/rustc_mir_transform/src/cross_crate_inline.rs @@ -34,6 +34,15 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { return true; } + // compiler-builtins only defines intrinsics (which are handled above by checking + // contains_extern_indicator) and helper functions used by those intrinsics. The helper + // functions should always be inlined into intrinsics that use them. This check does not + // guarantee that we get the optimizations we want, but it makes them *much* easier. + // See https://github.com/rust-lang/rust/issues/73135 + if tcx.is_compiler_builtins(rustc_span::def_id::LOCAL_CRATE) { + return true; + } + if tcx.has_attr(def_id, sym::rustc_intrinsic) { // Intrinsic fallback bodies are always cross-crate inlineable. // To ensure that the MIR inliner doesn't cluelessly try to inline fallback diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 7b17966343084..6facb0dd6ecee 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -165,7 +165,8 @@ where // estimates. { let _prof_timer = tcx.prof.generic_activity("cgu_partitioning_merge_cgus"); - merge_codegen_units(cx, &mut codegen_units); + let cgu_contents = merge_codegen_units(cx, &mut codegen_units); + rename_codegen_units(cx, &mut codegen_units, cgu_contents); debug_dump(tcx, "MERGE", &codegen_units); } @@ -200,7 +201,6 @@ where I: Iterator>, { let mut codegen_units = UnordMap::default(); - let is_incremental_build = cx.tcx.sess.opts.incremental.is_some(); let mut internalization_candidates = UnordSet::default(); // Determine if monomorphizations instantiated in this crate will be made @@ -227,20 +227,8 @@ where } } - let characteristic_def_id = characteristic_def_id_of_mono_item(cx.tcx, mono_item); - let is_volatile = is_incremental_build && mono_item.is_generic_fn(); - - let cgu_name = match characteristic_def_id { - Some(def_id) => compute_codegen_unit_name( - cx.tcx, - cgu_name_builder, - def_id, - is_volatile, - cgu_name_cache, - ), - None => fallback_cgu_name(cgu_name_builder), - }; - + let cgu_name = + compute_codegen_unit_name(cx.tcx, cgu_name_builder, mono_item, cgu_name_cache); let cgu = codegen_units.entry(cgu_name).or_insert_with(|| CodegenUnit::new(cgu_name)); let mut can_be_internalized = true; @@ -321,7 +309,7 @@ where fn merge_codegen_units<'tcx>( cx: &PartitioningCx<'_, 'tcx>, codegen_units: &mut Vec>, -) { +) -> UnordMap> { assert!(cx.tcx.sess.codegen_units().as_usize() >= 1); // A sorted order here ensures merging is deterministic. @@ -331,6 +319,13 @@ fn merge_codegen_units<'tcx>( let mut cgu_contents: UnordMap> = codegen_units.iter().map(|cgu| (cgu.name(), vec![cgu.name()])).collect(); + // When compiling compiler_builtins, we do not want to put multiple intrinsics in a CGU. + // There may be mergeable CGUs under this constraint, but just skipping over merging is much + // simpler. + if cx.tcx.is_compiler_builtins(LOCAL_CRATE) { + return cgu_contents; + } + // If N is the maximum number of CGUs, and the CGUs are sorted from largest // to smallest, we repeatedly find which CGU in codegen_units[N..] has the // greatest overlap of inlined items with codegen_units[N-1], merge that @@ -421,6 +416,14 @@ fn merge_codegen_units<'tcx>( // Don't update `cgu_contents`, that's only for incremental builds. } + cgu_contents +} + +fn rename_codegen_units<'tcx>( + cx: &PartitioningCx<'_, 'tcx>, + codegen_units: &mut Vec>, + cgu_contents: UnordMap>, +) { let cgu_name_builder = &mut CodegenUnitNameBuilder::new(cx.tcx); // Rename the newly merged CGUs. @@ -678,13 +681,26 @@ fn characteristic_def_id_of_mono_item<'tcx>( } } -fn compute_codegen_unit_name( - tcx: TyCtxt<'_>, +fn compute_codegen_unit_name<'tcx>( + tcx: TyCtxt<'tcx>, name_builder: &mut CodegenUnitNameBuilder<'_>, - def_id: DefId, - volatile: bool, + mono_item: MonoItem<'tcx>, cache: &mut CguNameCache, ) -> Symbol { + // When compiling compiler_builtins, we do not want to put multiple intrinsics in a CGU. + // Using the symbol name as the CGU name puts every GloballyShared item in its own CGU, but in + // an optimized build we actually want every item in the crate that isn't an intrinsic to get + // LocalCopy so that it is easy to inline away. In an unoptimized build, this CGU naming + // strategy probably generates more CGUs than we strictly need. But it is simple. + if tcx.is_compiler_builtins(LOCAL_CRATE) { + let name = mono_item.symbol_name(tcx); + return Symbol::intern(name.name); + } + + let Some(def_id) = characteristic_def_id_of_mono_item(tcx, mono_item) else { + return fallback_cgu_name(name_builder); + }; + // Find the innermost module that is not nested within a function. let mut current_def_id = def_id; let mut cgu_def_id = None; @@ -712,6 +728,9 @@ fn compute_codegen_unit_name( let cgu_def_id = cgu_def_id.unwrap(); + let is_incremental_build = tcx.sess.opts.incremental.is_some(); + let volatile = is_incremental_build && mono_item.is_generic_fn(); + *cache.entry((cgu_def_id, volatile)).or_insert_with(|| { let def_path = tcx.def_path(cgu_def_id); diff --git a/library/Cargo.toml b/library/Cargo.toml index e59aa518804f3..9fcddfc019bb2 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -11,19 +11,6 @@ exclude = [ "windows_targets" ] -[profile.release.package.compiler_builtins] -# For compiler-builtins we always use a high number of codegen units. -# The goal here is to place every single intrinsic into its own object -# file to avoid symbol clashes with the system libgcc if possible. Note -# that this number doesn't actually produce this many object files, we -# just don't create more than this number of object files. -# -# It's a bit of a bummer that we have to pass this here, unfortunately. -# Ideally this would be specified through an env var to Cargo so Cargo -# knows how many CGUs are for this specific crate, but for now -# per-crate configuration isn't specifiable in the environment. -codegen-units = 10000 - # These dependencies of the standard library implement symbolication for # backtraces on most platforms. Their debuginfo causes both linking to be slower # (more data to chew through) and binaries to be larger without really all that diff --git a/tests/run-make/compiler-builtins/Cargo.toml b/tests/run-make/compiler-builtins-linkage/Cargo.toml similarity index 100% rename from tests/run-make/compiler-builtins/Cargo.toml rename to tests/run-make/compiler-builtins-linkage/Cargo.toml diff --git a/tests/run-make/compiler-builtins/lib.rs b/tests/run-make/compiler-builtins-linkage/lib.rs similarity index 100% rename from tests/run-make/compiler-builtins/lib.rs rename to tests/run-make/compiler-builtins-linkage/lib.rs diff --git a/tests/run-make/compiler-builtins/rmake.rs b/tests/run-make/compiler-builtins-linkage/rmake.rs similarity index 92% rename from tests/run-make/compiler-builtins/rmake.rs rename to tests/run-make/compiler-builtins-linkage/rmake.rs index 10093db2258df..e45b38003ed23 100644 --- a/tests/run-make/compiler-builtins/rmake.rs +++ b/tests/run-make/compiler-builtins-linkage/rmake.rs @@ -1,4 +1,7 @@ -//! The compiler_builtins library is special. It can call functions in core, but it must not +//! The compiler_builtins library is special. When linkers are passed multiple libraries, they +//! expect undefined symbols mentioned by libraries on the left to be defined in libraries to the +//! right. Since calls to compiler_builtins may be inserted during codegen, it is placed all the way +//! to the right. Therefore, compiler_builtins can call functions in core but it must not //! require linkage against a build of core. If it ever does, building the standard library *may* //! result in linker errors, depending on whether the linker in use applies optimizations first or //! resolves symbols first. So the portable and safe approach is to forbid such a linkage diff --git a/tests/run-make/compiler-builtins-partitioning/Cargo.toml b/tests/run-make/compiler-builtins-partitioning/Cargo.toml new file mode 100644 index 0000000000000..869210c4a683c --- /dev/null +++ b/tests/run-make/compiler-builtins-partitioning/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "scratch" +version = "0.1.0" +edition = "2021" + +[lib] +path = "lib.rs" diff --git a/tests/run-make/compiler-builtins-partitioning/lib.rs b/tests/run-make/compiler-builtins-partitioning/lib.rs new file mode 100644 index 0000000000000..0c9ac1ac8e4bd --- /dev/null +++ b/tests/run-make/compiler-builtins-partitioning/lib.rs @@ -0,0 +1 @@ +#![no_std] diff --git a/tests/run-make/compiler-builtins-partitioning/rmake.rs b/tests/run-make/compiler-builtins-partitioning/rmake.rs new file mode 100644 index 0000000000000..d8f5e77494997 --- /dev/null +++ b/tests/run-make/compiler-builtins-partitioning/rmake.rs @@ -0,0 +1,105 @@ +//! The compiler_builtins library is special. It exists to export a number of intrinsics which may +//! also be provided by libgcc or compiler-rt, and when an intrinsic is provided by another +//! library, we want that definition to override the one in compiler_builtins because we expect +//! that those implementations are more optimized than compiler_builtins. To make sure that an +//! attempt to override a compiler_builtins intrinsic does not result in a multiple definitions +//! linker error, the compiler has special CGU partitioning logic for compiler_builtins that +//! ensures every intrinsic gets its own CGU. +//! +//! This test is slightly overfit to the current compiler_builtins CGU naming strategy; it doesn't +//! distinguish between "multiple intrinsics are in one object file!" which would be very bad, and +//! "This object file has an intrinsic and also some of its helper functions!" which would be okay. +//! +//! This test ensures that the compiler_builtins rlib has only one intrinsic in each object file. + +// wasm and nvptx targets don't produce rlib files that object can parse. +//@ ignore-wasm +//@ ignore-nvptx64 + +#![deny(warnings)] + +use std::str; + +use run_make_support::object::read::Object; +use run_make_support::object::read::archive::ArchiveFile; +use run_make_support::object::{ObjectSymbol, SymbolKind}; +use run_make_support::rfs::{read, read_dir}; +use run_make_support::{cargo, object, path, target}; + +fn main() { + println!("Testing compiler_builtins CGU partitioning for {}", target()); + + // CGU partitioning has some special cases for codegen-units=1, so we also test 2 CGUs. + for cgus in [1, 2] { + for profile in ["debug", "release"] { + run_test(profile, cgus); + } + } +} + +fn run_test(profile: &str, cgus: usize) { + println!("Testing with profile {profile} and -Ccodegen-units={cgus}"); + + let target_dir = path("target"); + + let mut cmd = cargo(); + cmd.args(&[ + "build", + "--manifest-path", + "Cargo.toml", + "-Zbuild-std=core", + "--target", + &target(), + ]) + .env("RUSTFLAGS", &format!("-Ccodegen-units={cgus}")) + .env("CARGO_TARGET_DIR", &target_dir) + .env("RUSTC_BOOTSTRAP", "1") + // Visual Studio 2022 requires that the LIB env var be set so it can + // find the Windows SDK. + .env("LIB", std::env::var("LIB").unwrap_or_default()); + if profile == "release" { + cmd.arg("--release"); + } + cmd.run(); + + let rlibs_path = target_dir.join(target()).join(profile).join("deps"); + let compiler_builtins_rlib = read_dir(rlibs_path) + .find_map(|e| { + let path = e.unwrap().path(); + let file_name = path.file_name().unwrap().to_str().unwrap(); + if file_name.starts_with("libcompiler_builtins") && file_name.ends_with(".rlib") { + Some(path) + } else { + None + } + }) + .unwrap(); + + // rlib files are archives, where the archive members are our CGUs, and we also have one called + // lib.rmeta which is the encoded metadata. Each of the CGUs is an object file. + let data = read(compiler_builtins_rlib); + + let archive = ArchiveFile::parse(&*data).unwrap(); + for member in archive.members() { + let member = member.unwrap(); + if member.name() == b"lib.rmeta" { + continue; + } + let data = member.data(&*data).unwrap(); + let object = object::File::parse(&*data).unwrap(); + + let mut global_text_symbols = 0; + println!("Inspecting object {}", str::from_utf8(&member.name()).unwrap()); + for symbol in object + .symbols() + .filter(|symbol| matches!(symbol.kind(), SymbolKind::Text)) + .filter(|symbol| symbol.is_definition() && symbol.is_global()) + { + println!("symbol: {:?}", symbol.name().unwrap()); + global_text_symbols += 1; + } + // Assert that this object/CGU does not define multiple global text symbols. + // We permit the 0 case because some CGUs may only be assigned a static. + assert!(global_text_symbols <= 1); + } +}