Skip to content

Commit e5d744e

Browse files
michaelwoeristermichaelwoerister
committed
Improve linkage assignment in trans::partitioning.
1 parent 6fa32f1 commit e5d744e

File tree

13 files changed

+146
-69
lines changed

13 files changed

+146
-69
lines changed

‎src/librustc_trans/base.rs‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2555,8 +2555,8 @@ fn iter_functions(llmod: llvm::ModuleRef) -> ValueIter {
25552555
///
25562556
/// This list is later used by linkers to determine the set of symbols needed to
25572557
/// be exposed from a dynamic library and it's also encoded into the metadata.
2558-
pub fn filter_reachable_ids(scx: &SharedCrateContext) -> NodeSet {
2559-
scx.reachable().iter().map(|x| *x).filter(|&id| {
2558+
pub fn filter_reachable_ids(tcx: TyCtxt, reachable: NodeSet) -> NodeSet {
2559+
reachable.into_iter().filter(|&id| {
25602560
// Next, we want to ignore some FFI functions that are not exposed from
25612561
// this crate. Reachable FFI functions can be lumped into two
25622562
// categories:
@@ -2570,9 +2570,9 @@ pub fn filter_reachable_ids(scx: &SharedCrateContext) -> NodeSet {
25702570
//
25712571
// As a result, if this id is an FFI item (foreign item) then we only
25722572
// let it through if it's included statically.
2573-
match scx.tcx().map.get(id) {
2573+
match tcx.map.get(id) {
25742574
hir_map::NodeForeignItem(..) => {
2575-
scx.sess().cstore.is_statically_included_foreign_item(id)
2575+
tcx.sess.cstore.is_statically_included_foreign_item(id)
25762576
}
25772577

25782578
// Only consider nodes that actually have exported symbols.
@@ -2582,8 +2582,8 @@ pub fn filter_reachable_ids(scx: &SharedCrateContext) -> NodeSet {
25822582
node: hir::ItemFn(..), .. }) |
25832583
hir_map::NodeImplItem(&hir::ImplItem {
25842584
node: hir::ImplItemKind::Method(..), .. }) => {
2585-
let def_id = scx.tcx().map.local_def_id(id);
2586-
let scheme = scx.tcx().lookup_item_type(def_id);
2585+
let def_id = tcx.map.local_def_id(id);
2586+
let scheme = tcx.lookup_item_type(def_id);
25872587
scheme.generics.types.is_empty()
25882588
}
25892589

@@ -2605,6 +2605,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
26052605
let krate = tcx.map.krate();
26062606

26072607
let ty::CrateAnalysis { export_map, reachable, name, .. } = analysis;
2608+
let reachable = filter_reachable_ids(tcx, reachable);
26082609

26092610
let check_overflow = if let Some(v) = tcx.sess.opts.debugging_opts.force_overflow_checks {
26102611
v
@@ -2628,12 +2629,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
26282629
reachable,
26292630
check_overflow,
26302631
check_dropflag);
2631-
2632-
let reachable_symbol_ids = filter_reachable_ids(&shared_ccx);
2633-
26342632
// Translate the metadata.
26352633
let metadata = time(tcx.sess.time_passes(), "write metadata", || {
2636-
write_metadata(&shared_ccx, &reachable_symbol_ids)
2634+
write_metadata(&shared_ccx, shared_ccx.reachable())
26372635
});
26382636

26392637
let metadata_module = ModuleTranslation {
@@ -2762,7 +2760,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
27622760
}
27632761

27642762
let sess = shared_ccx.sess();
2765-
let mut reachable_symbols = reachable_symbol_ids.iter().map(|&id| {
2763+
let mut reachable_symbols = shared_ccx.reachable().iter().map(|&id| {
27662764
let def_id = shared_ccx.tcx().map.local_def_id(id);
27672765
Instance::mono(&shared_ccx, def_id).symbol_name(&shared_ccx)
27682766
}).collect::<Vec<_>>();
@@ -2918,7 +2916,8 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a
29182916
partitioning::partition(scx.tcx(),
29192917
items.iter().cloned(),
29202918
strategy,
2921-
&inlining_map)
2919+
&inlining_map,
2920+
scx.reachable())
29222921
});
29232922

29242923
if scx.sess().opts.debugging_opts.print_trans_items.is_some() {

‎src/librustc_trans/partitioning.rs‎

Lines changed: 82 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ use rustc::ty::TyCtxt;
126126
use rustc::ty::item_path::characteristic_def_id_of_type;
127127
use syntax::parse::token::{self, InternedString};
128128
use trans_item::TransItem;
129-
use util::nodemap::{FnvHashMap, FnvHashSet};
129+
use util::nodemap::{FnvHashMap, FnvHashSet, NodeSet};
130130

131131
pub struct CodegenUnit<'tcx> {
132132
pub name: InternedString,
@@ -147,14 +147,23 @@ const FALLBACK_CODEGEN_UNIT: &'static str = "__rustc_fallback_codegen_unit";
147147
pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
148148
trans_items: I,
149149
strategy: PartitioningStrategy,
150-
inlining_map: &InliningMap<'tcx>)
150+
inlining_map: &InliningMap<'tcx>,
151+
reachable: &NodeSet)
151152
-> Vec<CodegenUnit<'tcx>>
152153
where I: Iterator<Item = TransItem<'tcx>>
153154
{
155+
if let PartitioningStrategy::FixedUnitCount(1) = strategy {
156+
// If there is only a single codegen-unit, we can use a very simple
157+
// scheme and don't have to bother with doing much analysis.
158+
return vec![single_codegen_unit(tcx, trans_items, reachable)];
159+
}
160+
154161
// In the first step, we place all regular translation items into their
155162
// respective 'home' codegen unit. Regular translation items are all
156163
// functions and statics defined in the local crate.
157-
let mut initial_partitioning = place_root_translation_items(tcx, trans_items);
164+
let mut initial_partitioning = place_root_translation_items(tcx,
165+
trans_items,
166+
reachable);
158167

159168
// If the partitioning should produce a fixed count of codegen units, merge
160169
// until that count is reached.
@@ -179,7 +188,8 @@ struct PreInliningPartitioning<'tcx> {
179188
struct PostInliningPartitioning<'tcx>(Vec<CodegenUnit<'tcx>>);
180189

181190
fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
182-
trans_items: I)
191+
trans_items: I,
192+
_reachable: &NodeSet)
183193
-> PreInliningPartitioning<'tcx>
184194
where I: Iterator<Item = TransItem<'tcx>>
185195
{
@@ -219,7 +229,18 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
219229
TransItem::Static(..) => llvm::ExternalLinkage,
220230
TransItem::DropGlue(..) => unreachable!(),
221231
// Is there any benefit to using ExternalLinkage?:
222-
TransItem::Fn(..) => llvm::WeakODRLinkage,
232+
TransItem::Fn(ref instance) => {
233+
if instance.substs.types.is_empty() {
234+
// This is a non-generic functions, we always
235+
// make it visible externally on the chance that
236+
// it might be used in another codegen unit.
237+
llvm::ExternalLinkage
238+
} else {
239+
// Monomorphizations of generic functions are
240+
// always weak-odr
241+
llvm::WeakODRLinkage
242+
}
243+
}
223244
}
224245
}
225246
};
@@ -282,13 +303,6 @@ fn merge_codegen_units<'tcx>(initial_partitioning: &mut PreInliningPartitioning<
282303
items: FnvHashMap()
283304
});
284305
}
285-
286-
fn numbered_codegen_unit_name(crate_name: &str, index: usize) -> InternedString {
287-
token::intern_and_get_ident(&format!("{}{}{}",
288-
crate_name,
289-
NUMBERED_CODEGEN_UNIT_MARKER,
290-
index)[..])
291-
}
292306
}
293307

