Skip to content

Commit

Permalink
Rollup merge of rust-lang#73136 - alexcrichton:thinlto-compiler-built…
Browse files Browse the repository at this point in the history
…ins, r=Mark-Simulacrum

Change how compiler-builtins gets many CGUs

This commit intends to fix an accidental regression from rust-lang#70846. The
goal of rust-lang#70846 was to build compiler-builtins with a maximal number of
CGUs to ensure that each module in the source corresponds to an object
file. This high degree of control for compiler-builtins is desirable to
ensure that there's at most one exported symbol per CGU, ideally
enabling compiler-builtins to not conflict with the system libgcc as
often.

In rust-lang#70846, however, only part of the compiler understands that
compiler-builtins is built with many CGUs. The rest of the compiler
thinks it's building with `sess.codegen_units()`. Notably the
calculation of `sess.lto()` consults `sess.codegen_units()`, which when
there's only one CGU it disables ThinLTO. This means that
compiler-builtins is built without ThinLTO, which is quite harmful to
performance! This is the root of the cause from rust-lang#73135 where intrinsics
were found to not be inlining trivial functions.

The fix applied in this commit is to remove the special-casing of
compiler-builtins in the compiler. Instead the build system is now
responsible for special-casing compiler-builtins. It doesn't know
exactly how many CGUs will be needed but it passes a large number that
is assumed to be much greater than the number of source-level modules
needed. After reading the various locations in the compiler source, this
seemed like the best solution rather than adding more and more special
casing in the compiler for compiler-builtins.

Closes rust-lang#73135
  • Loading branch information
Manishearth authored Jun 16, 2020
2 parents 289dabd + d6156e8 commit 16b1a49
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 48 deletions.
13 changes: 13 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ debug-assertions = false
debug = false
debug-assertions = false

[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

# We want the RLS to use the version of Cargo that we've got vendored in this
# repository to ensure that the same exact version of Cargo is used by both the
# RLS and the Cargo binary itself. The RLS depends on Cargo as a git repository
Expand Down
9 changes: 1 addition & 8 deletions src/librustc_mir/monomorphize/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,18 +454,11 @@ fn default_visibility(tcx: TyCtxt<'_>, id: DefId, is_generic: bool) -> Visibilit
fn merge_codegen_units<'tcx>(
tcx: TyCtxt<'tcx>,
initial_partitioning: &mut PreInliningPartitioning<'tcx>,
mut target_cgu_count: usize,
target_cgu_count: usize,
) {
assert!(target_cgu_count >= 1);
let codegen_units = &mut initial_partitioning.codegen_units;

if tcx.is_compiler_builtins(LOCAL_CRATE) {
// Compiler builtins require some degree of control over how mono items
// are partitioned into compilation units. Provide it by keeping the
// original partitioning when compiling the compiler builtins crate.
target_cgu_count = codegen_units.len();
}

// Note that at this point in time the `codegen_units` here may not be in a
// deterministic order (but we know they're deterministically the same set).
// We want this merging to produce a deterministic ordering of codegen units
Expand Down
40 changes: 0 additions & 40 deletions src/test/codegen-units/partitioning/compiler-builtins.rs

This file was deleted.

0 comments on commit 16b1a49

Please sign in to comment.