From 1c03bfe3b43c06bc439c5369a180958eb4360361 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 10 Jun 2016 19:06:21 -0400 Subject: [PATCH] trans: Adjust linkage assignment so that we don't need weak linkage. --- src/librustc_trans/glue.rs | 2 +- src/librustc_trans/monomorphize.rs | 2 +- src/librustc_trans/partitioning.rs | 88 ++++++++++++------- src/librustc_trans/trans_item.rs | 11 ++- .../partitioning/extern-drop-glue.rs | 8 +- .../partitioning/extern-generic.rs | 4 +- .../inlining-from-extern-crate.rs | 4 +- .../partitioning/local-drop-glue.rs | 10 +-- .../partitioning/local-generic.rs | 11 +-- .../methods-are-with-self-type.rs | 5 ++ 10 files changed, 85 insertions(+), 60 deletions(-) diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 87a1d58443374..ef7d0ea165d60 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -255,7 +255,7 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // on MIR. Thus, we'll instantiate the missing function on demand in // this codegen unit, so that things keep working. - TransItem::DropGlue(g).predefine(ccx, llvm::LinkOnceODRLinkage); + TransItem::DropGlue(g).predefine(ccx, llvm::InternalLinkage); TransItem::DropGlue(g).define(ccx); // Now that we made sure that the glue function is in ccx.drop_glues, diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index a97e3e5a78dba..00c0e91103500 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -137,7 +137,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ccx.stats().n_fallback_instantiations.set(ccx.stats() .n_fallback_instantiations .get() + 1); - trans_item.predefine(ccx, llvm::PrivateLinkage); + trans_item.predefine(ccx, llvm::InternalLinkage); trans_item.define(ccx); } } diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index edf5db81b1843..8073359ede87e 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -265,15 +265,11 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let mut codegen_units = FnvHashMap(); for trans_item in trans_items { - let is_root = match trans_item { - TransItem::Static(..) => true, - TransItem::DropGlue(..) => false, - TransItem::Fn(_) => !trans_item.is_from_extern_crate(), - }; + let is_root = !trans_item.is_instantiated_only_on_demand(); if is_root { let characteristic_def_id = characteristic_def_id_of_trans_item(tcx, trans_item); - let is_volatile = trans_item.is_lazily_instantiated(); + let is_volatile = 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), @@ -304,9 +300,9 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // it might be used in another codegen unit. llvm::ExternalLinkage } else { - // Monomorphizations of generic functions are - // always weak-odr - llvm::WeakODRLinkage + // In the current setup, generic functions cannot + // be roots. + unreachable!() } } } @@ -395,25 +391,30 @@ 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 { - 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 - new_codegen_unit.items.insert(trans_item, - llvm::AvailableExternallyLinkage); - } else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() { - // An instantiation of this item is always available in the - // crate it was imported from. - new_codegen_unit.items.insert(trans_item, - llvm::AvailableExternallyLinkage); - } else { - // We can't be sure if this will also be instantiated - // somewhere else, so we add an instance here with - // LinkOnceODRLinkage. That way the item can be discarded if - // it's not needed (inlined) after all. - new_codegen_unit.items.insert(trans_item, - llvm::LinkOnceODRLinkage); - } + assert!(trans_item.is_instantiated_only_on_demand()); + // 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); } } @@ -521,17 +522,36 @@ fn single_codegen_unit<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } TransItem::DropGlue(_) => { - llvm::PrivateLinkage + llvm::InternalLinkage } TransItem::Fn(instance) => { - if trans_item.is_generic_fn() || - trans_item.is_from_extern_crate() || - !reachable.contains(&tcx.map - .as_local_node_id(instance.def) - .unwrap()) { + if trans_item.is_generic_fn() { + // FIXME(mw): Assigning internal linkage to all + // monomorphizations is potentially a waste of space + // since monomorphizations could be shared between + // crates. The main reason for making them internal is + // a limitation in MingW's binutils that cannot deal + // with COFF object that have more than 2^15 sections, + // which is something that can happen for large programs + // when every function gets put into its own COMDAT + // section. llvm::InternalLinkage - } else { + } else if trans_item.is_from_extern_crate() { + // 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. + llvm::InternalLinkage + } else if reachable.contains(&tcx.map + .as_local_node_id(instance.def) + .unwrap()) { llvm::ExternalLinkage + } else { + // Functions that are not visible outside this crate can + // be marked as internal. + llvm::InternalLinkage } } } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index b9700f2cacfc2..b7b18b2631bee 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -256,8 +256,10 @@ impl<'a, 'tcx> TransItem<'tcx> { pub fn requests_inline(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { match *self { TransItem::Fn(ref instance) => { - let attributes = tcx.get_attrs(instance.def); - attr::requests_inline(&attributes[..]) + !instance.substs.types.is_empty() || { + let attributes = tcx.get_attrs(instance.def); + attr::requests_inline(&attributes[..]) + } } TransItem::DropGlue(..) => true, TransItem::Static(..) => false, @@ -272,9 +274,10 @@ impl<'a, 'tcx> TransItem<'tcx> { } } - pub fn is_lazily_instantiated(&self) -> bool { + pub fn is_instantiated_only_on_demand(&self) -> bool { match *self { - TransItem::Fn(ref instance) => !instance.substs.types.is_empty(), + TransItem::Fn(ref instance) => !instance.def.is_local() || + !instance.substs.types.is_empty(), TransItem::DropGlue(..) => true, TransItem::Static(..) => false, } diff --git a/src/test/codegen-units/partitioning/extern-drop-glue.rs b/src/test/codegen-units/partitioning/extern-drop-glue.rs index 7072a211d2401..910ffd2959ed0 100644 --- a/src/test/codegen-units/partitioning/extern-drop-glue.rs +++ b/src/test/codegen-units/partitioning/extern-drop-glue.rs @@ -20,15 +20,15 @@ // aux-build:cgu_extern_drop_glue.rs extern crate cgu_extern_drop_glue; -//~ TRANS_ITEM drop-glue cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[OnceODR] extern_drop_glue-mod1[OnceODR] -//~ TRANS_ITEM drop-glue-contents cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[OnceODR] extern_drop_glue-mod1[OnceODR] +//~ TRANS_ITEM drop-glue cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[Internal] extern_drop_glue-mod1[Internal] +//~ TRANS_ITEM drop-glue-contents cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[Internal] extern_drop_glue-mod1[Internal] struct LocalStruct(cgu_extern_drop_glue::Struct); //~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External] fn user() { - //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[OnceODR] + //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[Internal] let _ = LocalStruct(cgu_extern_drop_glue::Struct(0)); } @@ -40,7 +40,7 @@ mod mod1 { //~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External] fn user() { - //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[OnceODR] + //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[Internal] let _ = LocalStruct(cgu_extern_drop_glue::Struct(0)); } } diff --git a/src/test/codegen-units/partitioning/extern-generic.rs b/src/test/codegen-units/partitioning/extern-generic.rs index 5801727494f7c..58f904f48a17d 100644 --- a/src/test/codegen-units/partitioning/extern-generic.rs +++ b/src/test/codegen-units/partitioning/extern-generic.rs @@ -58,7 +58,7 @@ 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[OnceODR] extern_generic-mod1[OnceODR] extern_generic-mod2[OnceODR] extern_generic-mod1-mod1[OnceODR] -//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ extern_generic[OnceODR] extern_generic-mod1[OnceODR] extern_generic-mod2[OnceODR] extern_generic-mod1-mod1[OnceODR] +//~ 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] //~ TRANS_ITEM drop-glue i8 diff --git a/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs b/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs index 285b068fe1c6c..118513f65541b 100644 --- a/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs +++ b/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs @@ -21,8 +21,8 @@ extern crate cgu_explicit_inlining; // This test makes sure that items inlined from external crates are privately // instantiated in every codegen unit they are used in. -//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod1[Available] -//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod2[Available] +//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[Internal] inlining_from_extern_crate-mod1[Internal] +//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[Internal] inlining_from_extern_crate-mod2[Internal] //~ TRANS_ITEM fn inlining_from_extern_crate::user[0] @@ inlining_from_extern_crate[External] pub fn user() diff --git a/src/test/codegen-units/partitioning/local-drop-glue.rs b/src/test/codegen-units/partitioning/local-drop-glue.rs index dc50650de6d43..f61e3fe12931e 100644 --- a/src/test/codegen-units/partitioning/local-drop-glue.rs +++ b/src/test/codegen-units/partitioning/local-drop-glue.rs @@ -16,8 +16,8 @@ #![allow(dead_code)] #![crate_type="lib"] -//~ TRANS_ITEM drop-glue local_drop_glue::Struct[0] @@ local_drop_glue[OnceODR] local_drop_glue-mod1[OnceODR] -//~ TRANS_ITEM drop-glue-contents local_drop_glue::Struct[0] @@ local_drop_glue[OnceODR] local_drop_glue-mod1[OnceODR] +//~ TRANS_ITEM drop-glue local_drop_glue::Struct[0] @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal] +//~ TRANS_ITEM drop-glue-contents local_drop_glue::Struct[0] @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal] struct Struct { _a: u32 } @@ -27,7 +27,7 @@ impl Drop for Struct { fn drop(&mut self) {} } -//~ TRANS_ITEM drop-glue local_drop_glue::Outer[0] @@ local_drop_glue[OnceODR] +//~ TRANS_ITEM drop-glue local_drop_glue::Outer[0] @@ local_drop_glue[Internal] struct Outer { _a: Struct } @@ -46,10 +46,10 @@ mod mod1 { use super::Struct; - //~ TRANS_ITEM drop-glue local_drop_glue::mod1[0]::Struct2[0] @@ local_drop_glue-mod1[OnceODR] + //~ TRANS_ITEM drop-glue local_drop_glue::mod1[0]::Struct2[0] @@ local_drop_glue-mod1[Internal] struct Struct2 { _a: Struct, - //~ TRANS_ITEM drop-glue (u32, local_drop_glue::Struct[0]) @@ local_drop_glue-mod1[OnceODR] + //~ TRANS_ITEM drop-glue (u32, local_drop_glue::Struct[0]) @@ local_drop_glue-mod1[Internal] _b: (u32, Struct), } diff --git a/src/test/codegen-units/partitioning/local-generic.rs b/src/test/codegen-units/partitioning/local-generic.rs index bfc911e36e9a8..2d744169d3f8e 100644 --- a/src/test/codegen-units/partitioning/local-generic.rs +++ b/src/test/codegen-units/partitioning/local-generic.rs @@ -16,13 +16,10 @@ #![allow(dead_code)] #![crate_type="lib"] -// Used in different modules/codegen units but always instantiated in the same -// codegen unit. - -//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic.volatile[WeakODR] -//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic.volatile[WeakODR] -//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic.volatile[WeakODR] -//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[WeakODR] +//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic[Internal] +//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic-mod1[Internal] +//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic-mod1-mod1[Internal] +//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic-mod2[Internal] pub fn generic(x: T) -> T { x } //~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External] diff --git a/src/test/codegen-units/partitioning/methods-are-with-self-type.rs b/src/test/codegen-units/partitioning/methods-are-with-self-type.rs index f8e7d8d255465..1ea5aafd401d2 100644 --- a/src/test/codegen-units/partitioning/methods-are-with-self-type.rs +++ b/src/test/codegen-units/partitioning/methods-are-with-self-type.rs @@ -8,6 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Currently, all generic functions are instantiated in each codegen unit that +// uses them, even those not marked with #[inline], so this test does not make +// much sense at the moment. +// ignore-test + // ignore-tidy-linelength // We specify -Z incremental here because we want to test the partitioning for // incremental compilation