294308
fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartitioning<'tcx>,
@@ -319,6 +333,11 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit
319333
// so we just add it here with AvailableExternallyLinkage
320334
new_codegen_unit.items.insert(trans_item,
321335
llvm::AvailableExternallyLinkage);
336+
} else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() {
337+
// An instantiation of this item is always available in the
338+
// crate it was imported from.
339+
new_codegen_unit.items.insert(trans_item,
340+
llvm::AvailableExternallyLinkage);
322341
} else {
323342
// We can't be sure if this will also be instantiated
324343
// somewhere else, so we add an instance here with
@@ -414,3 +433,54 @@ fn compute_codegen_unit_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
414433

415434
return token::intern_and_get_ident(&mod_path[..]);
416435
}
436+
437+
fn single_codegen_unit<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
438+
trans_items: I,
439+
reachable: &NodeSet)
440+
-> CodegenUnit<'tcx>
441+
where I: Iterator<Item = TransItem<'tcx>>
442+
{
443+
let mut items = FnvHashMap();
444+
445+
for trans_item in trans_items {
446+
let linkage = trans_item.explicit_linkage(tcx).unwrap_or_else(|| {
447+
match trans_item {
448+
TransItem::Static(node_id) => {
449+
if reachable.contains(&node_id) {
450+
llvm::ExternalLinkage
451+
} else {
452+
llvm::InternalLinkage
453+
}
454+
}
455+
TransItem::DropGlue(_) => {
456+
llvm::InternalLinkage
457+
}
458+
TransItem::Fn(instance) => {
459+
if trans_item.is_generic_fn() ||
460+
trans_item.is_from_extern_crate() ||
461+
!reachable.contains(&tcx.map
462+
.as_local_node_id(instance.def)
463+
.unwrap()) {
464+
llvm::InternalLinkage
465+
} else {
466+
llvm::ExternalLinkage
467+
}
468+
}
469+
}
470+
});
471+
472+
items.insert(trans_item, linkage);
473+
}
474+
475+
CodegenUnit {
476+
name: numbered_codegen_unit_name(&tcx.crate_name[..], 0),
477+
items: items
478+
}
479+
}
480+
481+
fn numbered_codegen_unit_name(crate_name: &str, index: usize) -> InternedString {
482+
token::intern_and_get_ident(&format!("{}{}{}",
483+
crate_name,
484+
NUMBERED_CODEGEN_UNIT_MARKER,
485+
index)[..])
486+
}

