Auto merge of #68414 - michaelwoerister:share-drop-glue, r=alexcrichton

Also share drop-glue when compiling with -Zshare-generics (i.e. at opt-level=0)

This PR adds drop-glue to the set of monomorphizations that can be shared across crates via `-Zshare-generics`.

This version of the PR might have detrimental effects on performance as it makes lots of stuff dependent on a single query results (`upstream_monomorphizations_for(def_id_of_drop_in_place)`). That should be fixable but let's do a perf run first.

Potentially fixes issue #64140. (cc @alexcrichton)
The changes here are related to @matthewjasper's #67332 but should be mostly orthogonal.

r? @ghost

Author: bors

src/librustc/middle/exported_symbols.rs

@@ -1,10 +1,7 @@
-use crate::ich::StableHashingContext;
 use crate::ty::subst::SubstsRef;
-use crate::ty::{self, TyCtxt};
-use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
+use crate::ty::{self, Ty, TyCtxt};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
-use std::cmp;
-use std::mem;
+use rustc_macros::HashStable;
 
 /// The SymbolExportLevel of a symbols specifies from which kinds of crates
 /// the symbol will be exported. `C` symbols will be exported from any
@@ -24,10 +21,11 @@ impl SymbolExportLevel {
     }
 }
 
-#[derive(Eq, PartialEq, Debug, Copy, Clone, RustcEncodable, RustcDecodable)]
+#[derive(Eq, PartialEq, Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)]
 pub enum ExportedSymbol<'tcx> {
     NonGeneric(DefId),
     Generic(DefId, SubstsRef<'tcx>),
+    DropGlue(Ty<'tcx>),
     NoDefId(ty::SymbolName),
 }
 
@@ -40,46 +38,12 @@ impl<'tcx> ExportedSymbol<'tcx> {
             ExportedSymbol::Generic(def_id, substs) => {
                 tcx.symbol_name(ty::Instance::new(def_id, substs))
             }
+            ExportedSymbol::DropGlue(ty) => {
+                tcx.symbol_name(ty::Instance::resolve_drop_in_place(tcx, ty))
+            }
             ExportedSymbol::NoDefId(symbol_name) => symbol_name,
         }
     }
-
-    pub fn compare_stable(&self, tcx: TyCtxt<'tcx>, other: &ExportedSymbol<'tcx>) -> cmp::Ordering {
-        match *self {
-            ExportedSymbol::NonGeneric(self_def_id) => match *other {
-                ExportedSymbol::NonGeneric(other_def_id) => {
-                    tcx.def_path_hash(self_def_id).cmp(&tcx.def_path_hash(other_def_id))
-                }
-                ExportedSymbol::Generic(..) | ExportedSymbol::NoDefId(_) => cmp::Ordering::Less,
-            },
-            ExportedSymbol::Generic(self_def_id, self_substs) => match *other {
-                ExportedSymbol::NonGeneric(_) => cmp::Ordering::Greater,
-                ExportedSymbol::Generic(other_def_id, other_substs) => {
-                    // We compare the symbol names because they are cached as query
-                    // results which makes them relatively cheap to access repeatedly.
-                    //
-                    // It might be even faster to build a local cache of stable IDs
-                    // for sorting. Exported symbols are really only sorted once
-                    // in order to make the `exported_symbols` query result stable.
-                    let self_symbol_name =
-                        tcx.symbol_name(ty::Instance::new(self_def_id, self_substs));
-                    let other_symbol_name =
-                        tcx.symbol_name(ty::Instance::new(other_def_id, other_substs));
-
-                    self_symbol_name.cmp(&other_symbol_name)
-                }
-                ExportedSymbol::NoDefId(_) => cmp::Ordering::Less,
-            },
-            ExportedSymbol::NoDefId(self_symbol_name) => match *other {
-                ExportedSymbol::NonGeneric(_) | ExportedSymbol::Generic(..) => {
-                    cmp::Ordering::Greater
-                }
-                ExportedSymbol::NoDefId(ref other_symbol_name) => {
-                    self_symbol_name.cmp(other_symbol_name)
-                }
-            },
-        }
-    }
 }
 
 pub fn metadata_symbol_name(tcx: TyCtxt<'_>) -> String {
@@ -89,21 +53,3 @@ pub fn metadata_symbol_name(tcx: TyCtxt<'_>) -> String {
         tcx.crate_disambiguator(LOCAL_CRATE).to_fingerprint().to_hex()
     )
 }
