From 2eada5870615fafa1f40c4ce93f5f5adaf030cdf Mon Sep 17 00:00:00 2001 From: Alex Crichton <alex@alexcrichton.com> Date: Wed, 13 Sep 2017 15:24:13 -0700 Subject: [PATCH] rustc: Remove another global map from trans This commit removes the `crate_trans_items` field from the `CrateContext` of trans. This field, a big map, was calculated during partioning and was a set of all translation items. This isn't quite incremental-friendly because the map may change a lot but not have much effect on downstream consumers. Instead a new query was added for the one location this map was needed, along with a new comment explaining what the location is doing! --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/ty/maps.rs | 5 ++-- src/librustc_trans/base.rs | 48 +++++++++++++++++++++--------- src/librustc_trans/callee.rs | 36 ++++++++++++++++++++-- src/librustc_trans/context.rs | 14 ++------- src/librustc_trans/lib.rs | 3 +- 6 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index bfbf280d6dd..e3a9e969864 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -579,6 +579,7 @@ define_dep_nodes!( <'tcx> [] CollectAndPartitionTranslationItems, [] ExportName(DefId), [] ContainsExternIndicator(DefId), + [] IsTranslatedFunction(DefId), ); trait DepNodeParams<'a, 'gcx: 'tcx + 'a, 'tcx: 'a> : fmt::Debug { diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index dd6d90503ef..307921a3335 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -24,7 +24,7 @@ use middle::resolve_lifetime::{Region, ObjectLifetimeDefault}; use middle::stability::{self, DeprecationEntry}; use middle::lang_items::{LanguageItems, LangItem}; use middle::exported_symbols::SymbolExportLevel; -use middle::trans::{TransItem, CodegenUnit}; +use middle::trans::CodegenUnit; use mir; use mir::transform::{MirSuite, MirPassIndex}; use session::CompileResult; @@ -1391,9 +1391,10 @@ define_maps! { <'tcx> -> Arc<Vec<(String, DefId, SymbolExportLevel)>>, [] fn collect_and_partition_translation_items: collect_and_partition_translation_items_node(CrateNum) - -> (Arc<FxHashSet<TransItem<'tcx>>>, Vec<Arc<CodegenUnit<'tcx>>>), + -> (Arc<DefIdSet>, Arc<Vec<Arc<CodegenUnit<'tcx>>>>), [] fn export_name: ExportName(DefId) -> Option<Symbol>, [] fn contains_extern_indicator: ContainsExternIndicator(DefId) -> bool, + [] fn is_translated_function: IsTranslatedFunction(DefId) -> bool, } fn type_param_predicates<'tcx>((item_id, param_id): (DefId, DefId)) -> DepConstructor<'tcx> { diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 7941134cc7b..f60f069f4ea 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -35,7 +35,7 @@ use back::write::{self, OngoingCrateTranslation}; use llvm::{ContextRef, ModuleRef, ValueRef, Vector, get_param}; use llvm; use metadata; -use rustc::hir::def_id::{CrateNum, LOCAL_CRATE}; +use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::middle::lang_items::StartFnLangItem; use rustc::middle::trans::{Linkage, Visibility}; use rustc::middle::cstore::{EncodedMetadata, EncodedMetadataHashes}; @@ -75,7 +75,7 @@ use trans_item::{TransItem, TransItemExt, DefPathBasedNames}; use type_::Type; use type_of; use value::Value; -use rustc::util::nodemap::{NodeSet, FxHashMap, FxHashSet}; +use rustc::util::nodemap::{NodeSet, FxHashMap, FxHashSet, DefIdSet}; use CrateInfo; use libc::c_uint; @@ -990,8 +990,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Run the translation item collector and partition the collected items into // codegen units. - let (translation_items, codegen_units) = - shared_ccx.tcx().collect_and_partition_translation_items(LOCAL_CRATE); + let codegen_units = + shared_ccx.tcx().collect_and_partition_translation_items(LOCAL_CRATE).1; + let codegen_units = (*codegen_units).clone(); assert!(codegen_units.len() <= 1 || !tcx.sess.lto()); @@ -1076,8 +1077,7 @@ 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, - translation_items.clone())), + AssertDepGraphSafe(cgu), module_translation); all_stats.extend(stats); @@ -1118,13 +1118,12 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn module_translation<'a, 'tcx>( scx: AssertDepGraphSafe<&SharedCrateContext<'a, 'tcx>>, - args: AssertDepGraphSafe<(Arc<CodegenUnit<'tcx>>, - Arc<FxHashSet<TransItem<'tcx>>>)>) + args: AssertDepGraphSafe<Arc<CodegenUnit<'tcx>>>) -> (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, crate_trans_items)) = args; + let AssertDepGraphSafe(cgu) = args; let cgu_name = cgu.name().to_string(); let cgu_id = cgu.work_product_id(); @@ -1164,7 +1163,7 @@ 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, crate_trans_items); + let lcx = LocalCrateContext::new(scx, cgu); let module = { let ccx = CrateContext::new(scx, &lcx); let trans_items = ccx.codegen_unit() @@ -1353,7 +1352,7 @@ fn assert_symbols_are_distinct<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_i fn collect_and_partition_translation_items<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum, -) -> (Arc<FxHashSet<TransItem<'tcx>>>, Vec<Arc<CodegenUnit<'tcx>>>) +) -> (Arc<DefIdSet>, Arc<Vec<Arc<CodegenUnit<'tcx>>>>) { assert_eq!(cnum, LOCAL_CRATE); let time_passes = tcx.sess.time_passes(); @@ -1404,7 +1403,12 @@ fn collect_and_partition_translation_items<'a, 'tcx>( assert!(tcx.sess.opts.cg.codegen_units == codegen_units.len() || tcx.sess.opts.debugging_opts.incremental.is_some()); - let translation_items: FxHashSet<TransItem<'tcx>> = items.iter().cloned().collect(); + let translation_items: DefIdSet = items.iter().filter_map(|trans_item| { + match *trans_item { + TransItem::Fn(ref instance) => Some(instance.def_id()), + _ => None, + } + }).collect(); if tcx.sess.opts.debugging_opts.print_trans_items.is_some() { let mut item_to_cgus = FxHashMap(); @@ -1459,7 +1463,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>( } } - (Arc::new(translation_items), codegen_units) + (Arc::new(translation_items), Arc::new(codegen_units)) } impl CrateInfo { @@ -1505,9 +1509,25 @@ impl CrateInfo { } } -pub fn provide(providers: &mut Providers) { +fn is_translated_function(tcx: TyCtxt, id: DefId) -> bool { + // FIXME(#42293) needs red/green tracking to avoid failing a bunch of + // existing tests + tcx.dep_graph.with_ignore(|| { + let (all_trans_items, _) = + tcx.collect_and_partition_translation_items(LOCAL_CRATE); + all_trans_items.contains(&id) + }) +} + +pub fn provide_local(providers: &mut Providers) { providers.collect_and_partition_translation_items = collect_and_partition_translation_items; + + providers.is_translated_function = is_translated_function; +} + +pub fn provide_extern(providers: &mut Providers) { + providers.is_translated_function = is_translated_function; } pub fn linkage_to_llvm(linkage: Linkage) -> llvm::Linkage { diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index b62d60db23a..52e6dce24ed 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -23,7 +23,6 @@ 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 @@ -109,10 +108,43 @@ pub fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, attributes::unwind(llfn, true); } + // Apply an appropriate linkage/visibility value to our item that we + // just declared. + // + // This is sort of subtle. Inside our codegen unit we started off + // compilation by predefining all our own `TransItem` instances. That + // is, everything we're translating ourselves is already defined. That + // means that anything we're actually translating ourselves will have + // hit the above branch in `get_declared_value`. As a result, we're + // guaranteed here that we're declaring a symbol that won't get defined, + // or in other words we're referencing a foreign value. + // + // So because this is a foreign value we blanket apply an external + // linkage directive because it's coming from a different object file. + // The visibility here is where it gets tricky. This symbol could be + // referencing some foreign crate or foreign library (an `extern` + // block) in which case we want to leave the default visibility. We may + // also, though, have multiple codegen units. + // + // In the situation of multiple codegen units this function may be + // referencing a function from another codegen unit. If we're + // indeed referencing a symbol in another codegen unit then we're in one + // of two cases: + // + // * This is a symbol defined in a foreign crate and we're just + // monomorphizing in another codegen unit. In this case this symbols + // is for sure not exported, so both codegen units will be using + // hidden visibility. Hence, we apply a hidden visibility here. + // + // * This is a symbol defined in our local crate. If the symbol in the + // other codegen unit is also not exported then like with the foreign + // case we apply a hidden visibility. If the symbol is exported from + // the foreign object file, however, then we leave this at the + // default visibility as we'll just import it naturally. unsafe { llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage); - if ccx.crate_trans_items().contains(&TransItem::Fn(instance)) { + if ccx.tcx().is_translated_function(instance_def_id) { if instance_def_id.is_local() { if !ccx.tcx().is_exported_symbol(instance_def_id) { llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 62dadce76f0..a1666fa5231 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -22,14 +22,13 @@ 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}; use rustc::session::Session; use rustc::ty::{self, Ty, TyCtxt}; use rustc::ty::layout::{LayoutCx, LayoutError, LayoutTyper, TyLayout}; -use rustc::util::nodemap::{FxHashMap, FxHashSet}; +use rustc::util::nodemap::FxHashMap; use std::ffi::{CStr, CString}; use std::cell::{Cell, RefCell}; @@ -94,9 +93,6 @@ pub struct LocalCrateContext<'a, 'tcx: 'a> { stats: Stats, codegen_unit: Arc<CodegenUnit<'tcx>>, - /// The translation items of the whole crate. - crate_trans_items: Arc<FxHashSet<TransItem<'tcx>>>, - /// Cache instances of monomorphic and polymorphic items instances: RefCell<FxHashMap<Instance<'tcx>, ValueRef>>, /// Cache generated vtables @@ -349,8 +345,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { pub fn new(shared: &SharedCrateContext<'a, 'tcx>, - codegen_unit: Arc<CodegenUnit<'tcx>>, - crate_trans_items: Arc<FxHashSet<TransItem<'tcx>>>) + codegen_unit: Arc<CodegenUnit<'tcx>>) -> LocalCrateContext<'a, 'tcx> { unsafe { // Append ".rs" to LLVM module identifier. @@ -382,7 +377,6 @@ impl<'a, 'tcx> LocalCrateContext<'a, 'tcx> { llcx, stats: Stats::default(), codegen_unit, - crate_trans_items, instances: RefCell::new(FxHashMap()), vtables: RefCell::new(FxHashMap()), const_cstr_cache: RefCell::new(FxHashMap()), @@ -489,10 +483,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.local().codegen_unit } - pub fn crate_trans_items(&self) -> &FxHashSet<TransItem<'tcx>> { - &self.local().crate_trans_items - } - pub fn td(&self) -> llvm::TargetDataRef { unsafe { llvm::LLVMRustGetModuleDataLayout(self.llmod()) } } diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index d06c769458a..4cbc98d26de 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -251,10 +251,11 @@ __build_diagnostic_array! { librustc_trans, DIAGNOSTICS } pub fn provide_local(providers: &mut Providers) { back::symbol_names::provide(providers); back::symbol_export::provide_local(providers); - base::provide(providers); + base::provide_local(providers); } pub fn provide_extern(providers: &mut Providers) { back::symbol_names::provide(providers); back::symbol_export::provide_extern(providers); + base::provide_extern(providers); }