Skip to content

Commit

Permalink
trans: Adjust linkage assignment so that we don't need weak linkage.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelwoerister committed Jul 8, 2016
1 parent 051d391 commit 1c03bfe
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/librustc_trans/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/monomorphize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
88 changes: 54 additions & 34 deletions src/librustc_trans/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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!()
}
}
}
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/librustc_trans/trans_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/codegen-units/partitioning/extern-drop-glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand All @@ -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));
}
}
4 changes: 2 additions & 2 deletions src/test/codegen-units/partitioning/extern-generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions src/test/codegen-units/partitioning/local-drop-glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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),
}

Expand Down
11 changes: 4 additions & 7 deletions src/test/codegen-units/partitioning/local-generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1c03bfe

Please sign in to comment.