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

michaelwoerister · michaelwoerister · commit 9b55c5e52457 · 2016-06-28T16:02:42.000-04:00
diff --git 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
@@ -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
@@ -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,26 @@ 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
+                new_codegen_unit.items.insert(trans_item,
+                                              llvm::AvailableExternallyLinkage);
+            } else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() {
+                // 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 +518,25 @@ 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() {
                         llvm::InternalLinkage
-                    } else {
+                    } else if trans_item.is_from_extern_crate() {
+                        // 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 {
+                        llvm::InternalLinkage
                     }
                 }
             }
diff --git 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
@@ -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
@@ -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/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
@@ -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]
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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,`
`137`	`137`	`ccx.stats().n_fallback_instantiations.set(ccx.stats()`
`138`	`138`	`.n_fallback_instantiations`
`139`	`139`	`.get() + 1);`
`140`		`- trans_item.predefine(ccx, llvm::PrivateLinkage);`
	`140`	`+ trans_item.predefine(ccx, llvm::InternalLinkage);`
`141`	`141`	`trans_item.define(ccx);`
`142`	`142`	`}`
`143`	`143`	`}`
Original file line number	Diff line number	Diff line change
`@@ -256,8 +256,10 @@ impl<'a, 'tcx> TransItem<'tcx> {`
`256`	`256`	`pub fn requests_inline(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool {`
`257`	`257`	`match *self {`
`258`	`258`	`TransItem::Fn(ref instance) => {`
`259`		`- let attributes = tcx.get_attrs(instance.def);`
`260`		`- attr::requests_inline(&attributes[..])`
	`259`	`+ !instance.substs.types.is_empty() \|\| {`
	`260`	`+ let attributes = tcx.get_attrs(instance.def);`
	`261`	`+ attr::requests_inline(&attributes[..])`
	`262`	`+ }`
`261`	`263`	`}`
`262`	`264`	`TransItem::DropGlue(..) => true,`
`263`	`265`	`TransItem::Static(..) => false,`
`@@ -272,9 +274,10 @@ impl<'a, 'tcx> TransItem<'tcx> {`
`272`	`274`	`}`
`273`	`275`	`}`
`274`	`276`
`275`		`- pub fn is_lazily_instantiated(&self) -> bool {`
	`277`	`+ pub fn is_instantiated_only_on_demand(&self) -> bool {`
`276`	`278`	`match *self {`
`277`		`- TransItem::Fn(ref instance) => !instance.substs.types.is_empty(),`
	`279`	`+ TransItem::Fn(ref instance) => !instance.def.is_local() \|\|`
	`280`	`+ !instance.substs.types.is_empty(),`
`278`	`281`	`TransItem::DropGlue(..) => true,`
`279`	`282`	`TransItem::Static(..) => false,`
`280`	`283`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,15 +20,15 @@`
`20`	`20`	`// aux-build:cgu_extern_drop_glue.rs`
`21`	`21`	`extern crate cgu_extern_drop_glue;`
`22`	`22`
`23`		`-//~ TRANS_ITEM drop-glue cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[OnceODR] extern_drop_glue-mod1[OnceODR]`
`24`		`-//~ TRANS_ITEM drop-glue-contents cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[OnceODR] extern_drop_glue-mod1[OnceODR]`
	`23`	`+//~ TRANS_ITEM drop-glue cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[Internal] extern_drop_glue-mod1[Internal]`
	`24`	`+//~ TRANS_ITEM drop-glue-contents cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[Internal] extern_drop_glue-mod1[Internal]`
`25`	`25`
`26`	`26`	`struct LocalStruct(cgu_extern_drop_glue::Struct);`
`27`	`27`
`28`	`28`	`//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External]`
`29`	`29`	`fn user()`
`30`	`30`	`{`
`31`		`- //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[OnceODR]`
	`31`	`+ //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[Internal]`
`32`	`32`	`let _ = LocalStruct(cgu_extern_drop_glue::Struct(0));`
`33`	`33`	`}`
`34`	`34`
`@@ -40,7 +40,7 @@ mod mod1 {`
`40`	`40`	`//~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External]`
`41`	`41`	`fn user()`
`42`	`42`	`{`
`43`		`- //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[OnceODR]`
	`43`	`+ //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[Internal]`
`44`	`44`	`let _ = LocalStruct(cgu_extern_drop_glue::Struct(0));`
`45`	`45`	`}`
`46`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,8 +16,8 @@`
`16`	`16`	`#![allow(dead_code)]`
`17`	`17`	`#![crate_type="lib"]`
`18`	`18`
`19`		`-//~ TRANS_ITEM drop-glue local_drop_glue::Struct[0] @@ local_drop_glue[OnceODR] local_drop_glue-mod1[OnceODR]`
`20`		`-//~ TRANS_ITEM drop-glue-contents local_drop_glue::Struct[0] @@ local_drop_glue[OnceODR] local_drop_glue-mod1[OnceODR]`
	`19`	`+//~ TRANS_ITEM drop-glue local_drop_glue::Struct[0] @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal]`
	`20`	`+//~ TRANS_ITEM drop-glue-contents local_drop_glue::Struct[0] @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal]`
`21`	`21`	`struct Struct {`
`22`	`22`	`_a: u32`
`23`	`23`	`}`
`@@ -27,7 +27,7 @@ impl Drop for Struct {`
`27`	`27`	`fn drop(&mut self) {}`
`28`	`28`	`}`
`29`	`29`
`30`		`-//~ TRANS_ITEM drop-glue local_drop_glue::Outer[0] @@ local_drop_glue[OnceODR]`
	`30`	`+//~ TRANS_ITEM drop-glue local_drop_glue::Outer[0] @@ local_drop_glue[Internal]`
`31`	`31`	`struct Outer {`
`32`	`32`	`_a: Struct`
`33`	`33`	`}`
`@@ -46,10 +46,10 @@ mod mod1`
`46`	`46`	`{`
`47`	`47`	`use super::Struct;`
`48`	`48`
`49`		`- //~ TRANS_ITEM drop-glue local_drop_glue::mod1[0]::Struct2[0] @@ local_drop_glue-mod1[OnceODR]`
	`49`	`+ //~ TRANS_ITEM drop-glue local_drop_glue::mod1[0]::Struct2[0] @@ local_drop_glue-mod1[Internal]`
`50`	`50`	`struct Struct2 {`
`51`	`51`	`_a: Struct,`
`52`		`- //~ TRANS_ITEM drop-glue (u32, local_drop_glue::Struct[0]) @@ local_drop_glue-mod1[OnceODR]`
	`52`	`+ //~ TRANS_ITEM drop-glue (u32, local_drop_glue::Struct[0]) @@ local_drop_glue-mod1[Internal]`
`53`	`53`	`_b: (u32, Struct),`
`54`	`54`	`}`
`55`	`55`