‎src/librustc_trans/trans_item.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,14 @@ impl<'a, 'tcx> TransItem<'tcx> {
184184
}
185185
}
186186

187+
pub fn is_generic_fn(&self) -> bool {
188+
match *self {
189+
TransItem::Fn(ref instance) => !instance.substs.types.is_empty(),
190+
TransItem::DropGlue(..) |
191+
TransItem::Static(..) => false,
192+
}
193+
}
194+
187195
pub fn explicit_linkage(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option<llvm::Linkage> {
188196
let def_id = match *self {
189197
TransItem::Fn(ref instance) => instance.def,

‎src/test/codegen-units/partitioning/extern-drop-glue.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ extern crate cgu_extern_drop_glue;
2525

2626
struct LocalStruct(cgu_extern_drop_glue::Struct);
2727

28-
//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[WeakODR]
28+
//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External]
2929
fn user()
3030
{
3131
//~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[OnceODR]
@@ -37,7 +37,7 @@ mod mod1 {
3737

3838
struct LocalStruct(cgu_extern_drop_glue::Struct);
3939

40-
//~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[WeakODR]
40+
//~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External]
4141
fn user()
4242
{
4343
//~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[OnceODR]

‎src/test/codegen-units/partitioning/extern-generic.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,23 @@
1919
// aux-build:cgu_generic_function.rs
2020
extern crate cgu_generic_function;
2121

22-
//~ TRANS_ITEM fn extern_generic::user[0] @@ extern_generic[WeakODR]
22+
//~ TRANS_ITEM fn extern_generic::user[0] @@ extern_generic[External]
2323
fn user() {
2424
let _ = cgu_generic_function::foo("abc");
2525
}
2626

2727
mod mod1 {
2828
use cgu_generic_function;
2929

30-
//~ TRANS_ITEM fn extern_generic::mod1[0]::user[0] @@ extern_generic-mod1[WeakODR]
30+
//~ TRANS_ITEM fn extern_generic::mod1[0]::user[0] @@ extern_generic-mod1[External]
3131
fn user() {
3232
let _ = cgu_generic_function::foo("abc");
3333
}
3434

3535
mod mod1 {
3636
use cgu_generic_function;
3737

38-
//~ TRANS_ITEM fn extern_generic::mod1[0]::mod1[0]::user[0] @@ extern_generic-mod1-mod1[WeakODR]
38+
//~ TRANS_ITEM fn extern_generic::mod1[0]::mod1[0]::user[0] @@ extern_generic-mod1-mod1[External]
3939
fn user() {
4040
let _ = cgu_generic_function::foo("abc");
4141
}
@@ -45,14 +45,14 @@ mod mod1 {
4545
mod mod2 {
4646
use cgu_generic_function;
4747

48-
//~ TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[WeakODR]
48+
//~ TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[External]
4949
fn user() {
5050
let _ = cgu_generic_function::foo("abc");
5151
}
5252
}
5353

5454
mod mod3 {
55-
//~ TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[WeakODR]
55+
//~ TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[External]
5656
fn non_user() {}
5757
}
5858

‎src/test/codegen-units/partitioning/inlining-from-extern-crate.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ extern crate cgu_explicit_inlining;
2121
// This test makes sure that items inlined from external crates are privately
2222
// instantiated in every codegen unit they are used in.
2323

24-
//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[OnceODR] inlining_from_extern_crate-mod1[OnceODR]
25-
//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[OnceODR] inlining_from_extern_crate-mod2[OnceODR]
24+
//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod1[Available]
25+
//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod2[Available]
2626

27-
//~ TRANS_ITEM fn inlining_from_extern_crate::user[0] @@ inlining_from_extern_crate[WeakODR]
27+
//~ TRANS_ITEM fn inlining_from_extern_crate::user[0] @@ inlining_from_extern_crate[External]
2828
pub fn user()
2929
{
3030
cgu_explicit_inlining::inlined();
@@ -37,7 +37,7 @@ pub fn user()
3737
mod mod1 {
3838
use cgu_explicit_inlining;
3939

40-
//~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[WeakODR]
40+
//~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[External]
4141
pub fn user()
4242
{
4343
cgu_explicit_inlining::inlined();
@@ -50,7 +50,7 @@ mod mod1 {
5050
mod mod2 {
5151
use cgu_explicit_inlining;
5252

53-
//~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[WeakODR]
53+
//~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[External]
5454
pub fn user()
5555
{
5656
cgu_explicit_inlining::always_inlined();

‎src/test/codegen-units/partitioning/local-drop-glue.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ struct Struct {
2323
}
2424

2525
impl Drop for Struct {
26-
//~ TRANS_ITEM fn local_drop_glue::{{impl}}[0]::drop[0] @@ local_drop_glue[WeakODR]
26+
//~ TRANS_ITEM fn local_drop_glue::{{impl}}[0]::drop[0] @@ local_drop_glue[External]
2727
fn drop(&mut self) {}
2828
}
2929

@@ -32,7 +32,7 @@ struct Outer {
3232
_a: Struct
3333
}
3434

35-
//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[WeakODR]
35+
//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[External]
3636
fn user()
3737
{
3838
let _ = Outer {
@@ -53,7 +53,7 @@ mod mod1
5353
_b: (u32, Struct),
5454
}
5555

56-
//~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[WeakODR]
56+
//~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[External]
5757
fn user()
5858
{
5959
let _ = Struct2 {

‎src/test/codegen-units/partitioning/local-generic.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,23 @@
2525
//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[WeakODR]
2626
pub fn generic<T>(x: T) -> T { x }
2727

28-
//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[WeakODR]
28+
//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External]
2929
fn user() {
3030
let _ = generic(0u32);
3131
}
3232

3333
mod mod1 {
3434
pub use super::generic;
3535

36-
//~ TRANS_ITEM fn local_generic::mod1[0]::user[0] @@ local_generic-mod1[WeakODR]
36+
//~ TRANS_ITEM fn local_generic::mod1[0]::user[0] @@ local_generic-mod1[External]
3737
fn user() {
3838
let _ = generic(0u64);
3939
}
4040

4141
mod mod1 {
4242
use super::generic;
4343

44-
//~ TRANS_ITEM fn local_generic::mod1[0]::mod1[0]::user[0] @@ local_generic-mod1-mod1[WeakODR]
44+
//~ TRANS_ITEM fn local_generic::mod1[0]::mod1[0]::user[0] @@ local_generic-mod1-mod1[External]
4545
fn user() {
4646
let _ = generic('c');
4747
}
@@ -51,7 +51,7 @@ mod mod1 {
5151
mod mod2 {
5252
use super::generic;
5353

54-
//~ TRANS_ITEM fn local_generic::mod2[0]::user[0] @@ local_generic-mod2[WeakODR]
54+
//~ TRANS_ITEM fn local_generic::mod2[0]::user[0] @@ local_generic-mod2[External]
5555
fn user() {
5656
let _ = generic("abc");
5757
}

0 commit comments

Comments
 (0)