Skip to content

Commit

Permalink
Enable -Zshare-generics for inline(never) functions
Browse files Browse the repository at this point in the history
This avoids inlining cross-crate generic items when possible that are
already marked inline(never), implying that the author is not intending
for the function to be inlined by callers. As such, having a local copy
may make it easier for LLVM to optimize but mostly just adds to binary
bloat and codegen time (in theory, TBD on in practice).

It might also make sense it expand this with other heuristics (e.g., #[cold]).
  • Loading branch information
Mark-Simulacrum committed Mar 30, 2024
1 parent faae5f1 commit 5702d83
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 19 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4361,6 +4361,7 @@ dependencies = [
name = "rustc_monomorphize"
version = "0.0.0"
dependencies = [
"rustc_attr",
"rustc_data_structures",
"rustc_errors",
"rustc_fluent_macro",
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_llvm/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) ->
// This is a monomorphization. Its expected visibility depends
// on whether we are in share-generics mode.

if cx.tcx.sess.opts.share_generics() {
if cx.tcx.sess.opts.share_generics()
|| tcx.codegen_fn_attrs(instance_def_id).inline == rustc_attr::InlineAttr::Never
{
// We are in share_generics mode.

if let Some(instance_def_id) = instance_def_id.as_local() {
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ fn exported_symbols_provider_local(
));
}

if tcx.sess.opts.share_generics() && tcx.local_crate_exports_generics() {
if tcx.local_crate_exports_generics() {
use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility};
use rustc_middle::ty::InstanceDef;

Expand Down Expand Up @@ -334,6 +334,16 @@ fn exported_symbols_provider_local(
continue;
}

if !tcx.sess.opts.share_generics() {
if tcx.codegen_fn_attrs(mono_item.def_id()).inline == rustc_attr::InlineAttr::Never
{
// this is OK, we explicitly allow sharing inline(never) across crates even
// without share-generics.
} else {
continue;
}
}

match *mono_item {
MonoItem::Fn(Instance { def: InstanceDef::Item(def), args }) => {
if args.non_erasable_generics(tcx, def).next().is_some() {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ impl<'tcx> MonoItem<'tcx> {
return InstantiationMode::GloballyShared { may_conflict: false };
}

if let InlineAttr::Never = tcx.codegen_fn_attrs(instance.def_id()).inline
&& self.is_generic_fn(tcx)
{
// Upgrade inline(never) to a globally shared instance.
return InstantiationMode::GloballyShared { may_conflict: true };
}

// At this point we don't have explicit linkage and we're an
// inlined function. If we're inlining into all CGUs then we'll
// be creating a local copy per CGU.
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,8 +1301,6 @@ impl<'tcx> TyCtxt<'tcx> {

#[inline]
pub fn local_crate_exports_generics(self) -> bool {
debug_assert!(self.sess.opts.share_generics());

self.crate_types().iter().any(|crate_type| {
match crate_type {
CrateType::Executable
Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,19 +157,23 @@ impl<'tcx> Instance<'tcx> {
/// This method already takes into account the global `-Zshare-generics`
/// setting, always returning `None` if `share-generics` is off.
pub fn upstream_monomorphization(&self, tcx: TyCtxt<'tcx>) -> Option<CrateNum> {
// If we are not in share generics mode, we don't link to upstream
// monomorphizations but always instantiate our own internal versions
// instead.
if !tcx.sess.opts.share_generics() {
return None;
}

// If this is an item that is defined in the local crate, no upstream
// crate can know about it/provide a monomorphization.
if self.def_id().is_local() {
return None;
}

// If we are not in share generics mode, we don't link to upstream
// monomorphizations but always instantiate our own internal versions
// instead.
if !tcx.sess.opts.share_generics()
// However, if the def_id is marked inline(never), then it's fine to just reuse the
// upstream monomorphization.
&& tcx.codegen_fn_attrs(self.def_id()).inline != rustc_attr::InlineAttr::Never
{
return None;
}

// If this a non-generic instance, it cannot be a shared monomorphization.
self.args.non_erasable_generics(tcx, self.def_id()).next()?;

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_monomorphize/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
rustc_hir = { path = "../rustc_hir" }
rustc_attr = { path = "../rustc_attr" }

Check failure on line 12 in compiler/rustc_monomorphize/Cargo.toml

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line not in alphabetical order
rustc_macros = { path = "../rustc_macros" }
rustc_middle = { path = "../rustc_middle" }
rustc_session = { path = "../rustc_session" }
Expand Down
34 changes: 26 additions & 8 deletions compiler/rustc_monomorphize/src/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ where
// available to downstream crates. This depends on whether we are in
// share-generics mode and whether the current crate can even have
// downstream crates.
let export_generics =
cx.tcx.sess.opts.share_generics() && cx.tcx.local_crate_exports_generics();
let can_export_generics = cx.tcx.local_crate_exports_generics();
let always_export_generics = can_export_generics && cx.tcx.sess.opts.share_generics();

let cgu_name_builder = &mut CodegenUnitNameBuilder::new(cx.tcx);
let cgu_name_cache = &mut FxHashMap::default();
Expand Down Expand Up @@ -244,7 +244,8 @@ where
cx.tcx,
&mono_item,
&mut can_be_internalized,
export_generics,
can_export_generics,
always_export_generics,
);
if visibility == Visibility::Hidden && can_be_internalized {
internalization_candidates.insert(mono_item);
Expand Down Expand Up @@ -727,12 +728,19 @@ fn mono_item_linkage_and_visibility<'tcx>(
tcx: TyCtxt<'tcx>,
mono_item: &MonoItem<'tcx>,
can_be_internalized: &mut bool,
export_generics: bool,
can_export_generics: bool,
always_export_generics: bool,
) -> (Linkage, Visibility) {
if let Some(explicit_linkage) = mono_item.explicit_linkage(tcx) {
return (explicit_linkage, Visibility::Default);
}
let vis = mono_item_visibility(tcx, mono_item, can_be_internalized, export_generics);
let vis = mono_item_visibility(
tcx,
mono_item,
can_be_internalized,
can_export_generics,
always_export_generics,
);
(Linkage::External, vis)
}

Expand All @@ -755,7 +763,8 @@ fn mono_item_visibility<'tcx>(
tcx: TyCtxt<'tcx>,
mono_item: &MonoItem<'tcx>,
can_be_internalized: &mut bool,
export_generics: bool,
can_export_generics: bool,
always_export_generics: bool,
) -> Visibility {
let instance = match mono_item {
// This is pretty complicated; see below.
Expand Down Expand Up @@ -812,7 +821,11 @@ fn mono_item_visibility<'tcx>(

// Upstream `DefId` instances get different handling than local ones.
let Some(def_id) = def_id.as_local() else {
return if export_generics && is_generic {
return if is_generic
&& (always_export_generics
|| (can_export_generics
&& tcx.codegen_fn_attrs(def_id).inline == rustc_attr::InlineAttr::Never))
{
// If it is an upstream monomorphization and we export generics, we must make
// it available to downstream crates.
*can_be_internalized = false;
Expand All @@ -823,7 +836,12 @@ fn mono_item_visibility<'tcx>(
};

if is_generic {
if export_generics {
// An inline(never) function won't get inlined in upstream crates anyway, so we might as
// well export it.
if always_export_generics
|| (can_export_generics
&& tcx.codegen_fn_attrs(def_id).inline == rustc_attr::InlineAttr::Never)
{
if tcx.is_unreachable_local_definition(def_id) {
// This instance cannot be used from another crate.
Visibility::Hidden
Expand Down
11 changes: 11 additions & 0 deletions tests/codegen-units/partitioning/auxiliary/cgu_generic_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,21 @@ pub fn foo<T>(x: T) -> (T, u32, i8) {
#[inline(never)]
fn bar<T>(x: T) -> (T, Struct) {
let _ = not_exported_and_not_generic(0);
exported_and_generic::<u32>(0);
(x, Struct(1))
}

pub static F: fn(u32) -> u32 = exported_and_generic::<u32>;

// These should not contribute to the codegen items of other crates.

// This is generic, but it's only instantiated with a u32 argument and that instantiation is present
// in the local crate (see F above).
#[inline(never)]
pub fn exported_and_generic<T>(x: T) -> T {
x
}

#[inline(never)]
pub fn exported_but_not_generic(x: i32) -> i64 {
x as i64
Expand Down

0 comments on commit 5702d83

Please sign in to comment.