Auto merge of rust-lang#123244 - Mark-Simulacrum:share-inline-never-generics, r=saethlin

Enable -Zshare-generics for inline(never) functions

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 practice our benchmarks indicate this is
indeed a win for larger compilations, where the extra cost in dynamic
linking to these symbols is diminished compared to the advantages in
fewer copies that need optimizing in each binary.

It might also make sense it expand this with other heuristics (e.g.,
`#[cold]`) in the future, but this seems like a good starting point.

FWIW, I expect that doing cleanup in where we make the decision
what should/shouldn't be shared is also a good idea. Way too
much code needed to be tweaked to check this. But I'm hoping
to leave that for a follow-up PR rather than blocking this on it.

Author: bors

Cargo.lock

@@ -4137,6 +4137,7 @@ name = "rustc_monomorphize"
 version = "0.0.0"
 dependencies = [
  "rustc_abi",
+ "rustc_attr",
  "rustc_data_structures",
  "rustc_errors",
  "rustc_fluent_macro",

compiler/rustc_codegen_llvm/src/callee.rs

@@ -104,7 +104,10 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t
 
             let is_hidden = if is_generic {
                 // This is a monomorphization of a generic function.
-                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)
+                {
                     // When not sharing generics, all instances are in the same
                     // crate and have hidden visibility.
                     true

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -282,7 +282,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::InstanceKind;
 
@@ -310,6 +310,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: InstanceKind::Item(def), args }) => {
                     if args.non_erasable_generics().next().is_some() {

compiler/rustc_middle/src/mir/mono.rs

@@ -111,6 +111,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()
+                {
+                    // 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.

compiler/rustc_middle/src/ty/context.rs

@@ -1955,8 +1955,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

compiler/rustc_middle/src/ty/instance.rs

@@ -190,19 +190,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().next()?;
 

compiler/rustc_monomorphize/Cargo.toml

@@ -6,6 +6,7 @@ edition = "2021"
 [dependencies]
 # tidy-alphabetical-start
 rustc_abi = { path = "../rustc_abi" }
+rustc_attr = { path = "../rustc_attr" }
 rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_errors = { path = "../rustc_errors" }
 rustc_fluent_macro = { path = "../rustc_fluent_macro" }

compiler/rustc_monomorphize/src/partitioning.rs

@@ -208,8 +208,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 UnordMap::default();
@@ -249,7 +249,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);
@@ -739,12 +740,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)
 }
 
@@ -767,7 +775,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.
@@ -826,7 +835,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;
@@ -837,7 +850,10 @@ fn mono_item_visibility<'tcx>(
     };
 
     if is_generic {
-        if export_generics {
+        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

library/alloc/src/raw_vec.rs

@@ -757,7 +757,9 @@ impl<A: Allocator> RawVecInner<A> {
     }
 }
 
-#[inline(never)]
+// not marked inline(never) since we want optimizers to be able to observe the specifics of this
+// function, see tests/codegen/vec-reserve-extend.rs.
+#[cold]
 fn finish_grow<A>(
     new_layout: Layout,
     current_memory: Option<(NonNull<u8>, Layout)>,

library/std/src/lib.rs

@@ -362,6 +362,7 @@
 #![feature(strict_provenance_atomic_ptr)]
 #![feature(sync_unsafe_cell)]
 #![feature(ub_checks)]
+#![feature(used_with_arg)]
 // tidy-alphabetical-end
 //
 // Library features (alloc):