trans: Adjust linkage assignment so that we don't need weak linkage.

Author: michaelwoerister

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,

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);
             }
         }

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
                     }
                 }
             }

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,
         }

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));
     }
 }

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

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()

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),
     }
 

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]<u32> @@ local_generic.volatile[WeakODR]
-//~ TRANS_ITEM fn local_generic::generic[0]<u64> @@ local_generic.volatile[WeakODR]
-//~ TRANS_ITEM fn local_generic::generic[0]<char> @@ local_generic.volatile[WeakODR]
-//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[WeakODR]
+//~ 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]
 pub fn generic<T>(x: T) -> T { x }
 
 //~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External]

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