From 1ebfab5104ba354d5d3283fd16e086d0a8549623 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 12 Jul 2017 17:35:30 +0200 Subject: [PATCH 1/6] trans: Collect all accesses between trans-items, not just inlining edges. --- src/librustc_trans/collector.rs | 134 +++++++++++++++++++++++++------- 1 file changed, 105 insertions(+), 29 deletions(-) diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index a76abcf7b49a6..19d7198b49b7b 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -208,6 +208,8 @@ use rustc::util::nodemap::{FxHashSet, FxHashMap, DefIdMap}; use trans_item::{TransItem, DefPathBasedNames, InstantiationMode}; +use rustc_data_structures::bitvec::BitVector; + #[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)] pub enum TransItemCollectionMode { Eager, @@ -217,12 +219,16 @@ pub enum TransItemCollectionMode { /// Maps every translation item to all translation items it references in its /// body. pub struct InliningMap<'tcx> { - // Maps a source translation item to a range of target translation items - // that are potentially inlined by LLVM into the source. + // Maps a source translation item to the range of translation items + // accessed by it. // The two numbers in the tuple are the start (inclusive) and // end index (exclusive) within the `targets` vecs. index: FxHashMap, (usize, usize)>, targets: Vec>, + + // Contains one bit per translation item in the `targets` field. That bit + // is true if that translation item needs to be inlined into every CGU. + inlines: BitVector, } impl<'tcx> InliningMap<'tcx> { @@ -231,18 +237,33 @@ impl<'tcx> InliningMap<'tcx> { InliningMap { index: FxHashMap(), targets: Vec::new(), + inlines: BitVector::new(1024), } } - fn record_inlining_canditates(&mut self, - source: TransItem<'tcx>, - targets: I) - where I: Iterator> + fn record_accesses(&mut self, + source: TransItem<'tcx>, + targets: I) + where I: Iterator, bool)> { assert!(!self.index.contains_key(&source)); let start_index = self.targets.len(); - self.targets.extend(targets); + let (targets_size_hint, targets_size_hint_max) = targets.size_hint(); + debug_assert_eq!(targets_size_hint_max, Some(targets_size_hint)); + let new_items_count = targets_size_hint; + let new_items_count_total = new_items_count + self.targets.len(); + + self.targets.reserve(new_items_count); + self.inlines.grow(new_items_count_total); + + for (i, (target, inline)) in targets.enumerate() { + self.targets.push(target); + if inline { + self.inlines.insert(i + start_index); + } + } + let end_index = self.targets.len(); self.index.insert(source, (start_index, end_index)); } @@ -250,14 +271,27 @@ impl<'tcx> InliningMap<'tcx> { // Internally iterate over all items referenced by `source` which will be // made available for inlining. pub fn with_inlining_candidates(&self, source: TransItem<'tcx>, mut f: F) - where F: FnMut(TransItem<'tcx>) { - if let Some(&(start_index, end_index)) = self.index.get(&source) - { - for candidate in &self.targets[start_index .. end_index] { - f(*candidate) + where F: FnMut(TransItem<'tcx>) + { + if let Some(&(start_index, end_index)) = self.index.get(&source) { + for (i, candidate) in self.targets[start_index .. end_index] + .iter() + .enumerate() { + if self.inlines.contains(start_index + i) { + f(*candidate); + } } } } + + // Internally iterate over all items and the things each accesses. + pub fn iter_accesses(&self, mut f: F) + where F: FnMut(TransItem<'tcx>, &[TransItem<'tcx>]) + { + for (&accessor, &(start_index, end_index)) in &self.index { + f(accessor, &self.targets[start_index .. end_index]) + } + } } pub fn collect_crate_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, @@ -340,7 +374,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, recursion_depth_reset = None; - collect_neighbours(scx, instance, &mut neighbors); + collect_neighbours(scx, instance, true, &mut neighbors); } TransItem::Fn(instance) => { // Sanity check whether this ended up being collected accidentally @@ -352,14 +386,14 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, recursion_depths)); check_type_length_limit(scx.tcx(), instance); - collect_neighbours(scx, instance, &mut neighbors); + collect_neighbours(scx, instance, false, &mut neighbors); } TransItem::GlobalAsm(..) => { recursion_depth_reset = None; } } - record_inlining_canditates(scx.tcx(), starting_point, &neighbors[..], inlining_map); + record_accesses(scx.tcx(), starting_point, &neighbors[..], inlining_map); for neighbour in neighbors { collect_items_rec(scx, neighbour, visited, recursion_depths, inlining_map); @@ -372,7 +406,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, debug!("END collect_items_rec({})", starting_point.to_string(scx.tcx())); } -fn record_inlining_canditates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, +fn record_accesses<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, caller: TransItem<'tcx>, callees: &[TransItem<'tcx>], inlining_map: &mut InliningMap<'tcx>) { @@ -380,11 +414,12 @@ fn record_inlining_canditates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_item.instantiation_mode(tcx) == InstantiationMode::LocalCopy }; - let inlining_candidates = callees.into_iter() - .map(|x| *x) - .filter(is_inlining_candidate); + let accesses = callees.into_iter() + .map(|trans_item| { + (*trans_item, is_inlining_candidate(trans_item)) + }); - inlining_map.record_inlining_canditates(caller, inlining_candidates); + inlining_map.record_accesses(caller, accesses); } fn check_recursion_limit<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -459,7 +494,8 @@ struct MirNeighborCollector<'a, 'tcx: 'a> { scx: &'a SharedCrateContext<'a, 'tcx>, mir: &'a mir::Mir<'tcx>, output: &'a mut Vec>, - param_substs: &'tcx Substs<'tcx> + param_substs: &'tcx Substs<'tcx>, + const_context: bool, } impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { @@ -540,7 +576,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { let substs = self.scx.tcx().trans_apply_param_substs(self.param_substs, &substs); let instance = monomorphize::resolve(self.scx, def_id, substs); - collect_neighbours(self.scx, instance, self.output); + collect_neighbours(self.scx, instance, true, self.output); } self.super_constant(constant, location); @@ -556,8 +592,16 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { match *kind { mir::TerminatorKind::Call { ref func, .. } => { let callee_ty = func.ty(self.mir, tcx); - let callee_ty = tcx.trans_apply_param_substs(self.param_substs, &callee_ty); - visit_fn_use(self.scx, callee_ty, true, &mut self.output); + + let skip_const = self.const_context && match callee_ty.sty { + ty::TyFnDef(def_id, _) => self.scx.tcx().is_const_fn(def_id), + _ => false + }; + + if !skip_const { + let callee_ty = tcx.trans_apply_param_substs(self.param_substs, &callee_ty); + visit_fn_use(self.scx, callee_ty, true, &mut self.output); + } } mir::TerminatorKind::Drop { ref location, .. } | mir::TerminatorKind::DropAndReplace { ref location, .. } => { @@ -576,6 +620,22 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { self.super_terminator_kind(block, kind, location); } + + fn visit_static(&mut self, + static_: &mir::Static<'tcx>, + context: mir::visit::LvalueContext<'tcx>, + location: Location) { + debug!("visiting static {:?} @ {:?}", static_.def_id, location); + + let tcx = self.scx.tcx(); + let instance = Instance::mono(tcx, static_.def_id); + if should_trans_locally(tcx, &instance) { + let node_id = tcx.hir.as_local_node_id(static_.def_id).unwrap(); + self.output.push(TransItem::Static(node_id)); + } + + self.super_static(static_, context, location); + } } fn visit_drop_use<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, @@ -850,8 +910,14 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> { // const items only generate translation items if they are // actually used somewhere. Just declaring them is insufficient. } - hir::ItemFn(.., ref generics, _) => { - if !generics.is_type_parameterized() { + hir::ItemFn(_, _, constness, _, ref generics, _) => { + let is_const = match constness { + hir::Constness::Const => true, + hir::Constness::NotConst => false, + }; + + if !generics.is_type_parameterized() && + (!is_const || self.mode == TransItemCollectionMode::Eager) { let def_id = self.scx.tcx().hir.local_def_id(item.id); debug!("RootCollector: ItemFn({})", @@ -872,12 +938,13 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> { fn visit_impl_item(&mut self, ii: &'v hir::ImplItem) { match ii.node { hir::ImplItemKind::Method(hir::MethodSig { + constness, ref generics, .. }, _) => { let hir_map = &self.scx.tcx().hir; let parent_node_id = hir_map.get_parent_node(ii.id); - let is_impl_generic = match hir_map.expect_item(parent_node_id) { + let is_impl_generic = || match hir_map.expect_item(parent_node_id) { &hir::Item { node: hir::ItemImpl(_, _, _, ref generics, ..), .. @@ -889,7 +956,14 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> { } }; - if !generics.is_type_parameterized() && !is_impl_generic { + let is_const = match constness { + hir::Constness::Const => true, + hir::Constness::NotConst => false, + }; + + if (!is_const || self.mode == TransItemCollectionMode::Eager) && + !generics.is_type_parameterized() && + !is_impl_generic() { let def_id = self.scx.tcx().hir.local_def_id(ii.id); debug!("RootCollector: MethodImplItem({})", @@ -958,6 +1032,7 @@ fn create_trans_items_for_default_impls<'a, 'tcx>(scx: &SharedCrateContext<'a, ' /// Scan the MIR in order to find function calls, closures, and drop-glue fn collect_neighbours<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, instance: Instance<'tcx>, + const_context: bool, output: &mut Vec>) { let mir = scx.tcx().instance_mir(instance.def); @@ -966,7 +1041,8 @@ fn collect_neighbours<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, scx: scx, mir: &mir, output: output, - param_substs: instance.substs + param_substs: instance.substs, + const_context, }; visitor.visit_mir(&mir); From 2f07eb3261c89fb7b34d0eed1e446c73e55f7d2f Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 12 Jul 2017 17:37:58 +0200 Subject: [PATCH 2/6] trans: Internalize symbols at the trans-item level, without relying on LLVM. --- src/librustc_trans/back/linker.rs | 8 +- src/librustc_trans/back/lto.rs | 2 +- src/librustc_trans/back/symbol_export.rs | 112 +++++++---- src/librustc_trans/base.rs | 189 +++--------------- src/librustc_trans/callee.rs | 22 ++- src/librustc_trans/collector.rs | 10 +- src/librustc_trans/consts.rs | 14 ++ src/librustc_trans/context.rs | 33 +++- src/librustc_trans/debuginfo/utils.rs | 2 +- src/librustc_trans/partitioning.rs | 238 +++++++++++++++++++---- src/librustc_trans/trans_item.rs | 18 +- 11 files changed, 383 insertions(+), 265 deletions(-) diff --git a/src/librustc_trans/back/linker.rs b/src/librustc_trans/back/linker.rs index 0b15886083a4e..a178695cba97e 100644 --- a/src/librustc_trans/back/linker.rs +++ b/src/librustc_trans/back/linker.rs @@ -19,7 +19,7 @@ use std::process::Command; use context::SharedCrateContext; use back::archive; -use back::symbol_export::{self, ExportedSymbols}; +use back::symbol_export::ExportedSymbols; use rustc::middle::dependency_format::Linkage; use rustc::hir::def_id::{LOCAL_CRATE, CrateNum}; use rustc_back::LinkerFlavor; @@ -687,10 +687,8 @@ fn exported_symbols(scx: &SharedCrateContext, exported_symbols: &ExportedSymbols, crate_type: CrateType) -> Vec { - let export_threshold = symbol_export::crate_export_threshold(crate_type); - let mut symbols = Vec::new(); - exported_symbols.for_each_exported_symbol(LOCAL_CRATE, export_threshold, |name, _| { + exported_symbols.for_each_exported_symbol(LOCAL_CRATE, |name, _, _| { symbols.push(name.to_owned()); }); @@ -702,7 +700,7 @@ fn exported_symbols(scx: &SharedCrateContext, // For each dependency that we are linking to statically ... if *dep_format == Linkage::Static { // ... we add its symbol list to our export list. - exported_symbols.for_each_exported_symbol(cnum, export_threshold, |name, _| { + exported_symbols.for_each_exported_symbol(cnum, |name, _, _| { symbols.push(name.to_owned()); }) } diff --git a/src/librustc_trans/back/lto.rs b/src/librustc_trans/back/lto.rs index c402bdea2b23f..feed127b0b60b 100644 --- a/src/librustc_trans/back/lto.rs +++ b/src/librustc_trans/back/lto.rs @@ -66,7 +66,7 @@ pub fn run(cgcx: &CodegenContext, let export_threshold = symbol_export::crates_export_threshold(&cgcx.crate_types); - let symbol_filter = &|&(ref name, level): &(String, _)| { + let symbol_filter = &|&(ref name, _, level): &(String, _, _)| { if symbol_export::is_below_threshold(level, export_threshold) { let mut bytes = Vec::with_capacity(name.len() + 1); bytes.extend(name.bytes()); diff --git a/src/librustc_trans/back/symbol_export.rs b/src/librustc_trans/back/symbol_export.rs index 52fe747858cc4..72071f8cec990 100644 --- a/src/librustc_trans/back/symbol_export.rs +++ b/src/librustc_trans/back/symbol_export.rs @@ -8,10 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use context::SharedCrateContext; use monomorphize::Instance; -use rustc::util::nodemap::FxHashMap; -use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE}; +use rustc::util::nodemap::{FxHashMap, NodeSet}; +use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE, INVALID_CRATE, CRATE_DEF_INDEX}; use rustc::session::config; use rustc::ty::TyCtxt; use syntax::attr; @@ -28,59 +27,87 @@ pub enum SymbolExportLevel { } /// The set of symbols exported from each crate in the crate graph. +#[derive(Debug)] pub struct ExportedSymbols { - exports: FxHashMap>, + pub export_threshold: SymbolExportLevel, + exports: FxHashMap>, + local_exports: NodeSet, } impl ExportedSymbols { pub fn empty() -> ExportedSymbols { ExportedSymbols { + export_threshold: SymbolExportLevel::C, exports: FxHashMap(), + local_exports: NodeSet(), } } - pub fn compute<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) -> ExportedSymbols { - let mut local_crate: Vec<_> = scx - .exported_symbols() + pub fn compute<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + local_exported_symbols: &NodeSet) + -> ExportedSymbols { + let export_threshold = crates_export_threshold(&tcx.sess.crate_types.borrow()); + + let mut local_crate: Vec<_> = local_exported_symbols .iter() .map(|&node_id| { - scx.tcx().hir.local_def_id(node_id) + tcx.hir.local_def_id(node_id) }) .map(|def_id| { - let name = scx.tcx().symbol_name(Instance::mono(scx.tcx(), def_id)); - let export_level = export_level(scx, def_id); + let name = tcx.symbol_name(Instance::mono(tcx, def_id)); + let export_level = export_level(tcx, def_id); debug!("EXPORTED SYMBOL (local): {} ({:?})", name, export_level); - (str::to_owned(&name), export_level) + (str::to_owned(&name), def_id, export_level) }) .collect(); - if scx.sess().entry_fn.borrow().is_some() { - local_crate.push(("main".to_string(), SymbolExportLevel::C)); + let mut local_exports = local_crate + .iter() + .filter_map(|&(_, def_id, level)| { + if is_below_threshold(level, export_threshold) { + tcx.hir.as_local_node_id(def_id) + } else { + None + } + }) + .collect::(); + + const INVALID_DEF_ID: DefId = DefId { + krate: INVALID_CRATE, + index: CRATE_DEF_INDEX, + }; + + if let Some(_) = *tcx.sess.entry_fn.borrow() { + local_crate.push(("main".to_string(), + INVALID_DEF_ID, + SymbolExportLevel::C)); } - if let Some(id) = scx.sess().derive_registrar_fn.get() { - let def_id = scx.tcx().hir.local_def_id(id); + if let Some(id) = tcx.sess.derive_registrar_fn.get() { + let def_id = tcx.hir.local_def_id(id); let idx = def_id.index; - let disambiguator = scx.sess().local_crate_disambiguator(); - let registrar = scx.sess().generate_derive_registrar_symbol(disambiguator, idx); - local_crate.push((registrar, SymbolExportLevel::C)); + let disambiguator = tcx.sess.local_crate_disambiguator(); + let registrar = tcx.sess.generate_derive_registrar_symbol(disambiguator, idx); + local_crate.push((registrar, def_id, SymbolExportLevel::C)); + local_exports.insert(id); } - if scx.sess().crate_types.borrow().contains(&config::CrateTypeDylib) { - local_crate.push((metadata_symbol_name(scx.tcx()), + if tcx.sess.crate_types.borrow().contains(&config::CrateTypeDylib) { + local_crate.push((metadata_symbol_name(tcx), + INVALID_DEF_ID, SymbolExportLevel::Rust)); } let mut exports = FxHashMap(); exports.insert(LOCAL_CRATE, local_crate); - for cnum in scx.sess().cstore.crates() { + for cnum in tcx.sess.cstore.crates() { debug_assert!(cnum != LOCAL_CRATE); // If this crate is a plugin and/or a custom derive crate, then // we're not even going to link those in so we skip those crates. - if scx.sess().cstore.plugin_registrar_fn(cnum).is_some() || - scx.sess().cstore.derive_registrar_fn(cnum).is_some() { + if tcx.sess.cstore.plugin_registrar_fn(cnum).is_some() || + tcx.sess.cstore.derive_registrar_fn(cnum).is_some() { continue; } @@ -92,16 +119,16 @@ impl ExportedSymbols { // Down below we'll hardwire all of the symbols to the `Rust` export // level instead. let special_runtime_crate = - scx.tcx().is_panic_runtime(cnum.as_def_id()) || - scx.sess().cstore.is_compiler_builtins(cnum); + tcx.is_panic_runtime(cnum.as_def_id()) || + tcx.sess.cstore.is_compiler_builtins(cnum); - let crate_exports = scx - .sess() + let crate_exports = tcx + .sess .cstore .exported_symbols(cnum) .iter() .map(|&def_id| { - let name = scx.tcx().symbol_name(Instance::mono(scx.tcx(), def_id)); + let name = tcx.symbol_name(Instance::mono(tcx, def_id)); let export_level = if special_runtime_crate { // We can probably do better here by just ensuring that // it has hidden visibility rather than public @@ -118,10 +145,10 @@ impl ExportedSymbols { SymbolExportLevel::Rust } } else { - export_level(scx, def_id) + export_level(tcx, def_id) }; debug!("EXPORTED SYMBOL (re-export): {} ({:?})", name, export_level); - (str::to_owned(&name), export_level) + (str::to_owned(&name), def_id, export_level) }) .collect(); @@ -129,14 +156,16 @@ impl ExportedSymbols { } return ExportedSymbols { - exports: exports + export_threshold, + exports, + local_exports, }; - fn export_level(scx: &SharedCrateContext, + fn export_level(tcx: TyCtxt, sym_def_id: DefId) -> SymbolExportLevel { - let attrs = scx.tcx().get_attrs(sym_def_id); - if attr::contains_extern_indicator(scx.sess().diagnostic(), &attrs) { + let attrs = tcx.get_attrs(sym_def_id); + if attr::contains_extern_indicator(tcx.sess.diagnostic(), &attrs) { SymbolExportLevel::C } else { SymbolExportLevel::Rust @@ -144,9 +173,13 @@ impl ExportedSymbols { } } + pub fn local_exports(&self) -> &NodeSet { + &self.local_exports + } + pub fn exported_symbols(&self, cnum: CrateNum) - -> &[(String, SymbolExportLevel)] { + -> &[(String, DefId, SymbolExportLevel)] { match self.exports.get(&cnum) { Some(exports) => exports, None => &[] @@ -155,13 +188,12 @@ impl ExportedSymbols { pub fn for_each_exported_symbol(&self, cnum: CrateNum, - export_threshold: SymbolExportLevel, mut f: F) - where F: FnMut(&str, SymbolExportLevel) + where F: FnMut(&str, DefId, SymbolExportLevel) { - for &(ref name, export_level) in self.exported_symbols(cnum) { - if is_below_threshold(export_level, export_threshold) { - f(&name, export_level) + for &(ref name, def_id, export_level) in self.exported_symbols(cnum) { + if is_below_threshold(export_level, self.export_threshold) { + f(&name, def_id, export_level) } } } diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index ccb4742e4504a..3443160bded76 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -77,6 +77,7 @@ use rustc::util::nodemap::{NodeSet, FxHashMap, FxHashSet}; use libc::c_uint; use std::ffi::{CStr, CString}; use std::str; +use std::sync::Arc; use std::i32; use syntax_pos::Span; use syntax::attr; @@ -796,131 +797,6 @@ fn write_metadata<'a, 'gcx>(tcx: TyCtxt<'a, 'gcx, 'gcx>, return (metadata_llcx, metadata_llmod, metadata); } -/// Find any symbols that are defined in one compilation unit, but not declared -/// in any other compilation unit. Give these symbols internal linkage. -fn internalize_symbols<'a, 'tcx>(sess: &Session, - scx: &SharedCrateContext<'a, 'tcx>, - translation_items: &FxHashSet>, - llvm_modules: &[ModuleLlvm], - exported_symbols: &ExportedSymbols) { - let export_threshold = - symbol_export::crates_export_threshold(&sess.crate_types.borrow()); - - let exported_symbols = exported_symbols - .exported_symbols(LOCAL_CRATE) - .iter() - .filter(|&&(_, export_level)| { - symbol_export::is_below_threshold(export_level, export_threshold) - }) - .map(|&(ref name, _)| &name[..]) - .collect::>(); - - let tcx = scx.tcx(); - - let incr_comp = sess.opts.debugging_opts.incremental.is_some(); - - // 'unsafe' because we are holding on to CStr's from the LLVM module within - // this block. - unsafe { - let mut referenced_somewhere = FxHashSet(); - - // Collect all symbols that need to stay externally visible because they - // are referenced via a declaration in some other codegen unit. In - // incremental compilation, we don't need to collect. See below for more - // information. - if !incr_comp { - for ll in llvm_modules { - for val in iter_globals(ll.llmod).chain(iter_functions(ll.llmod)) { - let linkage = llvm::LLVMRustGetLinkage(val); - // We only care about external declarations (not definitions) - // and available_externally definitions. - let is_available_externally = - linkage == llvm::Linkage::AvailableExternallyLinkage; - let is_decl = llvm::LLVMIsDeclaration(val) == llvm::True; - - if is_decl || is_available_externally { - let symbol_name = CStr::from_ptr(llvm::LLVMGetValueName(val)); - referenced_somewhere.insert(symbol_name); - } - } - } - } - - // Also collect all symbols for which we cannot adjust linkage, because - // it is fixed by some directive in the source code. - let (locally_defined_symbols, linkage_fixed_explicitly) = { - let mut locally_defined_symbols = FxHashSet(); - let mut linkage_fixed_explicitly = FxHashSet(); - - for trans_item in translation_items { - let symbol_name = str::to_owned(&trans_item.symbol_name(tcx)); - if trans_item.explicit_linkage(tcx).is_some() { - linkage_fixed_explicitly.insert(symbol_name.clone()); - } - locally_defined_symbols.insert(symbol_name); - } - - (locally_defined_symbols, linkage_fixed_explicitly) - }; - - // Examine each external definition. If the definition is not used in - // any other compilation unit, and is not reachable from other crates, - // then give it internal linkage. - for ll in llvm_modules { - for val in iter_globals(ll.llmod).chain(iter_functions(ll.llmod)) { - let linkage = llvm::LLVMRustGetLinkage(val); - - let is_externally_visible = (linkage == llvm::Linkage::ExternalLinkage) || - (linkage == llvm::Linkage::LinkOnceODRLinkage) || - (linkage == llvm::Linkage::WeakODRLinkage); - - if !is_externally_visible { - // This symbol is not visible outside of its codegen unit, - // so there is nothing to do for it. - continue; - } - - let name_cstr = CStr::from_ptr(llvm::LLVMGetValueName(val)); - let name_str = name_cstr.to_str().unwrap(); - - if exported_symbols.contains(&name_str) { - // This symbol is explicitly exported, so we can't - // mark it as internal or hidden. - continue; - } - - let is_declaration = llvm::LLVMIsDeclaration(val) == llvm::True; - - if is_declaration { - if locally_defined_symbols.contains(name_str) { - // Only mark declarations from the current crate as hidden. - // Otherwise we would mark things as hidden that are - // imported from other crates or native libraries. - llvm::LLVMRustSetVisibility(val, llvm::Visibility::Hidden); - } - } else { - let has_fixed_linkage = linkage_fixed_explicitly.contains(name_str); - - if !has_fixed_linkage { - // In incremental compilation mode, we can't be sure that - // we saw all references because we don't know what's in - // cached compilation units, so we always assume that the - // given item has been referenced. - if incr_comp || referenced_somewhere.contains(&name_cstr) { - llvm::LLVMRustSetVisibility(val, llvm::Visibility::Hidden); - } else { - llvm::LLVMRustSetLinkage(val, llvm::Linkage::InternalLinkage); - } - - llvm::LLVMSetDLLStorageClass(val, llvm::DLLStorageClass::Default); - llvm::UnsetComdat(val); - } - } - } - } - } -} - // Create a `__imp_ = &symbol` global for every public static `symbol`. // This is required to satisfy `dllimport` references to static data in .rlibs // when using MSVC linker. We do this only for data, as linker can fix up @@ -992,15 +868,6 @@ fn iter_globals(llmod: llvm::ModuleRef) -> ValueIter { } } -fn iter_functions(llmod: llvm::ModuleRef) -> ValueIter { - unsafe { - ValueIter { - cur: llvm::LLVMGetFirstFunction(llmod), - step: llvm::LLVMGetNextFunction, - } - } -} - /// The context provided lists a set of reachable ids as calculated by /// middle::reachable, but this contains far more ids and symbols than we're /// actually exposing from the object file. This function will filter the set in @@ -1063,20 +930,19 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let krate = tcx.hir.krate(); let ty::CrateAnalysis { reachable, .. } = analysis; - let exported_symbols = find_exported_symbols(tcx, &reachable); let check_overflow = tcx.sess.overflow_checks(); let link_meta = link::build_link_meta(incremental_hashes_map); + let exported_symbol_node_ids = find_exported_symbols(tcx, &reachable); let shared_ccx = SharedCrateContext::new(tcx, - exported_symbols, check_overflow, output_filenames); // Translate the metadata. let (metadata_llcx, metadata_llmod, metadata) = time(tcx.sess.time_passes(), "write metadata", || { - write_metadata(tcx, &link_meta, shared_ccx.exported_symbols()) + write_metadata(tcx, &link_meta, &exported_symbol_node_ids) }); let metadata_module = ModuleTranslation { @@ -1090,7 +956,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let no_builtins = attr::contains_name(&krate.attrs, "no_builtins"); - // Skip crate items and just output metadata in -Z no-trans mode. if tcx.sess.opts.debugging_opts.no_trans || !tcx.sess.opts.output_types.should_trans() { @@ -1110,10 +975,15 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; } + let exported_symbols = Arc::new(ExportedSymbols::compute(tcx, + &exported_symbol_node_ids)); + // Run the translation item collector and partition the collected items into // codegen units. let (translation_items, codegen_units) = - collect_and_partition_translation_items(&shared_ccx); + collect_and_partition_translation_items(&shared_ccx, &exported_symbols); + + let translation_items = Arc::new(translation_items); let mut all_stats = Stats::default(); let modules: Vec = codegen_units @@ -1123,7 +993,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let ((stats, module), _) = tcx.dep_graph.with_task(dep_node, AssertDepGraphSafe(&shared_ccx), - AssertDepGraphSafe(cgu), + AssertDepGraphSafe((cgu, + translation_items.clone(), + exported_symbols.clone())), module_translation); all_stats.extend(stats); module @@ -1132,16 +1004,18 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn module_translation<'a, 'tcx>( scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>, - args: AssertDepGraphSafe>) + args: AssertDepGraphSafe<(CodegenUnit<'tcx>, + Arc>>, + Arc)>) -> (Stats, ModuleTranslation) { // FIXME(#40304): We ought to be using the id as a key and some queries, I think. let AssertDepGraphSafe(scx) = scx; - let AssertDepGraphSafe(cgu) = args; + let AssertDepGraphSafe((cgu, crate_trans_items, exported_symbols)) = args; let cgu_name = String::from(cgu.name()); let cgu_id = cgu.work_product_id(); - let symbol_name_hash = cgu.compute_symbol_name_hash(scx); + let symbol_name_hash = cgu.compute_symbol_name_hash(scx, &exported_symbols); // Check whether there is a previous work-product we can // re-use. Not only must the file exist, and the inputs not @@ -1176,13 +1050,13 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } // Instantiate translation items without filling out definitions yet... - let lcx = LocalCrateContext::new(scx, cgu); + let lcx = LocalCrateContext::new(scx, cgu, crate_trans_items, exported_symbols); let module = { let ccx = CrateContext::new(scx, &lcx); let trans_items = ccx.codegen_unit() .items_in_deterministic_order(ccx.tcx()); - for &(trans_item, linkage) in &trans_items { - trans_item.predefine(&ccx, linkage); + for &(trans_item, (linkage, visibility)) in &trans_items { + trans_item.predefine(&ccx, linkage, visibility); } // ... and now that we have everything pre-defined, fill out those definitions. @@ -1272,8 +1146,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let sess = shared_ccx.sess(); - let exported_symbols = ExportedSymbols::compute(&shared_ccx); - // Get the list of llvm modules we created. We'll do a few wacky // transforms on them now. @@ -1285,16 +1157,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }) .collect(); - // Now that we have all symbols that are exported from the CGUs of this - // crate, we can run the `internalize_symbols` pass. - time(shared_ccx.sess().time_passes(), "internalize symbols", || { - internalize_symbols(sess, - &shared_ccx, - &translation_items, - &llvm_modules, - &exported_symbols); - }); - if sess.target.target.options.is_like_msvc && sess.crate_types.borrow().iter().any(|ct| *ct == config::CrateTypeRlib) { create_imps(sess, &llvm_modules); @@ -1355,7 +1217,8 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, allocator_module: allocator_module, link: link_meta, metadata: metadata, - exported_symbols: exported_symbols, + exported_symbols: Arc::try_unwrap(exported_symbols) + .expect("There's still a reference to exported_symbols?"), no_builtins: no_builtins, linker_info: linker_info, windows_subsystem: windows_subsystem, @@ -1410,7 +1273,8 @@ fn assert_symbols_are_distinct<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_i } } -fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) +fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, + exported_symbols: &ExportedSymbols) -> (FxHashSet>, Vec>) { let time_passes = scx.sess().time_passes(); @@ -1452,7 +1316,8 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a partitioning::partition(scx, items.iter().cloned(), strategy, - &inlining_map) + &inlining_map, + exported_symbols) }); assert!(scx.tcx().sess.opts.cg.codegen_units == codegen_units.len() || @@ -1480,7 +1345,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a let mut cgus = item_to_cgus.get_mut(i).unwrap_or(&mut empty); cgus.as_mut_slice().sort_by_key(|&(ref name, _)| name.clone()); cgus.dedup(); - for &(ref cgu_name, linkage) in cgus.iter() { + for &(ref cgu_name, (linkage, _)) in cgus.iter() { output.push_str(" "); output.push_str(&cgu_name); diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index dc788dc4b4834..76f94565bae51 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -23,6 +23,7 @@ use monomorphize::{self, Instance}; use rustc::hir::def_id::DefId; use rustc::ty::TypeFoldable; use rustc::ty::subst::Substs; +use trans_item::TransItem; use type_of; /// Translates a reference to a fn/method item, monomorphizing and @@ -99,19 +100,32 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let attrs = instance.def.attrs(ccx.tcx()); attributes::from_fn_attrs(ccx, &attrs, llfn); + let instance_def_id = instance.def_id(); + // Perhaps questionable, but we assume that anything defined // *in Rust code* may unwind. Foreign items like `extern "C" { // fn foo(); }` are assumed not to unwind **unless** they have // a `#[unwind]` attribute. - if !tcx.is_foreign_item(instance.def_id()) { + if !tcx.is_foreign_item(instance_def_id) { attributes::unwind(llfn, true); - unsafe { - llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage); + } + + unsafe { + llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage); + + if ccx.crate_trans_items().contains(&TransItem::Fn(instance)) { + if let Some(node_id) = tcx.hir.as_local_node_id(instance_def_id) { + if !ccx.exported_symbols().local_exports().contains(&node_id) { + llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); + } + } else { + llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); + } } } if ccx.use_dll_storage_attrs() && - ccx.sess().cstore.is_dllimport_foreign_item(instance.def_id()) + ccx.sess().cstore.is_dllimport_foreign_item(instance_def_id) { unsafe { llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport); diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 19d7198b49b7b..4e1870be56132 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -243,21 +243,19 @@ impl<'tcx> InliningMap<'tcx> { fn record_accesses(&mut self, source: TransItem<'tcx>, - targets: I) - where I: Iterator, bool)> + new_targets: I) + where I: Iterator, bool)> + ExactSizeIterator { assert!(!self.index.contains_key(&source)); let start_index = self.targets.len(); - let (targets_size_hint, targets_size_hint_max) = targets.size_hint(); - debug_assert_eq!(targets_size_hint_max, Some(targets_size_hint)); - let new_items_count = targets_size_hint; + let new_items_count = new_targets.len(); let new_items_count_total = new_items_count + self.targets.len(); self.targets.reserve(new_items_count); self.inlines.grow(new_items_count_total); - for (i, (target, inline)) in targets.enumerate() { + for (i, (target, inline)) in new_targets.enumerate() { self.targets.push(target); if inline { self.inlines.insert(i + start_index); diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index eac0a06256719..da2a58398634e 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -104,6 +104,12 @@ pub fn get_static(ccx: &CrateContext, def_id: DefId) -> ValueRef { let g = declare::define_global(ccx, &sym[..], llty).unwrap(); + if !ccx.exported_symbols().local_exports().contains(&id) { + unsafe { + llvm::LLVMRustSetVisibility(g, llvm::Visibility::Hidden); + } + } + (g, attrs) } @@ -243,8 +249,16 @@ pub fn trans_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let name_str_ref = CStr::from_ptr(llvm::LLVMGetValueName(g)); let name_string = CString::new(name_str_ref.to_bytes()).unwrap(); llvm::LLVMSetValueName(g, empty_string.as_ptr()); + + let linkage = llvm::LLVMRustGetLinkage(g); + let visibility = llvm::LLVMRustGetVisibility(g); + let new_g = llvm::LLVMRustGetOrInsertGlobal( ccx.llmod(), name_string.as_ptr(), val_llty.to_ref()); + + llvm::LLVMRustSetLinkage(new_g, linkage); + llvm::LLVMRustSetVisibility(new_g, visibility); + // To avoid breaking any invariants, we leave around the old // global for the moment; we'll replace all references to it // with the new global later. (See base::trans_crate.) diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 208e994653b58..bec39e3cde6d0 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -16,11 +16,13 @@ use rustc::hir::def_id::DefId; use rustc::traits; use debuginfo; use callee; +use back::symbol_export::ExportedSymbols; use base; use declare; use monomorphize::Instance; use partitioning::CodegenUnit; +use trans_item::TransItem; use type_::Type; use rustc_data_structures::base_n; use rustc::session::config::{self, NoDebugInfo, OutputFilenames}; @@ -28,13 +30,14 @@ use rustc::session::Session; use rustc::ty::subst::Substs; use rustc::ty::{self, Ty, TyCtxt}; use rustc::ty::layout::{LayoutCx, LayoutError, LayoutTyper, TyLayout}; -use rustc::util::nodemap::{NodeSet, DefIdMap, FxHashMap}; +use rustc::util::nodemap::{DefIdMap, FxHashMap, FxHashSet}; use std::ffi::{CStr, CString}; use std::cell::{Cell, RefCell}; use std::ptr; use std::iter; use std::str; +use std::sync::Arc; use std::marker::PhantomData; use syntax::ast; use syntax::symbol::InternedString; @@ -76,7 +79,6 @@ impl Stats { /// crate, so it must not contain references to any LLVM data structures /// (aside from metadata-related ones). pub struct SharedCrateContext<'a, 'tcx: 'a> { - exported_symbols: NodeSet, tcx: TyCtxt<'a, 'tcx, 'tcx>, check_overflow: bool, @@ -94,6 +96,13 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> { llcx: ContextRef, stats: Stats, codegen_unit: CodegenUnit<'tcx>, + + /// The translation items of the whole crate. + crate_trans_items: Arc>>, + + /// Information about which symbols are exported from the crate. + exported_symbols: Arc, + /// Cache instances of monomorphic and polymorphic items instances: RefCell, ValueRef>>, /// Cache generated vtables @@ -265,7 +274,6 @@ pub unsafe fn create_context_and_module(sess: &Session, mod_name: &str) -> (Cont impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { pub fn new(tcx: TyCtxt<'b, 'tcx, 'tcx>, - exported_symbols: NodeSet, check_overflow: bool, output_filenames: &'b OutputFilenames) -> SharedCrateContext<'b, 'tcx> { @@ -315,7 +323,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { let use_dll_storage_attrs = tcx.sess.target.target.options.is_like_msvc; SharedCrateContext { - exported_symbols: exported_symbols, tcx: tcx, check_overflow: check_overflow, use_dll_storage_attrs: use_dll_storage_attrs, @@ -335,10 +342,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { ty.is_freeze(self.tcx, ty::ParamEnv::empty(traits::Reveal::All), DUMMY_SP) } - pub fn exported_symbols<'a>(&'a self) -> &'a NodeSet { - &self.exported_symbols - } - pub fn tcx<'a>(&'a self) -> TyCtxt<'a, 'tcx, 'tcx> { self.tcx } @@ -362,7 +365,9 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { pub fn new(shared: &SharedCrateContext<'a, 'tcx>, - codegen_unit: CodegenUnit<'tcx>) + codegen_unit: CodegenUnit<'tcx>, + crate_trans_items: Arc>>, + exported_symbols: Arc,) -> LocalCrateContext<'a, 'tcx> { unsafe { // Append ".rs" to LLVM module identifier. @@ -394,6 +399,8 @@ impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { llcx: llcx, stats: Stats::default(), codegen_unit: codegen_unit, + crate_trans_items, + exported_symbols, instances: RefCell::new(FxHashMap()), vtables: RefCell::new(FxHashMap()), const_cstr_cache: RefCell::new(FxHashMap()), @@ -504,6 +511,14 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.local().codegen_unit } + pub fn crate_trans_items(&self) -> &FxHashSet> { + &self.local().crate_trans_items + } + + pub fn exported_symbols(&self) -> &ExportedSymbols { + &self.local().exported_symbols + } + pub fn td(&self) -> llvm::TargetDataRef { unsafe { llvm::LLVMRustGetModuleDataLayout(self.llmod()) } } diff --git a/src/librustc_trans/debuginfo/utils.rs b/src/librustc_trans/debuginfo/utils.rs index 0555714d623e6..6df509f34a472 100644 --- a/src/librustc_trans/debuginfo/utils.rs +++ b/src/librustc_trans/debuginfo/utils.rs @@ -37,7 +37,7 @@ pub fn is_node_local_to_unit(cx: &CrateContext, node_id: ast::NodeId) -> bool // visible). It might better to use the `exported_items` set from // `driver::CrateAnalysis` in the future, but (atm) this set is not // available in the translation pass. - !cx.shared().exported_symbols().contains(&node_id) + !cx.exported_symbols().local_exports().contains(&node_id) } #[allow(non_snake_case)] diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 68b72087b2e65..5e445652dc473 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -102,6 +102,7 @@ //! source-level module, functions from the same module will be available for //! inlining, even when they are not marked #[inline]. +use back::symbol_export::ExportedSymbols; use collector::InliningMap; use common; use context::SharedCrateContext; @@ -110,14 +111,15 @@ use rustc::dep_graph::{DepNode, WorkProductId}; use rustc::hir::def_id::DefId; use rustc::hir::map::DefPathData; use rustc::session::config::NUMBERED_CODEGEN_UNIT_MARKER; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, TyCtxt, InstanceDef}; use rustc::ty::item_path::characteristic_def_id_of_type; +use rustc::util::nodemap::{FxHashMap, FxHashSet}; use rustc_incremental::IchHasher; +use std::collections::hash_map::Entry; use std::hash::Hash; use syntax::ast::NodeId; use syntax::symbol::{Symbol, InternedString}; use trans_item::{TransItem, InstantiationMode}; -use rustc::util::nodemap::{FxHashMap, FxHashSet}; pub enum PartitioningStrategy { /// Generate one codegen unit per source-level module. @@ -134,16 +136,16 @@ pub struct CodegenUnit<'tcx> { /// as well as the crate name and disambiguator. name: InternedString, - items: FxHashMap, llvm::Linkage>, + items: FxHashMap, (llvm::Linkage, llvm::Visibility)>, } impl<'tcx> CodegenUnit<'tcx> { pub fn new(name: InternedString, - items: FxHashMap, llvm::Linkage>) + items: FxHashMap, (llvm::Linkage, llvm::Visibility)>) -> Self { CodegenUnit { - name: name, - items: items, + name, + items, } } @@ -159,7 +161,7 @@ impl<'tcx> CodegenUnit<'tcx> { &self.name } - pub fn items(&self) -> &FxHashMap, llvm::Linkage> { + pub fn items(&self) -> &FxHashMap, (llvm::Linkage, llvm::Visibility)> { &self.items } @@ -172,10 +174,11 @@ impl<'tcx> CodegenUnit<'tcx> { } pub fn compute_symbol_name_hash<'a>(&self, - scx: &SharedCrateContext<'a, 'tcx>) + scx: &SharedCrateContext<'a, 'tcx>, + exported_symbols: &ExportedSymbols) -> u64 { let mut state = IchHasher::new(); - let exported_symbols = scx.exported_symbols(); + let exported_symbols = exported_symbols.local_exports(); let all_items = self.items_in_deterministic_order(scx.tcx()); for (item, _) in all_items { let symbol_name = item.symbol_name(scx.tcx()); @@ -200,7 +203,8 @@ impl<'tcx> CodegenUnit<'tcx> { pub fn items_in_deterministic_order<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) - -> Vec<(TransItem<'tcx>, llvm::Linkage)> { + -> Vec<(TransItem<'tcx>, + (llvm::Linkage, llvm::Visibility))> { // The codegen tests rely on items being process in the same order as // they appear in the file, so for local items, we sort by node_id first #[derive(PartialEq, Eq, PartialOrd, Ord)] @@ -233,7 +237,8 @@ const FALLBACK_CODEGEN_UNIT: &'static str = "__rustc_fallback_codegen_unit"; pub fn partition<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, trans_items: I, strategy: PartitioningStrategy, - inlining_map: &InliningMap<'tcx>) + inlining_map: &InliningMap<'tcx>, + exported_symbols: &ExportedSymbols) -> Vec> where I: Iterator> { @@ -243,6 +248,7 @@ pub fn partition<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, // respective 'home' codegen unit. Regular translation items are all // functions and statics defined in the local crate. let mut initial_partitioning = place_root_translation_items(scx, + exported_symbols, trans_items); debug_dump(tcx, "INITIAL PARTITONING:", initial_partitioning.codegen_units.iter()); @@ -259,13 +265,22 @@ pub fn partition<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, // translation items have to go into each codegen unit. These additional // translation items can be drop-glue, functions from external crates, and // local functions the definition of which is marked with #[inline]. - let post_inlining = place_inlined_translation_items(initial_partitioning, - inlining_map); + let mut post_inlining = place_inlined_translation_items(initial_partitioning, + inlining_map); + + debug_dump(tcx, "POST INLINING:", post_inlining.codegen_units.iter()); - debug_dump(tcx, "POST INLINING:", post_inlining.0.iter()); + // Next we try to make as many symbols "internal" as possible, so LLVM has + // more freedom to optimize. + internalize_symbols(tcx, &mut post_inlining, inlining_map); // Finally, sort by codegen unit name, so that we get deterministic results - let mut result = post_inlining.0; + let PostInliningPartitioning { + codegen_units: mut result, + trans_item_placements: _, + internalization_candidates: _, + } = post_inlining; + result.sort_by(|cgu1, cgu2| { (&cgu1.name[..]).cmp(&cgu2.name[..]) }); @@ -284,19 +299,37 @@ pub fn partition<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, struct PreInliningPartitioning<'tcx> { codegen_units: Vec>, roots: FxHashSet>, + internalization_candidates: FxHashSet>, } -struct PostInliningPartitioning<'tcx>(Vec>); +/// For symbol internalization, we need to know whether a symbol/trans-item is +/// accessed from outside the codegen unit it is defined in. This type is used +/// to keep track of that. +#[derive(Clone, PartialEq, Eq, Debug)] +enum TransItemPlacement { + SingleCgu { cgu_name: InternedString }, + MultipleCgus, +} + +struct PostInliningPartitioning<'tcx> { + codegen_units: Vec>, + trans_item_placements: FxHashMap, TransItemPlacement>, + internalization_candidates: FxHashSet>, +} fn place_root_translation_items<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, + exported_symbols: &ExportedSymbols, trans_items: I) -> PreInliningPartitioning<'tcx> where I: Iterator> { let tcx = scx.tcx(); + let exported_symbols = exported_symbols.local_exports(); + let mut roots = FxHashSet(); let mut codegen_units = FxHashMap(); let is_incremental_build = tcx.sess.opts.incremental.is_some(); + let mut internalization_candidates = FxHashSet(); for trans_item in trans_items { let is_root = trans_item.instantiation_mode(tcx) == InstantiationMode::GloballyShared; @@ -318,18 +351,52 @@ fn place_root_translation_items<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, let mut codegen_unit = codegen_units.entry(codegen_unit_name.clone()) .or_insert_with(make_codegen_unit); - let linkage = match trans_item.explicit_linkage(tcx) { - Some(explicit_linkage) => explicit_linkage, + let (linkage, visibility) = match trans_item.explicit_linkage(tcx) { + Some(explicit_linkage) => (explicit_linkage, llvm::Visibility::Default), None => { match trans_item { - TransItem::Fn(..) | - TransItem::Static(..) | - TransItem::GlobalAsm(..) => llvm::ExternalLinkage, + TransItem::Fn(ref instance) => { + let visibility = match instance.def { + InstanceDef::Item(def_id) => { + if let Some(node_id) = tcx.hir.as_local_node_id(def_id) { + if exported_symbols.contains(&node_id) { + llvm::Visibility::Default + } else { + internalization_candidates.insert(trans_item); + llvm::Visibility::Hidden + } + } else { + internalization_candidates.insert(trans_item); + llvm::Visibility::Hidden + } + } + InstanceDef::FnPtrShim(..) | + InstanceDef::Virtual(..) | + InstanceDef::Intrinsic(..) | + InstanceDef::ClosureOnceShim { .. } | + InstanceDef::DropGlue(..) => { + bug!("partitioning: Encountered unexpected + root translation item: {:?}", + trans_item) + } + }; + (llvm::ExternalLinkage, visibility) + } + TransItem::Static(node_id) | + TransItem::GlobalAsm(node_id) => { + let visibility = if exported_symbols.contains(&node_id) { + llvm::Visibility::Default + } else { + internalization_candidates.insert(trans_item); + llvm::Visibility::Hidden + }; + (llvm::ExternalLinkage, visibility) + } } } }; - codegen_unit.items.insert(trans_item, linkage); + codegen_unit.items.insert(trans_item, (linkage, visibility)); roots.insert(trans_item); } } @@ -338,15 +405,16 @@ fn place_root_translation_items<'a, 'tcx, I>(scx: &SharedCrateContext<'a, 'tcx>, // crate with just types (for example), we could wind up with no CGU if codegen_units.is_empty() { let codegen_unit_name = Symbol::intern(FALLBACK_CODEGEN_UNIT).as_str(); - codegen_units.entry(codegen_unit_name.clone()) - .or_insert_with(|| CodegenUnit::empty(codegen_unit_name.clone())); + codegen_units.insert(codegen_unit_name.clone(), + CodegenUnit::empty(codegen_unit_name.clone())); } PreInliningPartitioning { codegen_units: codegen_units.into_iter() .map(|(_, codegen_unit)| codegen_unit) .collect(), - roots: roots, + roots, + internalization_candidates, } } @@ -388,37 +456,75 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit inlining_map: &InliningMap<'tcx>) -> PostInliningPartitioning<'tcx> { let mut new_partitioning = Vec::new(); + let mut trans_item_placements = FxHashMap(); + + let PreInliningPartitioning { + codegen_units: initial_cgus, + roots, + internalization_candidates, + } = initial_partitioning; - for codegen_unit in &initial_partitioning.codegen_units[..] { + let single_codegen_unit = initial_cgus.len() == 1; + + for old_codegen_unit in initial_cgus { // Collect all items that need to be available in this codegen unit let mut reachable = FxHashSet(); - for root in codegen_unit.items.keys() { + for root in old_codegen_unit.items.keys() { follow_inlining(*root, inlining_map, &mut reachable); } - let mut new_codegen_unit = - CodegenUnit::empty(codegen_unit.name.clone()); + let mut new_codegen_unit = CodegenUnit { + name: old_codegen_unit.name, + items: FxHashMap(), + }; // Add all translation items that are not already there for trans_item in reachable { - if let Some(linkage) = codegen_unit.items.get(&trans_item) { + if let Some(linkage) = old_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) { + if roots.contains(&trans_item) { bug!("GloballyShared trans-item inlined into other CGU: \ {:?}", trans_item); } // This is a cgu-private copy - new_codegen_unit.items.insert(trans_item, llvm::InternalLinkage); + new_codegen_unit.items.insert(trans_item, + (llvm::InternalLinkage, llvm::Visibility::Default)); + } + + if !single_codegen_unit { + // If there is more than one codegen unit, we need to keep track + // in which codegen units each translation item is placed: + match trans_item_placements.entry(trans_item) { + Entry::Occupied(e) => { + let placement = e.into_mut(); + debug_assert!(match *placement { + TransItemPlacement::SingleCgu { ref cgu_name } => { + *cgu_name != new_codegen_unit.name + } + TransItemPlacement::MultipleCgus => true, + }); + *placement = TransItemPlacement::MultipleCgus; + } + Entry::Vacant(e) => { + e.insert(TransItemPlacement::SingleCgu { + cgu_name: new_codegen_unit.name.clone() + }); + } + } } } new_partitioning.push(new_codegen_unit); } - return PostInliningPartitioning(new_partitioning); + return PostInliningPartitioning { + codegen_units: new_partitioning, + trans_item_placements, + internalization_candidates, + }; fn follow_inlining<'tcx>(trans_item: TransItem<'tcx>, inlining_map: &InliningMap<'tcx>, @@ -433,6 +539,72 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit } } +fn internalize_symbols<'a, 'tcx>(_tcx: TyCtxt<'a, 'tcx, 'tcx>, + partitioning: &mut PostInliningPartitioning<'tcx>, + inlining_map: &InliningMap<'tcx>) { + if partitioning.codegen_units.len() == 1 { + // Fast path for when there is only one codegen unit. In this case we + // can internalize all candidates, since there is nowhere else they + // could be accessed from. + for cgu in &mut partitioning.codegen_units { + for candidate in &partitioning.internalization_candidates { + cgu.items.insert(*candidate, (llvm::InternalLinkage, + llvm::Visibility::Default)); + } + } + + return; + } + + // Build a map from every translation item to all the translation items that + // reference it. + let mut accessor_map: FxHashMap, Vec>> = FxHashMap(); + inlining_map.iter_accesses(|accessor, accessees| { + for accessee in accessees { + accessor_map.entry(*accessee) + .or_insert(Vec::new()) + .push(accessor); + } + }); + + let trans_item_placements = &partitioning.trans_item_placements; + + // For each internalization candidates in each codegen unit, check if it is + // accessed from outside its defining codegen unit. + for cgu in &mut partitioning.codegen_units { + let home_cgu = TransItemPlacement::SingleCgu { + cgu_name: cgu.name.clone() + }; + + 'item: + for (accessee, &mut (ref mut linkage, _)) in &mut cgu.items { + if !partitioning.internalization_candidates.contains(accessee) { + // This item is no candidate for internalizing, so skip it. + continue + } + debug_assert_eq!(trans_item_placements[accessee], home_cgu); + + if let Some(accessors) = accessor_map.get(accessee) { + if accessors.iter() + .filter_map(|accessor| { + // Some accessors might not have been + // instantiated. We can safely ignore those. + trans_item_placements.get(accessor) + }) + .any(|placement| *placement != home_cgu) { + // Found an accessor from another CGU, so skip to the next + // item without marking this one as internal. + continue 'item; + } + } + + // If we got here, we did not find any accesses from other CGUs, + // so it's fine to make this translation item internal. + *linkage = llvm::InternalLinkage; + } + } +} + fn characteristic_def_id_of_trans_item<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, trans_item: TransItem<'tcx>) -> Option { diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 8a77b26580999..b94fd13c3a4a2 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -99,7 +99,8 @@ impl<'a, 'tcx> TransItem<'tcx> { pub fn predefine(&self, ccx: &CrateContext<'a, 'tcx>, - linkage: llvm::Linkage) { + linkage: llvm::Linkage, + visibility: llvm::Visibility) { debug!("BEGIN PREDEFINING '{} ({})' in cgu {}", self.to_string(ccx.tcx()), self.to_raw_string(), @@ -111,10 +112,10 @@ impl<'a, 'tcx> TransItem<'tcx> { match *self { TransItem::Static(node_id) => { - TransItem::predefine_static(ccx, node_id, linkage, &symbol_name); + TransItem::predefine_static(ccx, node_id, linkage, visibility, &symbol_name); } TransItem::Fn(instance) => { - TransItem::predefine_fn(ccx, instance, linkage, &symbol_name); + TransItem::predefine_fn(ccx, instance, linkage, visibility, &symbol_name); } TransItem::GlobalAsm(..) => {} } @@ -128,6 +129,7 @@ impl<'a, 'tcx> TransItem<'tcx> { fn predefine_static(ccx: &CrateContext<'a, 'tcx>, node_id: ast::NodeId, linkage: llvm::Linkage, + visibility: llvm::Visibility, symbol_name: &str) { let def_id = ccx.tcx().hir.local_def_id(node_id); let instance = Instance::mono(ccx.tcx(), def_id); @@ -139,7 +141,10 @@ impl<'a, 'tcx> TransItem<'tcx> { &format!("symbol `{}` is already defined", symbol_name)) }); - unsafe { llvm::LLVMRustSetLinkage(g, linkage) }; + unsafe { + llvm::LLVMRustSetLinkage(g, linkage); + llvm::LLVMRustSetVisibility(g, visibility); + } ccx.instances().borrow_mut().insert(instance, g); ccx.statics().borrow_mut().insert(g, def_id); @@ -148,6 +153,7 @@ impl<'a, 'tcx> TransItem<'tcx> { fn predefine_fn(ccx: &CrateContext<'a, 'tcx>, instance: Instance<'tcx>, linkage: llvm::Linkage, + visibility: llvm::Visibility, symbol_name: &str) { assert!(!instance.substs.needs_infer() && !instance.substs.has_param_types()); @@ -172,6 +178,10 @@ impl<'a, 'tcx> TransItem<'tcx> { unsafe { llvm::LLVMRustSetVisibility(lldecl, llvm::Visibility::Hidden); } + } else { + unsafe { + llvm::LLVMRustSetVisibility(lldecl, visibility); + } } debug!("predefine_fn: mono_ty = {:?} instance = {:?}", mono_ty, instance); From c93e62b2c573d2e82d05ce1f88c7344f6e253794 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 13 Jul 2017 13:29:14 +0200 Subject: [PATCH 3/6] Adapt cgu-partitioning tests to pre-trans symbol internalization. --- .../partitioning/extern-drop-glue.rs | 4 +- .../partitioning/extern-generic.rs | 12 +++--- .../inlining-from-extern-crate.rs | 4 +- .../partitioning/local-drop-glue.rs | 4 +- .../partitioning/local-generic.rs | 8 ++-- .../partitioning/local-inlining.rs | 6 +-- .../partitioning/local-transitive-inlining.rs | 4 +- .../partitioning/regular-modules.rs | 42 +++++++++---------- .../codegen-units/partitioning/statics.rs | 20 ++++----- .../partitioning/vtable-through-const.rs | 2 +- 10 files changed, 53 insertions(+), 53 deletions(-) diff --git a/src/test/codegen-units/partitioning/extern-drop-glue.rs b/src/test/codegen-units/partitioning/extern-drop-glue.rs index f28c4872111c9..4e6ae167024e3 100644 --- a/src/test/codegen-units/partitioning/extern-drop-glue.rs +++ b/src/test/codegen-units/partitioning/extern-drop-glue.rs @@ -24,7 +24,7 @@ extern crate cgu_extern_drop_glue; struct LocalStruct(cgu_extern_drop_glue::Struct); -//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External] +//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[Internal] fn user() { //~ TRANS_ITEM fn core::ptr[0]::drop_in_place[0] @@ extern_drop_glue[Internal] @@ -36,7 +36,7 @@ mod mod1 { struct LocalStruct(cgu_extern_drop_glue::Struct); - //~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External] + //~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[Internal] fn user() { //~ TRANS_ITEM fn core::ptr[0]::drop_in_place[0] @@ extern_drop_glue-mod1[Internal] diff --git a/src/test/codegen-units/partitioning/extern-generic.rs b/src/test/codegen-units/partitioning/extern-generic.rs index e32c946f8554f..bdb31265e2fb2 100644 --- a/src/test/codegen-units/partitioning/extern-generic.rs +++ b/src/test/codegen-units/partitioning/extern-generic.rs @@ -19,7 +19,7 @@ // aux-build:cgu_generic_function.rs extern crate cgu_generic_function; -//~ TRANS_ITEM fn extern_generic::user[0] @@ extern_generic[External] +//~ TRANS_ITEM fn extern_generic::user[0] @@ extern_generic[Internal] fn user() { let _ = cgu_generic_function::foo("abc"); } @@ -27,7 +27,7 @@ fn user() { mod mod1 { use cgu_generic_function; - //~ TRANS_ITEM fn extern_generic::mod1[0]::user[0] @@ extern_generic-mod1[External] + //~ TRANS_ITEM fn extern_generic::mod1[0]::user[0] @@ extern_generic-mod1[Internal] fn user() { let _ = cgu_generic_function::foo("abc"); } @@ -35,7 +35,7 @@ mod mod1 { mod mod1 { use cgu_generic_function; - //~ TRANS_ITEM fn extern_generic::mod1[0]::mod1[0]::user[0] @@ extern_generic-mod1-mod1[External] + //~ TRANS_ITEM fn extern_generic::mod1[0]::mod1[0]::user[0] @@ extern_generic-mod1-mod1[Internal] fn user() { let _ = cgu_generic_function::foo("abc"); } @@ -45,18 +45,18 @@ mod mod1 { mod mod2 { use cgu_generic_function; - //~ TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[External] + //~ TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[Internal] fn user() { let _ = cgu_generic_function::foo("abc"); } } mod mod3 { - //~ TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[External] + //~ TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[Internal] fn non_user() {} } // Make sure the two generic functions from the extern crate get instantiated // once for the current crate //~ TRANS_ITEM fn cgu_generic_function::foo[0]<&str> @@ cgu_generic_function.volatile[External] -//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ cgu_generic_function.volatile[External] +//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ cgu_generic_function.volatile[Internal] 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 118513f65541b..20920c9ebe432 100644 --- a/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs +++ b/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs @@ -37,7 +37,7 @@ pub fn user() mod mod1 { use cgu_explicit_inlining; - //~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[External] + //~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[Internal] pub fn user() { cgu_explicit_inlining::inlined(); @@ -50,7 +50,7 @@ mod mod1 { mod mod2 { use cgu_explicit_inlining; - //~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[External] + //~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[Internal] pub fn user() { cgu_explicit_inlining::always_inlined(); diff --git a/src/test/codegen-units/partitioning/local-drop-glue.rs b/src/test/codegen-units/partitioning/local-drop-glue.rs index 64f4f854c2d7f..d2ce847e108f0 100644 --- a/src/test/codegen-units/partitioning/local-drop-glue.rs +++ b/src/test/codegen-units/partitioning/local-drop-glue.rs @@ -31,7 +31,7 @@ struct Outer { _a: Struct } -//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[External] +//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[Internal] fn user() { let _ = Outer { @@ -52,7 +52,7 @@ mod mod1 _b: (u32, Struct), } - //~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[External] + //~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[Internal] fn user() { let _ = Struct2 { diff --git a/src/test/codegen-units/partitioning/local-generic.rs b/src/test/codegen-units/partitioning/local-generic.rs index 70fc75c6970b7..33e3745502fcf 100644 --- a/src/test/codegen-units/partitioning/local-generic.rs +++ b/src/test/codegen-units/partitioning/local-generic.rs @@ -22,7 +22,7 @@ //~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[External] pub fn generic(x: T) -> T { x } -//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External] +//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[Internal] fn user() { let _ = generic(0u32); } @@ -30,7 +30,7 @@ fn user() { mod mod1 { pub use super::generic; - //~ TRANS_ITEM fn local_generic::mod1[0]::user[0] @@ local_generic-mod1[External] + //~ TRANS_ITEM fn local_generic::mod1[0]::user[0] @@ local_generic-mod1[Internal] fn user() { let _ = generic(0u64); } @@ -38,7 +38,7 @@ mod mod1 { mod mod1 { use super::generic; - //~ TRANS_ITEM fn local_generic::mod1[0]::mod1[0]::user[0] @@ local_generic-mod1-mod1[External] + //~ TRANS_ITEM fn local_generic::mod1[0]::mod1[0]::user[0] @@ local_generic-mod1-mod1[Internal] fn user() { let _ = generic('c'); } @@ -48,7 +48,7 @@ mod mod1 { mod mod2 { use super::generic; - //~ TRANS_ITEM fn local_generic::mod2[0]::user[0] @@ local_generic-mod2[External] + //~ TRANS_ITEM fn local_generic::mod2[0]::user[0] @@ local_generic-mod2[Internal] fn user() { let _ = generic("abc"); } diff --git a/src/test/codegen-units/partitioning/local-inlining.rs b/src/test/codegen-units/partitioning/local-inlining.rs index 84ca8b1b0f641..a4d9e60d2280c 100644 --- a/src/test/codegen-units/partitioning/local-inlining.rs +++ b/src/test/codegen-units/partitioning/local-inlining.rs @@ -30,7 +30,7 @@ mod inline { mod user1 { use super::inline; - //~ TRANS_ITEM fn local_inlining::user1[0]::foo[0] @@ local_inlining-user1[External] + //~ TRANS_ITEM fn local_inlining::user1[0]::foo[0] @@ local_inlining-user1[Internal] fn foo() { inline::inlined_function(); } @@ -39,7 +39,7 @@ mod user1 { mod user2 { use super::inline; - //~ TRANS_ITEM fn local_inlining::user2[0]::bar[0] @@ local_inlining-user2[External] + //~ TRANS_ITEM fn local_inlining::user2[0]::bar[0] @@ local_inlining-user2[Internal] fn bar() { inline::inlined_function(); } @@ -47,7 +47,7 @@ mod user2 { mod non_user { - //~ TRANS_ITEM fn local_inlining::non_user[0]::baz[0] @@ local_inlining-non_user[External] + //~ TRANS_ITEM fn local_inlining::non_user[0]::baz[0] @@ local_inlining-non_user[Internal] fn baz() { } diff --git a/src/test/codegen-units/partitioning/local-transitive-inlining.rs b/src/test/codegen-units/partitioning/local-transitive-inlining.rs index 7e37f9ccaa5a9..1beaa186d9ee5 100644 --- a/src/test/codegen-units/partitioning/local-transitive-inlining.rs +++ b/src/test/codegen-units/partitioning/local-transitive-inlining.rs @@ -39,7 +39,7 @@ mod direct_user { mod indirect_user { use super::direct_user; - //~ TRANS_ITEM fn local_transitive_inlining::indirect_user[0]::bar[0] @@ local_transitive_inlining-indirect_user[External] + //~ TRANS_ITEM fn local_transitive_inlining::indirect_user[0]::bar[0] @@ local_transitive_inlining-indirect_user[Internal] fn bar() { direct_user::foo(); } @@ -47,7 +47,7 @@ mod indirect_user { mod non_user { - //~ TRANS_ITEM fn local_transitive_inlining::non_user[0]::baz[0] @@ local_transitive_inlining-non_user[External] + //~ TRANS_ITEM fn local_transitive_inlining::non_user[0]::baz[0] @@ local_transitive_inlining-non_user[Internal] fn baz() { } diff --git a/src/test/codegen-units/partitioning/regular-modules.rs b/src/test/codegen-units/partitioning/regular-modules.rs index 07c341203f9e2..9bdbc8b85e8b8 100644 --- a/src/test/codegen-units/partitioning/regular-modules.rs +++ b/src/test/codegen-units/partitioning/regular-modules.rs @@ -16,67 +16,67 @@ #![allow(dead_code)] #![crate_type="lib"] -//~ TRANS_ITEM fn regular_modules::foo[0] @@ regular_modules[External] +//~ TRANS_ITEM fn regular_modules::foo[0] @@ regular_modules[Internal] fn foo() {} -//~ TRANS_ITEM fn regular_modules::bar[0] @@ regular_modules[External] +//~ TRANS_ITEM fn regular_modules::bar[0] @@ regular_modules[Internal] fn bar() {} -//~ TRANS_ITEM static regular_modules::BAZ[0] @@ regular_modules[External] +//~ TRANS_ITEM static regular_modules::BAZ[0] @@ regular_modules[Internal] static BAZ: u64 = 0; mod mod1 { - //~ TRANS_ITEM fn regular_modules::mod1[0]::foo[0] @@ regular_modules-mod1[External] + //~ TRANS_ITEM fn regular_modules::mod1[0]::foo[0] @@ regular_modules-mod1[Internal] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod1[0]::bar[0] @@ regular_modules-mod1[External] + //~ TRANS_ITEM fn regular_modules::mod1[0]::bar[0] @@ regular_modules-mod1[Internal] fn bar() {} - //~ TRANS_ITEM static regular_modules::mod1[0]::BAZ[0] @@ regular_modules-mod1[External] + //~ TRANS_ITEM static regular_modules::mod1[0]::BAZ[0] @@ regular_modules-mod1[Internal] static BAZ: u64 = 0; mod mod1 { - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::foo[0] @@ regular_modules-mod1-mod1[External] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::foo[0] @@ regular_modules-mod1-mod1[Internal] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::bar[0] @@ regular_modules-mod1-mod1[External] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::bar[0] @@ regular_modules-mod1-mod1[Internal] fn bar() {} - //~ TRANS_ITEM static regular_modules::mod1[0]::mod1[0]::BAZ[0] @@ regular_modules-mod1-mod1[External] + //~ TRANS_ITEM static regular_modules::mod1[0]::mod1[0]::BAZ[0] @@ regular_modules-mod1-mod1[Internal] static BAZ: u64 = 0; } mod mod2 { - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::foo[0] @@ regular_modules-mod1-mod2[External] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::foo[0] @@ regular_modules-mod1-mod2[Internal] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::bar[0] @@ regular_modules-mod1-mod2[External] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::bar[0] @@ regular_modules-mod1-mod2[Internal] fn bar() {} - //~ TRANS_ITEM static regular_modules::mod1[0]::mod2[0]::BAZ[0] @@ regular_modules-mod1-mod2[External] + //~ TRANS_ITEM static regular_modules::mod1[0]::mod2[0]::BAZ[0] @@ regular_modules-mod1-mod2[Internal] static BAZ: u64 = 0; } } mod mod2 { - //~ TRANS_ITEM fn regular_modules::mod2[0]::foo[0] @@ regular_modules-mod2[External] + //~ TRANS_ITEM fn regular_modules::mod2[0]::foo[0] @@ regular_modules-mod2[Internal] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod2[0]::bar[0] @@ regular_modules-mod2[External] + //~ TRANS_ITEM fn regular_modules::mod2[0]::bar[0] @@ regular_modules-mod2[Internal] fn bar() {} - //~ TRANS_ITEM static regular_modules::mod2[0]::BAZ[0] @@ regular_modules-mod2[External] + //~ TRANS_ITEM static regular_modules::mod2[0]::BAZ[0] @@ regular_modules-mod2[Internal] static BAZ: u64 = 0; mod mod1 { - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::foo[0] @@ regular_modules-mod2-mod1[External] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::foo[0] @@ regular_modules-mod2-mod1[Internal] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::bar[0] @@ regular_modules-mod2-mod1[External] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::bar[0] @@ regular_modules-mod2-mod1[Internal] fn bar() {} - //~ TRANS_ITEM static regular_modules::mod2[0]::mod1[0]::BAZ[0] @@ regular_modules-mod2-mod1[External] + //~ TRANS_ITEM static regular_modules::mod2[0]::mod1[0]::BAZ[0] @@ regular_modules-mod2-mod1[Internal] static BAZ: u64 = 0; } mod mod2 { - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::foo[0] @@ regular_modules-mod2-mod2[External] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::foo[0] @@ regular_modules-mod2-mod2[Internal] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::bar[0] @@ regular_modules-mod2-mod2[External] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::bar[0] @@ regular_modules-mod2-mod2[Internal] fn bar() {} - //~ TRANS_ITEM static regular_modules::mod2[0]::mod2[0]::BAZ[0] @@ regular_modules-mod2-mod2[External] + //~ TRANS_ITEM static regular_modules::mod2[0]::mod2[0]::BAZ[0] @@ regular_modules-mod2-mod2[Internal] static BAZ: u64 = 0; } } diff --git a/src/test/codegen-units/partitioning/statics.rs b/src/test/codegen-units/partitioning/statics.rs index d06b3ac407a7f..8cbce12b52cad 100644 --- a/src/test/codegen-units/partitioning/statics.rs +++ b/src/test/codegen-units/partitioning/statics.rs @@ -15,34 +15,34 @@ #![crate_type="lib"] -//~ TRANS_ITEM static statics::FOO[0] @@ statics[External] +//~ TRANS_ITEM static statics::FOO[0] @@ statics[Internal] static FOO: u32 = 0; -//~ TRANS_ITEM static statics::BAR[0] @@ statics[External] +//~ TRANS_ITEM static statics::BAR[0] @@ statics[Internal] static BAR: u32 = 0; -//~ TRANS_ITEM fn statics::function[0] @@ statics[External] +//~ TRANS_ITEM fn statics::function[0] @@ statics[Internal] fn function() { - //~ TRANS_ITEM static statics::function[0]::FOO[0] @@ statics[External] + //~ TRANS_ITEM static statics::function[0]::FOO[0] @@ statics[Internal] static FOO: u32 = 0; - //~ TRANS_ITEM static statics::function[0]::BAR[0] @@ statics[External] + //~ TRANS_ITEM static statics::function[0]::BAR[0] @@ statics[Internal] static BAR: u32 = 0; } mod mod1 { - //~ TRANS_ITEM static statics::mod1[0]::FOO[0] @@ statics-mod1[External] + //~ TRANS_ITEM static statics::mod1[0]::FOO[0] @@ statics-mod1[Internal] static FOO: u32 = 0; - //~ TRANS_ITEM static statics::mod1[0]::BAR[0] @@ statics-mod1[External] + //~ TRANS_ITEM static statics::mod1[0]::BAR[0] @@ statics-mod1[Internal] static BAR: u32 = 0; - //~ TRANS_ITEM fn statics::mod1[0]::function[0] @@ statics-mod1[External] + //~ TRANS_ITEM fn statics::mod1[0]::function[0] @@ statics-mod1[Internal] fn function() { - //~ TRANS_ITEM static statics::mod1[0]::function[0]::FOO[0] @@ statics-mod1[External] + //~ TRANS_ITEM static statics::mod1[0]::function[0]::FOO[0] @@ statics-mod1[Internal] static FOO: u32 = 0; - //~ TRANS_ITEM static statics::mod1[0]::function[0]::BAR[0] @@ statics-mod1[External] + //~ TRANS_ITEM static statics::mod1[0]::function[0]::BAR[0] @@ statics-mod1[Internal] static BAR: u32 = 0; } } diff --git a/src/test/codegen-units/partitioning/vtable-through-const.rs b/src/test/codegen-units/partitioning/vtable-through-const.rs index c4594bb547ef5..74f2f84356701 100644 --- a/src/test/codegen-units/partitioning/vtable-through-const.rs +++ b/src/test/codegen-units/partitioning/vtable-through-const.rs @@ -67,7 +67,7 @@ mod mod1 { pub const ID_I64: fn(i64) -> i64 = id::; } -//~ TRANS_ITEM fn vtable_through_const::main[0] @@ vtable_through_const[External] +//~ TRANS_ITEM fn vtable_through_const::main[0] @@ vtable_through_const[Internal] fn main() { //~ TRANS_ITEM fn core::ptr[0]::drop_in_place[0] @@ vtable_through_const[Internal] From 678d37700d76b83c810143407769f7f481115b40 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 13 Jul 2017 14:43:56 +0200 Subject: [PATCH 4/6] Address some nits in trans-collector and partitioner. --- src/librustc_trans/base.rs | 4 +- src/librustc_trans/collector.rs | 70 +++++++++++++----------------- src/librustc_trans/partitioning.rs | 3 +- 3 files changed, 33 insertions(+), 44 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 3443160bded76..7b836399f9cb5 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1301,7 +1301,9 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a let (items, inlining_map) = time(time_passes, "translation item collection", || { - collector::collect_crate_translation_items(&scx, collection_mode) + collector::collect_crate_translation_items(&scx, + exported_symbols, + collection_mode) }); assert_symbols_are_distinct(scx.tcx(), items.iter()); diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 4e1870be56132..6a573b76eecdc 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -209,6 +209,7 @@ use rustc::util::nodemap::{FxHashSet, FxHashMap, DefIdMap}; use trans_item::{TransItem, DefPathBasedNames, InstantiationMode}; use rustc_data_structures::bitvec::BitVector; +use back::symbol_export::ExportedSymbols; #[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)] pub enum TransItemCollectionMode { @@ -293,13 +294,14 @@ impl<'tcx> InliningMap<'tcx> { } pub fn collect_crate_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, + exported_symbols: &ExportedSymbols, mode: TransItemCollectionMode) -> (FxHashSet>, InliningMap<'tcx>) { // We are not tracking dependencies of this pass as it has to be re-executed // every time no matter what. scx.tcx().dep_graph.with_ignore(|| { - let roots = collect_roots(scx, mode); + let roots = collect_roots(scx, exported_symbols, mode); debug!("Building translation item graph, beginning at roots"); let mut visited = FxHashSet(); @@ -321,6 +323,7 @@ pub fn collect_crate_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 't // Find all non-generic items by walking the HIR. These items serve as roots to // start monomorphizing from. fn collect_roots<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, + exported_symbols: &ExportedSymbols, mode: TransItemCollectionMode) -> Vec> { debug!("Collecting roots"); @@ -330,6 +333,7 @@ fn collect_roots<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, let mut visitor = RootCollector { scx: scx, mode: mode, + exported_symbols, output: &mut roots, }; @@ -853,6 +857,7 @@ fn create_trans_items_for_vtable_methods<'a, 'tcx>(scx: &SharedCrateContext<'a, struct RootCollector<'b, 'a: 'b, 'tcx: 'a + 'b> { scx: &'b SharedCrateContext<'a, 'tcx>, + exported_symbols: &'b ExportedSymbols, mode: TransItemCollectionMode, output: &'b mut Vec>, } @@ -908,20 +913,19 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> { // const items only generate translation items if they are // actually used somewhere. Just declaring them is insufficient. } - hir::ItemFn(_, _, constness, _, ref generics, _) => { - let is_const = match constness { - hir::Constness::Const => true, - hir::Constness::NotConst => false, - }; + hir::ItemFn(..) => { + let tcx = self.scx.tcx(); + let def_id = tcx.hir.local_def_id(item.id); - if !generics.is_type_parameterized() && - (!is_const || self.mode == TransItemCollectionMode::Eager) { - let def_id = self.scx.tcx().hir.local_def_id(item.id); + if (self.mode == TransItemCollectionMode::Eager || + !tcx.is_const_fn(def_id) || + self.exported_symbols.local_exports().contains(&item.id)) && + !item_has_type_parameters(tcx, def_id) { debug!("RootCollector: ItemFn({})", - def_id_to_string(self.scx.tcx(), def_id)); + def_id_to_string(tcx, def_id)); - let instance = Instance::mono(self.scx.tcx(), def_id); + let instance = Instance::mono(tcx, def_id); self.output.push(TransItem::Fn(instance)); } } @@ -935,39 +939,18 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> { fn visit_impl_item(&mut self, ii: &'v hir::ImplItem) { match ii.node { - hir::ImplItemKind::Method(hir::MethodSig { - constness, - ref generics, - .. - }, _) => { - let hir_map = &self.scx.tcx().hir; - let parent_node_id = hir_map.get_parent_node(ii.id); - let is_impl_generic = || match hir_map.expect_item(parent_node_id) { - &hir::Item { - node: hir::ItemImpl(_, _, _, ref generics, ..), - .. - } => { - generics.is_type_parameterized() - } - _ => { - bug!() - } - }; - - let is_const = match constness { - hir::Constness::Const => true, - hir::Constness::NotConst => false, - }; - - if (!is_const || self.mode == TransItemCollectionMode::Eager) && - !generics.is_type_parameterized() && - !is_impl_generic() { - let def_id = self.scx.tcx().hir.local_def_id(ii.id); + hir::ImplItemKind::Method(hir::MethodSig { .. }, _) => { + let tcx = self.scx.tcx(); + let def_id = tcx.hir.local_def_id(ii.id); + if (self.mode == TransItemCollectionMode::Eager || + !tcx.is_const_fn(def_id) || + self.exported_symbols.local_exports().contains(&ii.id)) && + !item_has_type_parameters(tcx, def_id) { debug!("RootCollector: MethodImplItem({})", - def_id_to_string(self.scx.tcx(), def_id)); + def_id_to_string(tcx, def_id)); - let instance = Instance::mono(self.scx.tcx(), def_id); + let instance = Instance::mono(tcx, def_id); self.output.push(TransItem::Fn(instance)); } } @@ -976,6 +959,11 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> { } } +fn item_has_type_parameters<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> bool { + let generics = tcx.generics_of(def_id); + generics.parent_types as usize + generics.types.len() > 0 +} + fn create_trans_items_for_default_impls<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, item: &'tcx hir::Item, output: &mut Vec>) { diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 5e445652dc473..1ff21bfdd94e3 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -576,7 +576,6 @@ fn internalize_symbols<'a, 'tcx>(_tcx: TyCtxt<'a, 'tcx, 'tcx>, cgu_name: cgu.name.clone() }; - 'item: for (accessee, &mut (ref mut linkage, _)) in &mut cgu.items { if !partitioning.internalization_candidates.contains(accessee) { // This item is no candidate for internalizing, so skip it. @@ -594,7 +593,7 @@ fn internalize_symbols<'a, 'tcx>(_tcx: TyCtxt<'a, 'tcx, 'tcx>, .any(|placement| *placement != home_cgu) { // Found an accessor from another CGU, so skip to the next // item without marking this one as internal. - continue 'item; + continue } } From 226bc92b646f905f43b8cab0d89e681709be9ede Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 17 Jul 2017 15:55:30 +0200 Subject: [PATCH 5/6] partitioning: Fix visibility of internalized symbols. --- src/librustc_trans/partitioning.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 1ff21bfdd94e3..7518948323b8b 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -576,7 +576,7 @@ fn internalize_symbols<'a, 'tcx>(_tcx: TyCtxt<'a, 'tcx, 'tcx>, cgu_name: cgu.name.clone() }; - for (accessee, &mut (ref mut linkage, _)) in &mut cgu.items { + for (accessee, linkage_and_visibility) in &mut cgu.items { if !partitioning.internalization_candidates.contains(accessee) { // This item is no candidate for internalizing, so skip it. continue @@ -599,7 +599,7 @@ fn internalize_symbols<'a, 'tcx>(_tcx: TyCtxt<'a, 'tcx, 'tcx>, // If we got here, we did not find any accesses from other CGUs, // so it's fine to make this translation item internal. - *linkage = llvm::InternalLinkage; + *linkage_and_visibility = (llvm::InternalLinkage, llvm::Visibility::Default); } } } From f6e5416a2f98685870ca6912beea42c00a446802 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 20 Jul 2017 14:10:50 +0200 Subject: [PATCH 6/6] trans: Make the collector search const fn invocations. --- src/librustc_trans/collector.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 6a573b76eecdc..b31295f4022ed 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -594,14 +594,25 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { match *kind { mir::TerminatorKind::Call { ref func, .. } => { let callee_ty = func.ty(self.mir, tcx); + let callee_ty = tcx.trans_apply_param_substs(self.param_substs, &callee_ty); - let skip_const = self.const_context && match callee_ty.sty { - ty::TyFnDef(def_id, _) => self.scx.tcx().is_const_fn(def_id), - _ => false + let constness = match (self.const_context, &callee_ty.sty) { + (true, &ty::TyFnDef(def_id, substs)) if self.scx.tcx().is_const_fn(def_id) => { + let instance = monomorphize::resolve(self.scx, def_id, substs); + Some(instance) + } + _ => None }; - if !skip_const { - let callee_ty = tcx.trans_apply_param_substs(self.param_substs, &callee_ty); + if let Some(const_fn_instance) = constness { + // If this is a const fn, called from a const context, we + // have to visit its body in order to find any fn reifications + // it might contain. + collect_neighbours(self.scx, + const_fn_instance, + true, + self.output); + } else { visit_fn_use(self.scx, callee_ty, true, &mut self.output); } }