Skip to content

Commit

Permalink
Auto merge of #68414 - michaelwoerister:share-drop-glue, r=<try>
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Jan 22, 2020
2 parents 2f688ac + 9494974 commit d9ce420
Show file tree
Hide file tree
Showing 14 changed files with 140 additions and 121 deletions.
38 changes: 0 additions & 38 deletions src/librustc/middle/exported_symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::ty::subst::SubstsRef;
use crate::ty::{self, TyCtxt};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use std::cmp;
use std::mem;

/// The SymbolExportLevel of a symbols specifies from which kinds of crates
Expand Down Expand Up @@ -43,43 +42,6 @@ impl<'tcx> ExportedSymbol<'tcx> {
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 {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 };
Expand All @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,11 @@ rustc_queries! {
}
query upstream_monomorphizations_for(_: DefId)
-> Option<&'tcx FxHashMap<SubstsRef<'tcx>, CrateNum>> {}

query upstream_drop_glue_for(substs: SubstsRef<'tcx>) -> Option<CrateNum> {
desc { "available upstream drop-glue for `{:?}`", substs }
no_force
}
}

Other {
Expand Down
52 changes: 48 additions & 4 deletions src/librustc/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -91,6 +91,38 @@ impl<'tcx> Instance<'tcx> {
let ty = tcx.type_of(self.def.def_id());
tcx.subst_and_normalize_erasing_regions(self.substs, param_env, &ty)
}

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 instance has non-erasable parameters, it cannot be a shared
// monomorphization. Non-generic instances are already handled above
// by `is_reachable_non_generic()`.
if self.substs.non_erasable_generics().next().is_none() {
return None;
}

if self.def_id().is_local() {
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> {
Expand All @@ -114,7 +146,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,
Expand All @@ -127,8 +164,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 {
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/ty/query/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
7 changes: 1 addition & 6 deletions src/librustc_codegen_llvm/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
44 changes: 33 additions & 11 deletions src/librustc_codegen_ssa/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc::middle::exported_symbols::{metadata_symbol_name, ExportedSymbol, SymbolExportLevel};
use rustc::session::config;
use rustc::ty::query::Providers;
use rustc::ty::subst::SubstsRef;
use rustc::ty::subst::{GenericArgKind, SubstsRef};
use rustc::ty::Instance;
use rustc::ty::{SymbolName, TyCtxt};
use rustc_codegen_utils::symbol_names;
Expand All @@ -17,8 +17,6 @@ use rustc_hir::Node;
use rustc_index::vec::IndexVec;
use syntax::expand::allocator::ALLOCATOR_METHODS;

pub type ExportedSymbols = FxHashMap<CrateNum, Arc<Vec<(String, SymbolExportLevel)>>>;

pub fn threshold(tcx: TyCtxt<'_>) -> SymbolExportLevel {
crates_export_threshold(&tcx.sess.crate_types.borrow())
}
Expand Down Expand Up @@ -96,7 +94,7 @@ fn reachable_non_generics_provider(
if !generics.requires_monomorphization(tcx) &&
// Functions marked with #[inline] are only ever codegened
// with "internal" linkage and are never exported.
!Instance::mono(tcx, def_id).def.requires_local(tcx)
!Instance::mono(tcx, def_id).def.generates_cgu_internal_copy(tcx)
{
Some(def_id)
} else {
Expand Down Expand Up @@ -240,19 +238,31 @@ fn exported_symbols_provider_local(
continue;
}

if let &MonoItem::Fn(Instance { def: InstanceDef::Item(def_id), substs }) = mono_item {
if substs.non_erasable_generics().next().is_some() {
symbols
.push((ExportedSymbol::Generic(def_id, substs), SymbolExportLevel::Rust));
match *mono_item {
MonoItem::Fn(Instance { def: InstanceDef::Item(def_id), substs }) => {
if substs.non_erasable_generics().next().is_some() {
let symbol = ExportedSymbol::Generic(def_id, substs);
symbols.push((symbol, SymbolExportLevel::Rust));
}
}
MonoItem::Fn(Instance { def: InstanceDef::DropGlue(def_id, Some(ty)), substs }) => {
// A little sanity-check
debug_assert_eq!(
substs.non_erasable_generics().next(),
Some(GenericArgKind::Type(ty))
);
let symbol = ExportedSymbol::Generic(def_id, substs);
symbols.push((symbol, SymbolExportLevel::Rust));
}
_ => {
// Any other symbols don't qualify for sharing
}
}
}
}

// Sort so we get a stable incr. comp. hash.
symbols.sort_unstable_by(|&(ref symbol1, ..), &(ref symbol2, ..)| {
symbol1.compare_stable(tcx, symbol2)
});
symbols.sort_by_cached_key(|s| s.0.symbol_name_for_local_instance(tcx));

Arc::new(symbols)
}
Expand Down Expand Up @@ -311,6 +321,17 @@ fn upstream_monomorphizations_for_provider(
tcx.upstream_monomorphizations(LOCAL_CRATE).get(&def_id)
}

fn upstream_drop_glue_for_provider<'tcx>(
tcx: TyCtxt<'tcx>,
substs: SubstsRef<'tcx>,
) -> Option<CrateNum> {
if let Some(def_id) = tcx.lang_items().drop_in_place_fn() {
tcx.upstream_monomorphizations(LOCAL_CRATE).get(&def_id).and_then(|monos| monos.get(&substs).cloned())
} else {
None
}
}

fn is_unreachable_local_definition_provider(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
if let Some(hir_id) = tcx.hir().as_local_hir_id(def_id) {
!tcx.reachable_set(LOCAL_CRATE).contains(&hir_id)
Expand All @@ -325,6 +346,7 @@ pub fn provide(providers: &mut Providers<'_>) {
providers.exported_symbols = exported_symbols_provider_local;
providers.upstream_monomorphizations = upstream_monomorphizations_provider;
providers.is_unreachable_local_definition = is_unreachable_local_definition_provider;
providers.upstream_drop_glue_for = upstream_drop_glue_for_provider;
}

pub fn provide_extern(providers: &mut Providers<'_>) {
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use super::command::Command;
use super::link::{self, get_linker, remove};
use super::linker::LinkerInfo;
use super::lto::{self, SerializedModule};
use super::symbol_export::{symbol_name_for_instance_in_crate, ExportedSymbols};
use super::symbol_export::symbol_name_for_instance_in_crate;

use crate::{
CachedModuleCodegen, CodegenResults, CompiledModule, CrateInfo, ModuleCodegen, ModuleKind,
RLIB_BYTECODE_EXTENSION,
Expand All @@ -12,6 +13,7 @@ use crate::traits::*;
use jobserver::{Acquired, Client};
use rustc::dep_graph::{WorkProduct, WorkProductFileKind, WorkProductId};
use rustc::middle::cstore::EncodedMetadata;
use rustc::middle::exported_symbols::SymbolExportLevel;
use rustc::session::config::{
self, Lto, OutputFilenames, OutputType, Passes, Sanitizer, SwitchWithOptPath,
};
Expand Down Expand Up @@ -205,6 +207,8 @@ impl<B: WriteBackendMethods> Clone for TargetMachineFactory<B> {
}
}

pub type ExportedSymbols = FxHashMap<CrateNum, Arc<Vec<(String, SymbolExportLevel)>>>;

/// Additional resources used by optimize_and_codegen (not module specific)
#[derive(Clone)]
pub struct CodegenContext<B: WriteBackendMethods> {
Expand Down
20 changes: 1 addition & 19 deletions src/librustc_codegen_utils/symbol_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,26 +129,8 @@ fn symbol_name_provider(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> ty::Symb
//
// For generics we might find re-usable upstream instances. For anything
// else we rely on their being a local copy available.

if is_generic(instance.substs) {
let def_id = instance.def_id();

if !def_id.is_local() && tcx.sess.opts.share_generics() {
// If we are re-using a monomorphization from another crate,
// we have to compute the symbol hash accordingly.
let upstream_monomorphizations = tcx.upstream_monomorphizations_for(def_id);

upstream_monomorphizations
.and_then(|monos| monos.get(&instance.substs).cloned())
// If there is no instance available upstream, there'll be
// one in the current crate.
.unwrap_or(LOCAL_CRATE)
} else {
// For generic functions defined in the current crate, there
// can be no upstream instances. Also, if we don't share
// generics, we'll instantiate a local copy too.
LOCAL_CRATE
}
instance.upstream_monomorphization(tcx).unwrap_or(LOCAL_CRATE)
} else {
// For non-generic things that need to avoid naming conflicts, we
// always instantiate a copy in the local crate.
Expand Down
Loading

0 comments on commit d9ce420

Please sign in to comment.