trans: Treat generics like regular functions, not like #[inline] functions during CGU partitioning.

This commit is contained in:
Michael Woerister 2017-01-09 09:52:08 -05:00
parent 02c7b117da
commit 5f90947c2c
9 changed files with 144 additions and 123 deletions

View File

@ -212,7 +212,7 @@ use glue::{self, DropGlueKind};
use monomorphize::{self, Instance};
use util::nodemap::{FxHashSet, FxHashMap, DefIdMap};
use trans_item::{TransItem, DefPathBasedNames};
use trans_item::{TransItem, DefPathBasedNames, InstantiationMode};
use std::iter;
@ -337,6 +337,10 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>,
}
TransItem::Static(node_id) => {
let def_id = scx.tcx().map.local_def_id(node_id);
// Sanity check whether this ended up being collected accidentally
debug_assert!(should_trans_locally(scx.tcx(), def_id));
let ty = scx.tcx().item_type(def_id);
let ty = glue::get_drop_glue_type(scx, ty);
neighbors.push(TransItem::DropGlue(DropGlueKind::Ty(ty)));
@ -346,6 +350,9 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>,
collect_neighbours(scx, Instance::mono(scx, def_id), &mut neighbors);
}
TransItem::Fn(instance) => {
// Sanity check whether this ended up being collected accidentally
debug_assert!(should_trans_locally(scx.tcx(), instance.def));
// Keep track of the monomorphization recursion depth
recursion_depth_reset = Some(check_recursion_limit(scx.tcx(),
instance,
@ -374,7 +381,7 @@ fn record_inlining_canditates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
callees: &[TransItem<'tcx>],
inlining_map: &mut InliningMap<'tcx>) {
let is_inlining_candidate = |trans_item: &TransItem<'tcx>| {
trans_item.needs_local_copy(tcx)
trans_item.instantiation_mode(tcx) == InstantiationMode::LocalCopy
};
let inlining_candidates = callees.into_iter()
@ -490,15 +497,16 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
.require(ExchangeMallocFnLangItem)
.unwrap_or_else(|e| self.scx.sess().fatal(&e));
assert!(can_have_local_instance(self.scx.tcx(), exchange_malloc_fn_def_id));
let empty_substs = self.scx.empty_substs_for_def_id(exchange_malloc_fn_def_id);
let exchange_malloc_fn_trans_item =
create_fn_trans_item(self.scx,
exchange_malloc_fn_def_id,
empty_substs,
self.param_substs);
if should_trans_locally(self.scx.tcx(), exchange_malloc_fn_def_id) {
let empty_substs = self.scx.empty_substs_for_def_id(exchange_malloc_fn_def_id);
let exchange_malloc_fn_trans_item =
create_fn_trans_item(self.scx,
exchange_malloc_fn_def_id,
empty_substs,
self.param_substs);
self.output.push(exchange_malloc_fn_trans_item);
self.output.push(exchange_malloc_fn_trans_item);
}
}
_ => { /* not interesting */ }
}
@ -609,7 +617,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
match tcx.item_type(def_id).sty {
ty::TyFnDef(def_id, _, f) => {
// Some constructors also have type TyFnDef but they are
// always instantiated inline and don't result in
// always instantiated inline and don't result in a
// translation item. Same for FFI functions.
if let Some(hir_map::NodeForeignItem(_)) = tcx.map.get_if_local(def_id) {
return false;
@ -625,7 +633,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
_ => return false
}
can_have_local_instance(tcx, def_id)
should_trans_locally(tcx, def_id)
}
}
@ -675,10 +683,27 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
}
}
fn can_have_local_instance<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> bool {
tcx.sess.cstore.can_have_local_instance(tcx, def_id)
// Returns true if we should translate an instance in the local crate.
// Returns false if we can just link to the upstream crate and therefore don't
// need a translation item.
fn should_trans_locally<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> bool {
if def_id.is_local() {
true
} else {
if tcx.sess.cstore.is_exported_symbol(def_id) ||
tcx.sess.cstore.is_foreign_item(def_id) {
// We can link to the item in question, no instance needed in this
// crate
false
} else {
if !tcx.sess.cstore.can_have_local_instance(tcx, def_id) {
bug!("Cannot create local trans-item for {:?}", def_id)
}
true
}
}
}
fn find_drop_glue_neighbors<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
@ -698,14 +723,15 @@ fn find_drop_glue_neighbors<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
// Make sure the BoxFreeFn lang-item gets translated if there is a boxed value.
if let ty::TyBox(content_type) = ty.sty {
let def_id = scx.tcx().require_lang_item(BoxFreeFnLangItem);
assert!(can_have_local_instance(scx.tcx(), def_id));
let box_free_fn_trans_item =
create_fn_trans_item(scx,
def_id,
scx.tcx().mk_substs(iter::once(Kind::from(content_type))),
scx.tcx().intern_substs(&[]));
output.push(box_free_fn_trans_item);
if should_trans_locally(scx.tcx(), def_id) {
let box_free_fn_trans_item =
create_fn_trans_item(scx,
def_id,
scx.tcx().mk_substs(iter::once(Kind::from(content_type))),
scx.tcx().intern_substs(&[]));
output.push(box_free_fn_trans_item);
}
}
// If the type implements Drop, also add a translation item for the
@ -735,7 +761,7 @@ fn find_drop_glue_neighbors<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
_ => bug!()
};
if can_have_local_instance(scx.tcx(), destructor_did) {
if should_trans_locally(scx.tcx(), destructor_did) {
let trans_item = create_fn_trans_item(scx,
destructor_did,
substs,
@ -1080,7 +1106,7 @@ fn create_trans_items_for_vtable_methods<'a, 'tcx>(scx: &SharedCrateContext<'a,
None
}
})
.filter(|&(def_id, _)| can_have_local_instance(scx.tcx(), def_id))
.filter(|&(def_id, _)| should_trans_locally(scx.tcx(), def_id))
.map(|(def_id, substs)| create_fn_trans_item(scx, def_id, substs, param_substs));
output.extend(methods);
}
@ -1255,7 +1281,7 @@ fn create_trans_items_for_default_impls<'a, 'tcx>(scx: &SharedCrateContext<'a, '
continue;
}
if can_have_local_instance(tcx, method.def_id) {
if should_trans_locally(tcx, method.def_id) {
let item = create_fn_trans_item(scx,
method.def_id,
callee_substs,

View File

@ -133,7 +133,7 @@ use std::sync::Arc;
use symbol_map::SymbolMap;
use syntax::ast::NodeId;
use syntax::symbol::{Symbol, InternedString};
use trans_item::TransItem;
use trans_item::{TransItem, InstantiationMode};
use util::nodemap::{FxHashMap, FxHashSet};
pub enum PartitioningStrategy {
@ -326,13 +326,15 @@ fn place_root_translation_items<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>,
let tcx = scx.tcx();
let mut roots = FxHashSet();
let mut codegen_units = FxHashMap();
let is_incremental_build = tcx.sess.opts.incremental.is_some();
for trans_item in trans_items {
let is_root = !trans_item.is_instantiated_only_on_demand(tcx);
let is_root = trans_item.instantiation_mode(tcx) == InstantiationMode::GloballyShared;
if is_root {
let characteristic_def_id = characteristic_def_id_of_trans_item(scx, trans_item);
let is_volatile = trans_item.is_generic_fn();
let is_volatile = is_incremental_build &&
trans_item.is_generic_fn();
let codegen_unit_name = match characteristic_def_id {
Some(def_id) => compute_codegen_unit_name(tcx, def_id, is_volatile),
@ -350,25 +352,9 @@ fn place_root_translation_items<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>,
Some(explicit_linkage) => explicit_linkage,
None => {
match trans_item {
TransItem::Fn(..) |
TransItem::Static(..) => llvm::ExternalLinkage,
TransItem::DropGlue(..) => unreachable!(),
// Is there any benefit to using ExternalLinkage?:
TransItem::Fn(ref instance) => {
if instance.substs.types().next().is_none() {
// This is a non-generic functions, we always
// make it visible externally on the chance that
// it might be used in another codegen unit.
// Later on base::internalize_symbols() will
// assign "internal" linkage to those symbols
// that are not referenced from other codegen
// units (and are not publicly visible).
llvm::ExternalLinkage
} else {
// In the current setup, generic functions cannot
// be roots.
unreachable!()
}
}
}
}
};
@ -448,29 +434,14 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit
if let Some(linkage) = codegen_unit.items.get(&trans_item) {
// This is a root, just copy it over
new_codegen_unit.items.insert(trans_item, *linkage);
} else if initial_partitioning.roots.contains(&trans_item) {
// This item will be instantiated in some other codegen unit,
// so we just add it here with AvailableExternallyLinkage
// FIXME(mw): I have not seen it happening yet but having
// available_externally here could potentially lead
// to the same problem with exception handling tables
// as in the case below.
new_codegen_unit.items.insert(trans_item,
llvm::AvailableExternallyLinkage);
} else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() {
// FIXME(mw): It would be nice if we could mark these as
// `AvailableExternallyLinkage`, since they should have
// been instantiated in the extern crate. But this
// sometimes leads to crashes on Windows because LLVM
// does not handle exception handling table instantiation
// reliably in that case.
new_codegen_unit.items.insert(trans_item, llvm::InternalLinkage);
} else {
// We can't be sure if this will also be instantiated
// somewhere else, so we add an instance here with
// InternalLinkage so we don't get any conflicts.
new_codegen_unit.items.insert(trans_item,
llvm::InternalLinkage);
if initial_partitioning.roots.contains(&trans_item) {
bug!("GloballyShared trans-item inlined into other CGU: \
{:?}", trans_item);
}
// This is a cgu-private copy
new_codegen_unit.items.insert(trans_item, llvm::InternalLinkage);
}
}

View File

@ -45,6 +45,18 @@ pub enum TransItem<'tcx> {
Static(NodeId)
}
/// Describes how a translation item will be instantiated in object files.
#[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
pub enum InstantiationMode {
/// There will be exactly one instance of the given TransItem. It will have
/// external linkage so that it can be linked to from other codegen units.
GloballyShared,
/// Each codegen unit containing a reference to the given TransItem will
/// have its own private copy of the function (with internal linkage).
LocalCopy,
}
impl<'a, 'tcx> TransItem<'tcx> {
pub fn define(&self, ccx: &CrateContext<'a, 'tcx>) {
@ -231,22 +243,21 @@ impl<'a, 'tcx> TransItem<'tcx> {
}
}
/// True if the translation item should only be translated to LLVM IR if
/// it is referenced somewhere (like inline functions, for example).
pub fn is_instantiated_only_on_demand(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool {
if self.explicit_linkage(tcx).is_some() {
return false;
}
pub fn instantiation_mode(&self,
tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> InstantiationMode {
match *self {
TransItem::Fn(ref instance) => {
!instance.def.is_local() ||
instance.substs.types().next().is_some() ||
common::is_closure(tcx, instance.def) ||
attr::requests_inline(&tcx.get_attrs(instance.def)[..])
if self.explicit_linkage(tcx).is_none() &&
(common::is_closure(tcx, instance.def) ||
attr::requests_inline(&tcx.get_attrs(instance.def)[..])) {
InstantiationMode::LocalCopy
} else {
InstantiationMode::GloballyShared
}
}
TransItem::DropGlue(..) => true,
TransItem::Static(..) => false,
TransItem::DropGlue(..) => InstantiationMode::LocalCopy,
TransItem::Static(..) => InstantiationMode::GloballyShared,
}
}
@ -260,18 +271,6 @@ impl<'a, 'tcx> TransItem<'tcx> {
}
}
/// Returns true if there has to be a local copy of this TransItem in every
/// codegen unit that references it (as with inlined functions, for example)
pub fn needs_local_copy(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool {
// Currently everything that is instantiated only on demand is done so
// with "internal" linkage, so we need a copy to be present in every
// codegen unit.
// This is coincidental: We could also instantiate something only if it
// is referenced (e.g. a regular, private function) but place it in its
// own codegen unit with "external" linkage.
self.is_instantiated_only_on_demand(tcx)
}
pub fn explicit_linkage(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option<llvm::Linkage> {
let def_id = match *self {
TransItem::Fn(ref instance) => instance.def,

View File

@ -57,8 +57,8 @@ mod mod3 {
}
// Make sure the two generic functions from the extern crate get instantiated
// privately in every module they are use in.
//~ TRANS_ITEM fn cgu_generic_function::foo[0]<&str> @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod2[Internal] extern_generic-mod1-mod1[Internal]
//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod2[Internal] extern_generic-mod1-mod1[Internal]
// once for the current crate
//~ TRANS_ITEM fn cgu_generic_function::foo[0]<&str> @@ cgu_generic_function.volatile[External]
//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ cgu_generic_function.volatile[External]
//~ TRANS_ITEM drop-glue i8

View File

@ -16,10 +16,10 @@
#![allow(dead_code)]
#![crate_type="lib"]
//~ TRANS_ITEM fn local_generic::generic[0]<u32> @@ local_generic[Internal]
//~ TRANS_ITEM fn local_generic::generic[0]<u64> @@ local_generic-mod1[Internal]
//~ TRANS_ITEM fn local_generic::generic[0]<char> @@ local_generic-mod1-mod1[Internal]
//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic-mod2[Internal]
//~ TRANS_ITEM fn local_generic::generic[0]<u32> @@ local_generic.volatile[External]
//~ TRANS_ITEM fn local_generic::generic[0]<u64> @@ local_generic.volatile[External]
//~ TRANS_ITEM fn local_generic::generic[0]<char> @@ local_generic.volatile[External]
//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[External]
pub fn generic<T>(x: T) -> T { x }
//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External]

View File

@ -74,19 +74,19 @@ fn main() {
// Since Trait1::do_something() is instantiated via its default implementation,
// it is considered a generic and is instantiated here only because it is
// referenced in this module.
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::Trait1[0]::do_something_else[0]<u32> @@ vtable_through_const[Internal]
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::Trait1[0]::do_something_else[0]<u32> @@ vtable_through_const-mod1.volatile[External]
// Although it is never used, Trait1::do_something_else() has to be
// instantiated locally here too, otherwise the <&u32 as &Trait1> vtable
// could not be fully constructed.
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::Trait1[0]::do_something[0]<u32> @@ vtable_through_const[Internal]
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::Trait1[0]::do_something[0]<u32> @@ vtable_through_const-mod1.volatile[External]
mod1::TRAIT1_REF.do_something();
// Same as above
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::{{impl}}[1]::do_something[0]<u8> @@ vtable_through_const[Internal]
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::{{impl}}[1]::do_something_else[0]<u8> @@ vtable_through_const[Internal]
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::{{impl}}[1]::do_something[0]<u8> @@ vtable_through_const-mod1.volatile[External]
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::{{impl}}[1]::do_something_else[0]<u8> @@ vtable_through_const-mod1.volatile[External]
mod1::TRAIT1_GEN_REF.do_something(0u8);
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::id[0]<char> @@ vtable_through_const[Internal]
//~ TRANS_ITEM fn vtable_through_const::mod1[0]::id[0]<char> @@ vtable_through_const-mod1.volatile[External]
mod1::ID_CHAR('x');
}

View File

@ -9,14 +9,32 @@
# 5. write the result into a file
dump-symbols = nm "$(TMPDIR)/lib$(1).rlib" \
| grep -E "some_test_function|Bar|bar" \
| grep -E "$(2)" \
| sed "s/.*\(_ZN.*E\).*/\1/" \
| sort \
> "$(TMPDIR)/$(1).nm"
> "$(TMPDIR)/$(1)$(3).nm"
# This test
# - compiles each of the two crates 2 times and makes sure each time we get
# exactly the same symbol names
# - makes sure that both crates agree on the same symbol names for monomorphic
# functions
all:
$(RUSTC) stable-symbol-names1.rs
$(call dump-symbols,stable_symbol_names1,generic_|mono_,_v1)
rm $(TMPDIR)/libstable_symbol_names1.rlib
$(RUSTC) stable-symbol-names1.rs
$(call dump-symbols,stable_symbol_names1,generic_|mono_,_v2)
cmp "$(TMPDIR)/stable_symbol_names1_v1.nm" "$(TMPDIR)/stable_symbol_names1_v2.nm"
$(RUSTC) stable-symbol-names2.rs
$(call dump-symbols,stable_symbol_names1)
$(call dump-symbols,stable_symbol_names2)
cmp "$(TMPDIR)/stable_symbol_names1.nm" "$(TMPDIR)/stable_symbol_names2.nm"
$(call dump-symbols,stable_symbol_names2,generic_|mono_,_v1)
rm $(TMPDIR)/libstable_symbol_names2.rlib
$(RUSTC) stable-symbol-names2.rs
$(call dump-symbols,stable_symbol_names2,generic_|mono_,_v2)
cmp "$(TMPDIR)/stable_symbol_names2_v1.nm" "$(TMPDIR)/stable_symbol_names2_v2.nm"
$(call dump-symbols,stable_symbol_names1,mono_,_cross)
$(call dump-symbols,stable_symbol_names2,mono_,_cross)
cmp "$(TMPDIR)/stable_symbol_names1_cross.nm" "$(TMPDIR)/stable_symbol_names2_cross.nm"

View File

@ -11,26 +11,31 @@
#![crate_type="rlib"]
pub trait Foo {
fn foo<T>();
fn generic_method<T>();
}
pub struct Bar;
impl Foo for Bar {
fn foo<T>() {}
fn generic_method<T>() {}
}
pub fn bar() {
Bar::foo::<Bar>();
pub fn mono_function() {
Bar::generic_method::<Bar>();
}
pub fn some_test_function<T>(t: T) -> T {
pub fn mono_function_lifetime<'a>(x: &'a u64) -> u64 {
*x
}
pub fn generic_function<T>(t: T) -> T {
t
}
pub fn user() {
some_test_function(0u32);
some_test_function("abc");
generic_function(0u32);
generic_function("abc");
let x = 2u64;
some_test_function(&x);
generic_function(&x);
let _ = mono_function_lifetime(&x);
}

View File

@ -13,14 +13,16 @@
extern crate stable_symbol_names1;
pub fn user() {
stable_symbol_names1::some_test_function(1u32);
stable_symbol_names1::some_test_function("def");
stable_symbol_names1::generic_function(1u32);
stable_symbol_names1::generic_function("def");
let x = 2u64;
stable_symbol_names1::some_test_function(&x);
stable_symbol_names1::generic_function(&x);
stable_symbol_names1::mono_function();
stable_symbol_names1::mono_function_lifetime(&0);
}
pub fn trait_impl_test_function() {
use stable_symbol_names1::*;
Bar::foo::<Bar>();
bar();
Bar::generic_method::<Bar>();
}