-
-impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for ExportedSymbol<'tcx> {
-    fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
-        mem::discriminant(self).hash_stable(hcx, hasher);
-        match *self {
-            ExportedSymbol::NonGeneric(def_id) => {
-                def_id.hash_stable(hcx, hasher);
-            }
-            ExportedSymbol::Generic(def_id, substs) => {
-                def_id.hash_stable(hcx, hasher);
-                substs.hash_stable(hcx, hasher);
-            }
-            ExportedSymbol::NoDefId(symbol_name) => {
-                symbol_name.hash_stable(hcx, hasher);
-            }
-        }
-    }
-}

src/librustc/mir/mono.rs

@@ -79,7 +79,7 @@ impl<'tcx> MonoItem<'tcx> {
     }
 
     pub fn instantiation_mode(&self, tcx: TyCtxt<'tcx>) -> InstantiationMode {
-        let inline_in_all_cgus = tcx
+        let generate_cgu_internal_copies = tcx
             .sess
             .opts
             .debugging_opts
@@ -93,7 +93,7 @@ impl<'tcx> MonoItem<'tcx> {
                 // If this function isn't inlined or otherwise has explicit
                 // linkage, then we'll be creating a globally shared version.
                 if self.explicit_linkage(tcx).is_some()
-                    || !instance.def.requires_local(tcx)
+                    || !instance.def.generates_cgu_internal_copy(tcx)
                     || Some(instance.def_id()) == entry_def_id
                 {
                     return InstantiationMode::GloballyShared { may_conflict: false };
@@ -102,7 +102,7 @@ impl<'tcx> MonoItem<'tcx> {
                 // 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
-                if inline_in_all_cgus {
+                if generate_cgu_internal_copies {
                     return InstantiationMode::LocalCopy;
                 }
 

src/librustc/query/mod.rs

@@ -776,13 +776,47 @@ rustc_queries! {
     }
 
     Codegen {
+        /// The entire set of monomorphizations the local crate can safely link
+        /// to because they are exported from upstream crates. Do not depend on
+        /// this directly, as its value changes anytime a monomorphization gets
+        /// added or removed in any upstream crate. Instead use the narrower
+        /// `upstream_monomorphizations_for`, `upstream_drop_glue_for`, or, even
+        /// better, `Instance::upstream_monomorphization()`.
         query upstream_monomorphizations(
             k: CrateNum
         ) -> &'tcx DefIdMap<FxHashMap<SubstsRef<'tcx>, CrateNum>> {
             desc { "collecting available upstream monomorphizations `{:?}`", k }
         }
+
+        /// Returns the set of upstream monomorphizations available for the
+        /// generic function identified by the given `def_id`. The query makes
+        /// sure to make a stable selection if the same monomorphization is
+        /// available in multiple upstream crates.
+        ///
+        /// You likely want to call `Instance::upstream_monomorphization()`
+        /// instead of invoking this query directly.
         query upstream_monomorphizations_for(_: DefId)
             -> Option<&'tcx FxHashMap<SubstsRef<'tcx>, CrateNum>> {}
+
+        /// Returns the upstream crate that exports drop-glue for the given
+        /// type (`substs` is expected to be a single-item list containing the
+        /// type one wants drop-glue for).
+        ///
+        /// This is a subset of `upstream_monomorphizations_for` in order to
+        /// increase dep-tracking granularity. Otherwise adding or removing any
+        /// type with drop-glue in any upstream crate would invalidate all
+        /// functions calling drop-glue of an upstream type.
+        ///
+        /// You likely want to call `Instance::upstream_monomorphization()`
+        /// instead of invoking this query directly.
+        ///
+        /// NOTE: This query could easily be extended to also support other
+        ///       common functions that have are large set of monomorphizations
+        ///       (like `Clone::clone` for example).
+        query upstream_drop_glue_for(substs: SubstsRef<'tcx>) -> Option<CrateNum> {
+            desc { "available upstream drop-glue for `{:?}`", substs }
+            no_force
+        }
     }
 
     Other {

src/librustc/ty/instance.rs

@@ -4,7 +4,7 @@ use crate::traits;
 use crate::ty::print::{FmtPrinter, Printer};
 use crate::ty::{self, SubstsRef, Ty, TyCtxt, TypeFoldable};
 use rustc_hir::def::Namespace;
-use rustc_hir::def_id::DefId;
+use rustc_hir::def_id::{CrateNum, DefId};
 use rustc_macros::HashStable;
 use rustc_target::spec::abi::Abi;
 
@@ -91,6 +91,40 @@ impl<'tcx> Instance<'tcx> {
         let ty = tcx.type_of(self.def.def_id());
         tcx.subst_and_normalize_erasing_regions(self.substs, param_env, &ty)
     }
+
+    /// Finds a crate that contains a monomorphization of this instance that
+    /// can be linked to from the local crate. A return value of `None` means
+    /// no upstream crate provides such an exported monomorphization.
+    ///
+    /// 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 this a non-generic instance, it cannot be a shared monomorphization.
+        if self.substs.non_erasable_generics().next().is_none() {
+            return None;
+        }
+
+        match self.def {
+            InstanceDef::Item(def_id) => tcx
+                .upstream_monomorphizations_for(def_id)
+                .and_then(|monos| monos.get(&self.substs).cloned()),
+            InstanceDef::DropGlue(_, Some(_)) => tcx.upstream_drop_glue_for(self.substs),
+            _ => None,
+        }
+    }
 }
 
 impl<'tcx> InstanceDef<'tcx> {
@@ -114,7 +148,12 @@ impl<'tcx> InstanceDef<'tcx> {
         tcx.get_attrs(self.def_id())
     }
 
-    pub fn is_inline(&self, tcx: TyCtxt<'tcx>) -> bool {
+    /// Returns `true` if the LLVM version of this instance is unconditionally
+    /// marked with `inline`. This implies that a copy of this instance is
+    /// generated in every codegen unit.
+    /// Note that this is only a hint. See the documentation for
+    /// `generates_cgu_internal_copy` for more information.
+    pub fn requires_inline(&self, tcx: TyCtxt<'tcx>) -> bool {
         use crate::hir::map::DefPathData;
         let def_id = match *self {
             ty::InstanceDef::Item(def_id) => def_id,
@@ -127,8 +166,15 @@ impl<'tcx> InstanceDef<'tcx> {
         }
     }
 
-    pub fn requires_local(&self, tcx: TyCtxt<'tcx>) -> bool {
-        if self.is_inline(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::InstanceDef::DropGlue(..) = *self {

src/librustc/ty/query/keys.rs

@@ -116,6 +116,15 @@ impl Key for (DefId, SimplifiedType) {
     }
 }
 
+impl<'tcx> Key for SubstsRef<'tcx> {
+    fn query_crate(&self) -> CrateNum {
+        LOCAL_CRATE
+    }
+    fn default_span(&self, _: TyCtxt<'_>) -> Span {
+        DUMMY_SP
+    }
+}
+
 impl<'tcx> Key for (DefId, SubstsRef<'tcx>) {
     fn query_crate(&self) -> CrateNum {
         self.0.krate

src/librustc_codegen_llvm/attributes.rs

@@ -246,7 +246,7 @@ pub fn from_fn_attrs(
     }
 
     // FIXME(eddyb) consolidate these two `inline` calls (and avoid overwrites).
-    if instance.def.is_inline(cx.tcx) {
+    if instance.def.requires_inline(cx.tcx) {
         inline(cx, llfn, attributes::InlineAttr::Hint);
     }
 

src/librustc_codegen_llvm/callee.rs

@@ -130,12 +130,7 @@ pub fn get_fn(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) -> &'ll Value
                     } else {
                         // This is a monomorphization of a generic function
                         // defined in an upstream crate.
-                        if cx
-                            .tcx
-                            .upstream_monomorphizations_for(instance_def_id)
-                            .map(|set| set.contains_key(instance.substs))
-                            .unwrap_or(false)
-                        {
+                        if instance.upstream_monomorphization(tcx).is_some() {
                             // This is instantiated in another crate. It cannot
                             // be `hidden`.
                         } else {