Auto merge of #136410 - saethlin:clean-up-cgu-internal-copy, r=<try>

Remove InstanceKind::generates_cgu_internal_copy

This is on top of #136394 for now.

`InstanceKind::generates_cgu_internal_copy` was called in two places:

1. By `reachable_non_generics`, but in that case the caller was guaranteed to not be generic and therefore we always fall through to just calling `cross_crate_inlinable`.
2. By `MonoItem::instantiation_mode`, but in that case the *only* thing that it does is some very complicated logic for selecting the instantiation mode for drop glue, and only very recently do we have a single codegen test for this. So I've touched up the logic in `cross_crate_inlinable` so that we don't try to claim that `drop_in_place` isn't cross-crate-inlinable, because it clearly is.

Now off to perf, and if we get regressions I'll start reintroducing the logic about drop glue but, but this time into `MonoItem::instantiation_mode` where it always should have been.

Author: bors

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -93,16 +93,7 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
                 return None;
             }
 
-            // Functions marked with #[inline] are codegened with "internal"
-            // linkage and are not exported unless marked with an extern
-            // indicator
-            if !Instance::mono(tcx, def_id.to_def_id()).def.generates_cgu_internal_copy(tcx)
-                || tcx.codegen_fn_attrs(def_id.to_def_id()).contains_extern_indicator()
-            {
-                Some(def_id)
-            } else {
-                None
-            }
+            if tcx.cross_crate_inlinable(def_id) { None } else { Some(def_id) }
         })
         .map(|def_id| {
             // We won't link right if this symbol is stripped during LTO.

compiler/rustc_middle/src/mir/mono.rs

@@ -92,51 +92,82 @@ impl<'tcx> MonoItem<'tcx> {
     }
 
     pub fn instantiation_mode(&self, tcx: TyCtxt<'tcx>) -> InstantiationMode {
-        let generate_cgu_internal_copies =
-            (tcx.sess.opts.optimize != OptLevel::No) && !tcx.sess.link_dead_code();
+        // The case handling here is written in the same style as cross_crate_inlinable, we first
+        // handle the cases where we must use a particular instantiation mode, then cascade down
+        // through a sequence of heuristics.
 
-        match *self {
-            MonoItem::Fn(ref instance) => {
-                let entry_def_id = tcx.entry_fn(()).map(|(id, _)| id);
-                // If this function isn't inlined or otherwise has an extern
-                // indicator, then we'll be creating a globally shared version.
-                let codegen_fn_attrs = tcx.codegen_fn_attrs(instance.def_id());
-                if codegen_fn_attrs.contains_extern_indicator()
-                    || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED)
-                    || !instance.def.generates_cgu_internal_copy(tcx)
-                    || Some(instance.def_id()) == entry_def_id
-                {
-                    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 this crate's build settings permit,
-                // we'll be creating a local copy per CGU.
-                if generate_cgu_internal_copies {
-                    return InstantiationMode::LocalCopy;
-                }
-
-                // Finally, if this is `#[inline(always)]` we're sure to respect
-                // that with an inline copy per CGU, but otherwise we'll be
-                // creating one copy of this `#[inline]` function which may
-                // conflict with upstream crates as it could be an exported
-                // symbol.
-                if tcx.codegen_fn_attrs(instance.def_id()).inline.always() {
-                    InstantiationMode::LocalCopy
-                } else {
-                    InstantiationMode::GloballyShared { may_conflict: true }
-                }
-            }
+        // The first thing we do is detect MonoItems which we must instantiate exactly once in the
+        // whole program.
+
+        // Statics and global_asm! must be instantiated exactly once.
+        let instance = match *self {
+            MonoItem::Fn(instance) => instance,
             MonoItem::Static(..) | MonoItem::GlobalAsm(..) => {
-                InstantiationMode::GloballyShared { may_conflict: false }
+                return InstantiationMode::GloballyShared { may_conflict: false };
             }
+        };
+
+        // Similarly, the executable entrypoint must be instantiated exactly once.
+        if let Some((entry_def_id, _)) = tcx.entry_fn(()) {
+            if instance.def_id() == entry_def_id {
+                return InstantiationMode::GloballyShared { may_conflict: false };
+            }
+        }
+
+        // If the function is #[naked] or contains any other attribute that requires exactly-once
+        // instantiation:
+        let codegen_fn_attrs = tcx.codegen_fn_attrs(instance.def_id());
+        if codegen_fn_attrs.contains_extern_indicator()
+            || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED)
+        {
+            return InstantiationMode::GloballyShared { may_conflict: false };
+        }
+
+        // We need to ensure that we do not decide the InstantiationMode of an exported symbol is
+        // LocalCopy. Since exported symbols are computed based on the output of
+        // cross_crate_inlinable, we are beholden to our previous decisions.
+        if !tcx.cross_crate_inlinable(instance.def_id()) {
+            return InstantiationMode::GloballyShared { may_conflict: false };
+        }
+
+        // Beginning of heuristics. The handling of link-dead-code and inline(always) are QoL only,
+        // the compiler should not crash and linkage should work, but codegen may be undesirable.
+
+        // -Clink-dead-code was given an unfortunate name; the point of the flag is to assist
+        // coverage tools which rely on having every function in the program appear in the
+        // generated code. If we select LocalCopy, functions which are not used because they are
+        // missing test coverage will disappear from such coverage reports, defeating the point.
+        // Note that -Cinstrument-coverage does not require such assistance from us, only coverage
+        // tools implemented without compiler support ironically require a special compiler flag.
+        if tcx.sess.link_dead_code() {
+            return InstantiationMode::GloballyShared { may_conflict: true };
+        }
+
+        // To ensure that #[inline(always)] can be inlined as much as possible, especially in unoptimized
+        // builds, we always select LocalCopy.
+        if codegen_fn_attrs.inline.always() {
+            return InstantiationMode::LocalCopy;
+        }
+
+        // #[inline(never)] functions in general are poor candidates for inlining and thus since
+        // LocalCopy generally increases code size for the benefit of optimizations from inlining,
+        // we want to give them GloballyShared codegen.
+        // The slight problem is that generic functions need to always support cross-crate
+        // compilation, so all previous stages of the compiler are obligated to treat generic
+        // functions the same as those that unconditionally get LocalCopy codegen. It's only when
+        // we get here that we can at least not codegen a #[inline(never)] generic function in all
+        // of our CGUs.
+        if let InlineAttr::Never = tcx.codegen_fn_attrs(instance.def_id()).inline
+            && self.is_generic_fn()
+        {
+            return InstantiationMode::GloballyShared { may_conflict: true };
+        }
+
+        // The fallthrough case is to generate LocalCopy for all optimized builds, and
+        // GloballyShared with conflict prevention when optimizations are disabled.
+        match tcx.sess.opts.optimize {
+            OptLevel::No => InstantiationMode::GloballyShared { may_conflict: true },
+            _ => InstantiationMode::LocalCopy,
         }
     }
 

compiler/rustc_middle/src/ty/instance.rs

@@ -301,50 +301,6 @@ impl<'tcx> InstanceKind<'tcx> {
         )
     }
 
-    /// Returns `true` if the machine code for this instance is instantiated in
-    /// each codegen unit that references it.
-    /// Note that this is only a hint! The compiler can globally decide to *not*
-    /// do this in order to speed up compilation. CGU-internal copies are
-    /// only exist to enable inlining. If inlining is not performed (e.g. at
-    /// `-Copt-level=0`) then the time for generating them is wasted and it's
-    /// better to create a single copy with external linkage.
-    pub fn generates_cgu_internal_copy(&self, tcx: TyCtxt<'tcx>) -> bool {
-        if self.requires_inline(tcx) {
-            return true;
-        }
-        if let ty::InstanceKind::DropGlue(.., Some(ty))
-        | ty::InstanceKind::AsyncDropGlueCtorShim(.., Some(ty)) = *self
-        {
-            // Drop glue generally wants to be instantiated at every codegen
-            // unit, but without an #[inline] hint. We should make this
-            // available to normal end-users.
-            if tcx.sess.opts.incremental.is_none() {
-                return true;
-            }
-            // When compiling with incremental, we can generate a *lot* of
-            // codegen units. Including drop glue into all of them has a
-            // considerable compile time cost.
-            //
-            // We include enums without destructors to allow, say, optimizing
-            // drops of `Option::None` before LTO. We also respect the intent of
-            // `#[inline]` on `Drop::drop` implementations.
-            return ty.ty_adt_def().is_none_or(|adt_def| {
-                match *self {
-                    ty::InstanceKind::DropGlue(..) => adt_def.destructor(tcx).map(|dtor| dtor.did),
-                    ty::InstanceKind::AsyncDropGlueCtorShim(..) => {
-                        adt_def.async_destructor(tcx).map(|dtor| dtor.ctor)
-                    }
-                    _ => unreachable!(),
-                }
-                .map_or_else(|| adt_def.is_enum(), |did| tcx.cross_crate_inlinable(did))
-            });
-        }
-        if let ty::InstanceKind::ThreadLocalShim(..) = *self {
-            return false;
-        }
-        tcx.cross_crate_inlinable(self.def_id())
-    }
-
     pub fn requires_caller_location(&self, tcx: TyCtxt<'_>) -> bool {
         match *self {
             InstanceKind::Item(def_id) | InstanceKind::Virtual(def_id, _) => {

compiler/rustc_mir_transform/src/cross_crate_inline.rs

@@ -75,6 +75,12 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         return false;
     }
 
+    if let Some(drop_in_place) = tcx.lang_items().drop_in_place_fn() {
+        if rustc_hir::def_id::DefId::from(def_id) == drop_in_place {
+            return true;
+        }
+    }
+
     if !tcx.is_mir_available(def_id) {
         return false;
     }