From 891c2a082fd562744b4495ed7bd0602414e93f2b Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 6 May 2016 20:02:09 -0400 Subject: [PATCH 01/21] trans: Make translation of statics collector-driven. --- src/librustc_trans/base.rs | 46 ++++-- src/librustc_trans/consts.rs | 52 ++++--- src/librustc_trans/trans_item.rs | 258 ++++++++++++++++++------------- 3 files changed, 220 insertions(+), 136 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 590220f0c8b..40ae5025180 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2249,8 +2249,10 @@ pub fn update_linkage(ccx: &CrateContext, } } -fn set_global_section(ccx: &CrateContext, llval: ValueRef, i: &hir::Item) { - if let Some(sect) = attr::first_attr_value_str_by_name(&i.attrs, "link_section") { +pub fn set_link_section(ccx: &CrateContext, + llval: ValueRef, + attrs: &[ast::Attribute]) { + if let Some(sect) = attr::first_attr_value_str_by_name(attrs, "link_section") { if contains_null(§) { ccx.sess().fatal(&format!("Illegal null byte in link_section value: `{}`", §)); } @@ -2280,7 +2282,7 @@ pub fn trans_item(ccx: &CrateContext, item: &hir::Item) { let empty_substs = ccx.empty_substs_for_def_id(def_id); let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; trans_fn(ccx, &decl, &body, llfn, empty_substs, item.id); - set_global_section(ccx, llfn, item); + set_link_section(ccx, llfn, &item.attrs); update_linkage(ccx, llfn, Some(item.id), @@ -2336,13 +2338,9 @@ pub fn trans_item(ccx: &CrateContext, item: &hir::Item) { enum_variant_size_lint(ccx, enum_definition, item.span, item.id); } } - hir::ItemStatic(_, m, ref expr) => { - let g = match consts::trans_static(ccx, m, expr, item.id, &item.attrs) { - Ok(g) => g, - Err(err) => ccx.tcx().sess.span_fatal(expr.span, &err.description()), - }; - set_global_section(ccx, g, item); - update_linkage(ccx, g, Some(item.id), OriginalTranslation); + hir::ItemStatic(..) => { + // Don't do anything here. Translation of statics has been moved to + // being "collector-driven". } _ => {} } @@ -2700,6 +2698,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let codegen_units = collect_and_partition_translation_items(&shared_ccx); let codegen_unit_count = codegen_units.len(); + assert!(tcx.sess.opts.cg.codegen_units == codegen_unit_count || tcx.sess.opts.debugging_opts.incremental.is_some()); @@ -2723,6 +2722,33 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; } + // Instantiate translation items without filling out definitions yet... + for ccx in crate_context_list.iter() { + for (&trans_item, &linkage) in &ccx.codegen_unit().items { + trans_item.predefine(&ccx, linkage); + } + } + + // ... and now that we have everything pre-defined, fill out those definitions. + for ccx in crate_context_list.iter() { + for (&trans_item, _) in &ccx.codegen_unit().items { + match trans_item { + TransItem::Static(node_id) => { + let item = ccx.tcx().map.expect_item(node_id); + if let hir::ItemStatic(_, m, ref expr) = item.node { + match consts::trans_static(&ccx, m, expr, item.id, &item.attrs) { + Ok(_) => { /* Cool, everything's alright. */ }, + Err(err) => ccx.tcx().sess.span_fatal(expr.span, &err.description()), + }; + } else { + span_bug!(item.span, "Mismatch between hir::Item type and TransItem type") + } + } + _ => { } + } + } + } + { let ccx = crate_context_list.get_ccx(0); diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index 5596ab0d819..1a38baeff37 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -1017,22 +1017,29 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) let g = if let Some(id) = ccx.tcx().map.as_local_node_id(def_id) { let llty = type_of::type_of(ccx, ty); - match ccx.tcx().map.get(id) { + let (g, attrs) = match ccx.tcx().map.get(id) { hir_map::NodeItem(&hir::Item { - span, node: hir::ItemStatic(..), .. + ref attrs, span, node: hir::ItemStatic(..), .. }) => { - // If this static came from an external crate, then - // we need to get the symbol from metadata instead of - // using the current crate's name/version - // information in the hash of the symbol - debug!("making {}", sym); + // Make sure that this is never executed for something inlined. + assert!(!ccx.external_srcs().borrow().contains_key(&id)); - // Create the global before evaluating the initializer; - // this is necessary to allow recursive statics. - declare::define_global(ccx, &sym, llty).unwrap_or_else(|| { - ccx.sess().span_fatal(span, - &format!("symbol `{}` is already defined", sym)) - }) + let defined_in_current_codegen_unit = ccx.codegen_unit() + .items + .contains_key(&TransItem::Static(id)); + if defined_in_current_codegen_unit { + if declare::get_declared_value(ccx, &sym).is_none() { + span_bug!(span, "trans: Static not properly pre-defined?"); + } + } else { + if declare::get_declared_value(ccx, &sym).is_some() { + span_bug!(span, "trans: Conflicting symbol names for static?"); + } + } + + let g = declare::define_global(ccx, &sym, llty).unwrap(); + + (g, attrs) } hir_map::NodeForeignItem(&hir::ForeignItem { @@ -1083,17 +1090,19 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) declare::declare_global(ccx, &sym, llty) }; - for attr in attrs { - if attr.check_name("thread_local") { - llvm::set_thread_local(g, true); - } - } - - g + (g, attrs) } item => bug!("get_static: expected static, found {:?}", item) + }; + + for attr in attrs { + if attr.check_name("thread_local") { + llvm::set_thread_local(g, true); + } } + + g } else { // FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow? // FIXME(nagisa): investigate whether it can be changed into define_global @@ -1197,6 +1206,9 @@ pub fn trans_static(ccx: &CrateContext, "thread_local") { llvm::set_thread_local(g, true); } + + base::set_link_section(ccx, g, attrs); + Ok(g) } } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index d7c5c41a156..92fddd7d77d 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -14,7 +14,9 @@ //! item-path. This is used for unit testing the code that generates //! paths etc in all kinds of annoying scenarios. -use base::llvm_linkage_by_name; +use base; +use context::CrateContext; +use declare; use glue::DropGlueKind; use llvm; use monomorphize::Instance; @@ -26,6 +28,8 @@ use std::hash::{Hash, Hasher}; use syntax::ast::{self, NodeId}; use syntax::attr; use syntax::parse::token; +use type_of; + #[derive(PartialEq, Eq, Clone, Copy, Debug)] pub enum TransItem<'tcx> { @@ -54,6 +58,153 @@ impl<'tcx> Hash for TransItem<'tcx> { } } + +impl<'tcx> TransItem<'tcx> { + + pub fn predefine<'ccx>(&self, + ccx: &CrateContext<'ccx, 'tcx>, + linkage: llvm::Linkage) { + match *self { + TransItem::Static(node_id) => { + TransItem::predefine_static(ccx, node_id, linkage); + } + _ => { + // Not yet implemented + } + } + } + + fn predefine_static<'a>(ccx: &CrateContext<'a, 'tcx>, + node_id: ast::NodeId, + linkage: llvm::Linkage) { + let def_id = ccx.tcx().map.local_def_id(node_id); + let ty = ccx.tcx().lookup_item_type(def_id).ty; + let llty = type_of::type_of(ccx, ty); + + match ccx.tcx().map.get(node_id) { + hir::map::NodeItem(&hir::Item { + span, node: hir::ItemStatic(..), .. + }) => { + let instance = Instance::mono(ccx.shared(), def_id); + let sym = instance.symbol_name(ccx.shared()); + debug!("making {}", sym); + + let g = declare::define_global(ccx, &sym, llty).unwrap_or_else(|| { + ccx.sess().span_fatal(span, + &format!("symbol `{}` is already defined", sym)) + }); + + llvm::SetLinkage(g, linkage); + } + + item => bug!("predefine_static: expected static, found {:?}", item) + } + } + + pub fn requests_inline<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { + match *self { + TransItem::Fn(ref instance) => { + let attributes = tcx.get_attrs(instance.def); + attr::requests_inline(&attributes[..]) + } + TransItem::DropGlue(..) => true, + TransItem::Static(..) => false, + } + } + + pub fn is_from_extern_crate(&self) -> bool { + match *self { + TransItem::Fn(ref instance) => !instance.def.is_local(), + TransItem::DropGlue(..) | + TransItem::Static(..) => false, + } + } + + pub fn is_lazily_instantiated(&self) -> bool { + match *self { + TransItem::Fn(ref instance) => !instance.substs.types.is_empty(), + TransItem::DropGlue(..) => true, + TransItem::Static(..) => false, + } + } + + pub fn explicit_linkage<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option { + let def_id = match *self { + TransItem::Fn(ref instance) => instance.def, + TransItem::Static(node_id) => tcx.map.local_def_id(node_id), + TransItem::DropGlue(..) => return None, + }; + + let attributes = tcx.get_attrs(def_id); + if let Some(name) = attr::first_attr_value_str_by_name(&attributes, "linkage") { + if let Some(linkage) = base::llvm_linkage_by_name(&name) { + Some(linkage) + } else { + let span = tcx.map.span_if_local(def_id); + if let Some(span) = span { + tcx.sess.span_fatal(span, "invalid linkage specified") + } else { + tcx.sess.fatal(&format!("invalid linkage specified: {}", name)) + } + } + } else { + None + } + } + + pub fn to_string<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { + let hir_map = &tcx.map; + + return match *self { + TransItem::DropGlue(dg) => { + let mut s = String::with_capacity(32); + match dg { + DropGlueKind::Ty(_) => s.push_str("drop-glue "), + DropGlueKind::TyContents(_) => s.push_str("drop-glue-contents "), + }; + push_unique_type_name(tcx, dg.ty(), &mut s); + s + } + TransItem::Fn(instance) => { + to_string_internal(tcx, "fn ", instance) + }, + TransItem::Static(node_id) => { + let def_id = hir_map.local_def_id(node_id); + let instance = Instance::new(def_id, + tcx.mk_substs(subst::Substs::empty())); + to_string_internal(tcx, "static ", instance) + }, + }; + + fn to_string_internal<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx,'tcx>, + prefix: &str, + instance: Instance<'tcx>) + -> String { + let mut result = String::with_capacity(32); + result.push_str(prefix); + push_instance_as_string(tcx, instance, &mut result); + result + } + } + + pub fn to_raw_string(&self) -> String { + match *self { + TransItem::DropGlue(dg) => { + format!("DropGlue({})", dg.ty() as *const _ as usize) + } + TransItem::Fn(instance) => { + format!("Fn({:?}, {})", + instance.def, + instance.substs as *const _ as usize) + } + TransItem::Static(id) => { + format!("Static({:?})", id) + } + } + } +} + + //=----------------------------------------------------------------------------- // TransItem String Keys //=----------------------------------------------------------------------------- @@ -277,108 +428,3 @@ pub fn type_to_string<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, push_unique_type_name(tcx, ty, &mut output); output } - -impl<'tcx> TransItem<'tcx> { - - pub fn requests_inline<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { - match *self { - TransItem::Fn(ref instance) => { - let attributes = tcx.get_attrs(instance.def); - attr::requests_inline(&attributes[..]) - } - TransItem::DropGlue(..) => true, - TransItem::Static(..) => false, - } - } - - pub fn is_from_extern_crate(&self) -> bool { - match *self { - TransItem::Fn(ref instance) => !instance.def.is_local(), - TransItem::DropGlue(..) | - TransItem::Static(..) => false, - } - } - - pub fn is_lazily_instantiated(&self) -> bool { - match *self { - TransItem::Fn(ref instance) => !instance.substs.types.is_empty(), - TransItem::DropGlue(..) => true, - TransItem::Static(..) => false, - } - } - - pub fn explicit_linkage<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option { - let def_id = match *self { - TransItem::Fn(ref instance) => instance.def, - TransItem::Static(node_id) => tcx.map.local_def_id(node_id), - TransItem::DropGlue(..) => return None, - }; - - let attributes = tcx.get_attrs(def_id); - if let Some(name) = attr::first_attr_value_str_by_name(&attributes, "linkage") { - if let Some(linkage) = llvm_linkage_by_name(&name) { - Some(linkage) - } else { - let span = tcx.map.span_if_local(def_id); - if let Some(span) = span { - tcx.sess.span_fatal(span, "invalid linkage specified") - } else { - tcx.sess.fatal(&format!("invalid linkage specified: {}", name)) - } - } - } else { - None - } - } - - pub fn to_string<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { - let hir_map = &tcx.map; - - return match *self { - TransItem::DropGlue(dg) => { - let mut s = String::with_capacity(32); - match dg { - DropGlueKind::Ty(_) => s.push_str("drop-glue "), - DropGlueKind::TyContents(_) => s.push_str("drop-glue-contents "), - }; - push_unique_type_name(tcx, dg.ty(), &mut s); - s - } - TransItem::Fn(instance) => { - to_string_internal(tcx, "fn ", instance) - }, - TransItem::Static(node_id) => { - let def_id = hir_map.local_def_id(node_id); - let empty_substs = tcx.mk_substs(subst::Substs::empty()); - let instance = Instance::new(def_id, empty_substs); - to_string_internal(tcx, "static ", instance) - }, - }; - - fn to_string_internal<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - prefix: &str, - instance: Instance<'tcx>) - -> String { - let mut result = String::with_capacity(32); - result.push_str(prefix); - push_instance_as_string(tcx, instance, &mut result); - result - } - } - - pub fn to_raw_string(&self) -> String { - match *self { - TransItem::DropGlue(dg) => { - format!("DropGlue({})", dg.ty() as *const _ as usize) - } - TransItem::Fn(instance) => { - format!("Fn({:?}, {})", - instance.def, - instance.substs as *const _ as usize) - } - TransItem::Static(id) => { - format!("Static({:?})", id) - } - } - } -} From 67171069472f57875ac8cf4c94e1a56fa635582a Mon Sep 17 00:00:00 2001 From: James Miller Date: Sat, 14 May 2016 05:41:42 +1200 Subject: [PATCH 02/21] Drive function item translation from collector Functions and method are declared ahead-of-time, including generic ones. Closures are not considered trans items anymore, instead they are translated on demands. --- src/librustc_trans/base.rs | 175 +++++++++++++++-------------- src/librustc_trans/closure.rs | 7 +- src/librustc_trans/collector.rs | 41 ++++--- src/librustc_trans/inline.rs | 64 ++--------- src/librustc_trans/monomorphize.rs | 11 +- src/librustc_trans/trans_item.rs | 90 ++++++++++++--- 6 files changed, 212 insertions(+), 176 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 40ae5025180..05ae347b534 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -75,6 +75,7 @@ use debuginfo::{self, DebugLoc, ToDebugLoc}; use declare; use expr; use glue; +use inline; use machine; use machine::{llalign_of_min, llsize_of, llsize_of_real}; use meth; @@ -1948,6 +1949,49 @@ pub fn trans_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, closure::ClosureEnv::NotClosure); } +pub fn trans_instance<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, instance: Instance<'tcx>) { + let instance = inline::maybe_inline_instance(ccx, instance); + + let fn_node_id = ccx.tcx().map.as_local_node_id(instance.def).unwrap(); + + let _s = StatRecorder::new(ccx, ccx.tcx().node_path_str(fn_node_id)); + debug!("trans_instance(instance={:?})", instance); + let _icx = push_ctxt("trans_instance"); + + let item = ccx.tcx().map.find(fn_node_id).unwrap(); + + let fn_ty = ccx.tcx().lookup_item_type(instance.def).ty; + let fn_ty = ccx.tcx().erase_regions(&fn_ty); + let fn_ty = monomorphize::apply_param_substs(ccx.tcx(), instance.substs, &fn_ty); + + let sig = ccx.tcx().erase_late_bound_regions(fn_ty.fn_sig()); + let sig = ccx.tcx().normalize_associated_type(&sig); + let abi = fn_ty.fn_abi(); + + let lldecl = match ccx.instances().borrow().get(&instance) { + Some(&val) => val, + None => bug!("Instance `{:?}` not already declared", instance) + }; + + match item { + hir_map::NodeItem(&hir::Item { + node: hir::ItemFn(ref decl, _, _, _, _, ref body), .. + }) | + hir_map::NodeTraitItem(&hir::TraitItem { + node: hir::MethodTraitItem( + hir::MethodSig { ref decl, .. }, Some(ref body)), .. + }) | + hir_map::NodeImplItem(&hir::ImplItem { + node: hir::ImplItemKind::Method( + hir::MethodSig { ref decl, .. }, ref body), .. + }) => { + trans_closure(ccx, decl, body, lldecl, instance, + fn_node_id, &sig, abi, closure::ClosureEnv::NotClosure); + } + _ => bug!("Instance is a {:?}?", item) + } +} + pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, ctor_ty: Ty<'tcx>, disr: Disr, @@ -2267,70 +2311,23 @@ pub fn trans_item(ccx: &CrateContext, item: &hir::Item) { let _icx = push_ctxt("trans_item"); let tcx = ccx.tcx(); - let from_external = ccx.external_srcs().borrow().contains_key(&item.id); - match item.node { - hir::ItemFn(ref decl, _, _, _, ref generics, ref body) => { - if !generics.is_type_parameterized() { - let trans_everywhere = attr::requests_inline(&item.attrs); - // Ignore `trans_everywhere` for cross-crate inlined items - // (`from_external`). `trans_item` will be called once for each - // compilation unit that references the item, so it will still get - // translated everywhere it's needed. - for (ref ccx, is_origin) in ccx.maybe_iter(!from_external && trans_everywhere) { - let def_id = tcx.map.local_def_id(item.id); - let empty_substs = ccx.empty_substs_for_def_id(def_id); - let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; - trans_fn(ccx, &decl, &body, llfn, empty_substs, item.id); - set_link_section(ccx, llfn, &item.attrs); - update_linkage(ccx, - llfn, - Some(item.id), - if is_origin { - OriginalTranslation - } else { - InlinedCopy - }); - - if is_entry_fn(ccx.sess(), item.id) { - create_entry_wrapper(ccx, item.span, llfn); - // check for the #[rustc_error] annotation, which forces an - // error in trans. This is used to write compile-fail tests - // that actually test that compilation succeeds without - // reporting an error. - if tcx.has_attr(def_id, "rustc_error") { - tcx.sess.span_fatal(item.span, "compilation successful"); - } - } + hir::ItemFn(_, _, _, _, _, _) => { + let def_id = tcx.map.local_def_id(item.id); + // check for the #[rustc_error] annotation, which forces an + // error in trans. This is used to write compile-fail tests + // that actually test that compilation succeeds without + // reporting an error. + if is_entry_fn(ccx.sess(), item.id) { + let empty_substs = ccx.empty_substs_for_def_id(def_id); + let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; + create_entry_wrapper(ccx, item.span, llfn); + if tcx.has_attr(def_id, "rustc_error") { + tcx.sess.span_fatal(item.span, "compilation successful"); } } - } - hir::ItemImpl(_, _, ref generics, _, _, ref impl_items) => { - // Both here and below with generic methods, be sure to recurse and look for - // items that we need to translate. - if !generics.ty_params.is_empty() { - return; - } - for impl_item in impl_items { - if let hir::ImplItemKind::Method(ref sig, ref body) = impl_item.node { - if sig.generics.ty_params.is_empty() { - let trans_everywhere = attr::requests_inline(&impl_item.attrs); - for (ref ccx, is_origin) in ccx.maybe_iter(trans_everywhere) { - let def_id = tcx.map.local_def_id(impl_item.id); - let empty_substs = ccx.empty_substs_for_def_id(def_id); - let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; - trans_fn(ccx, &sig.decl, body, llfn, empty_substs, impl_item.id); - update_linkage(ccx, llfn, Some(impl_item.id), - if is_origin { - OriginalTranslation - } else { - InlinedCopy - }); - } - } - } - } + // Function is actually translated in trans_instance } hir::ItemEnum(ref enum_definition, ref gens) => { if gens.ty_params.is_empty() { @@ -2338,8 +2335,9 @@ pub fn trans_item(ccx: &CrateContext, item: &hir::Item) { enum_variant_size_lint(ccx, enum_definition, item.span, item.id); } } + hir::ItemImpl(..) | hir::ItemStatic(..) => { - // Don't do anything here. Translation of statics has been moved to + // Don't do anything here. Translation has been moved to // being "collector-driven". } _ => {} @@ -2482,16 +2480,16 @@ fn internalize_symbols(cx: &CrateContextList, reachable: &HashSet<&str>) { let linkage = llvm::LLVMGetLinkage(val); // We only care about external declarations (not definitions) // and available_externally definitions. - if !(linkage == llvm::ExternalLinkage as c_uint && - llvm::LLVMIsDeclaration(val) != 0) && - !(linkage == llvm::AvailableExternallyLinkage as c_uint) { - continue; + let is_available_externally = linkage == llvm::AvailableExternallyLinkage as c_uint; + let is_decl = llvm::LLVMIsDeclaration(val) != 0; + + if is_decl || is_available_externally { + let name = CStr::from_ptr(llvm::LLVMGetValueName(val)) + .to_bytes() + .to_vec(); + declared.insert(name); } - let name = CStr::from_ptr(llvm::LLVMGetValueName(val)) - .to_bytes() - .to_vec(); - declared.insert(name); } } @@ -2501,21 +2499,27 @@ fn internalize_symbols(cx: &CrateContextList, reachable: &HashSet<&str>) { for ccx in cx.iter() { for val in iter_globals(ccx.llmod()).chain(iter_functions(ccx.llmod())) { let linkage = llvm::LLVMGetLinkage(val); - // We only care about external definitions. - if !((linkage == llvm::ExternalLinkage as c_uint || - linkage == llvm::WeakODRLinkage as c_uint) && - llvm::LLVMIsDeclaration(val) == 0) { - continue; - } - let name = CStr::from_ptr(llvm::LLVMGetValueName(val)) - .to_bytes() - .to_vec(); - if !declared.contains(&name) && - !reachable.contains(str::from_utf8(&name).unwrap()) { - llvm::SetLinkage(val, llvm::InternalLinkage); - llvm::SetDLLStorageClass(val, llvm::DefaultStorageClass); - llvm::UnsetComdat(val); + let is_external = linkage == llvm::ExternalLinkage as c_uint; + let is_weak_odr = linkage == llvm::WeakODRLinkage as c_uint; + let is_decl = llvm::LLVMIsDeclaration(val) != 0; + + // We only care about external definitions. + if (is_external || is_weak_odr) && !is_decl { + + let name = CStr::from_ptr(llvm::LLVMGetValueName(val)) + .to_bytes() + .to_vec(); + + let is_declared = declared.contains(&name); + let reachable = reachable.contains(str::from_utf8(&name).unwrap()); + + if !is_declared && !reachable { + llvm::SetLinkage(val, llvm::InternalLinkage); + llvm::SetDLLStorageClass(val, llvm::DefaultStorageClass); + llvm::UnsetComdat(val); + } + } } } @@ -2744,6 +2748,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, span_bug!(item.span, "Mismatch between hir::Item type and TransItem type") } } + TransItem::Fn(instance) => { + trans_instance(&ccx, instance); + } _ => { } } } @@ -2980,7 +2987,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a let mut item_keys: Vec<_> = items .iter() .map(|i| { - let mut output = i.to_string(scx.tcx()); + let mut output = i.to_string(scx); output.push_str(" @@"); let mut empty = Vec::new(); let mut cgus = item_to_cgus.get_mut(i).unwrap_or(&mut empty); diff --git a/src/librustc_trans/closure.rs b/src/librustc_trans/closure.rs index 9196cfce16f..bf9cde9a441 100644 --- a/src/librustc_trans/closure.rs +++ b/src/librustc_trans/closure.rs @@ -10,7 +10,7 @@ use arena::TypedArena; use back::symbol_names; -use llvm::{ValueRef, get_param, get_params}; +use llvm::{self, ValueRef, get_param, get_params}; use rustc::hir::def_id::DefId; use abi::{Abi, FnType}; use adt; @@ -167,7 +167,7 @@ fn get_or_create_closure_declaration<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, variadic: false }) })); - let llfn = declare::define_internal_fn(ccx, &symbol, function_type); + let llfn = declare::declare_fn(ccx, &symbol, function_type); // set an inline hint for all closures attributes::inline(llfn, attributes::InlineAttr::Hint); @@ -211,6 +211,7 @@ pub fn trans_closure_expr<'a, 'tcx>(dest: Dest<'a, 'tcx>, id, closure_def_id, closure_substs); let llfn = get_or_create_closure_declaration(ccx, closure_def_id, closure_substs); + llvm::SetLinkage(llfn, llvm::WeakODRLinkage); // Get the type of this closure. Use the current `param_substs` as // the closure substitutions. This makes sense because the closure @@ -377,7 +378,7 @@ fn trans_fn_once_adapter_shim<'a, 'tcx>( // Create the by-value helper. let function_name = symbol_names::internal_name_from_type_and_suffix(ccx, llonce_fn_ty, "once_shim"); - let lloncefn = declare::define_internal_fn(ccx, &function_name, llonce_fn_ty); + let lloncefn = declare::declare_fn(ccx, &function_name, llonce_fn_ty); attributes::set_frame_pointer_elimination(ccx, lloncefn); let (block_arena, fcx): (TypedArena<_>, FunctionContext); diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index eea6aec3726..8cb9fd23e49 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -325,7 +325,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, // We've been here already, no need to search again. return; } - debug!("BEGIN collect_items_rec({})", starting_point.to_string(scx.tcx())); + debug!("BEGIN collect_items_rec({})", starting_point.to_string(scx)); let mut neighbors = Vec::new(); let recursion_depth_reset; @@ -396,7 +396,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, recursion_depths.insert(def_id, depth); } - debug!("END collect_items_rec({})", starting_point.to_string(scx.tcx())); + debug!("END collect_items_rec({})", starting_point.to_string(scx)); } fn record_inlining_canditates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -456,12 +456,25 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { match *rvalue { mir::Rvalue::Aggregate(mir::AggregateKind::Closure(def_id, ref substs), _) => { - assert!(can_have_local_instance(self.scx.tcx(), def_id)); - let trans_item = create_fn_trans_item(self.scx.tcx(), - def_id, - substs.func_substs, - self.param_substs); - self.output.push(trans_item); + let mir = errors::expect(self.scx.sess().diagnostic(), self.scx.get_mir(def_id), + || format!("Could not find MIR for closure: {:?}", def_id)); + + let concrete_substs = monomorphize::apply_param_substs(self.scx.tcx(), + self.param_substs, + &substs.func_substs); + let concrete_substs = self.scx.tcx().erase_regions(&concrete_substs); + + let mut visitor = MirNeighborCollector { + scx: self.scx, + mir: &mir, + output: self.output, + param_substs: concrete_substs + }; + + visitor.visit_mir(&mir); + for promoted in &mir.promoted { + visitor.visit_mir(promoted); + } } // When doing an cast from a regular pointer to a fat pointer, we // have to instantiate all methods of the trait being cast to, so we @@ -1107,9 +1120,8 @@ impl<'b, 'a, 'v> hir_visit::Visitor<'v> for RootCollector<'b, 'a, 'v> { self.scx.tcx().map.local_def_id(item.id))); self.output.push(TransItem::Static(item.id)); } - hir::ItemFn(_, _, constness, _, ref generics, _) => { - if !generics.is_type_parameterized() && - constness == hir::Constness::NotConst { + hir::ItemFn(_, _, _, _, ref generics, _) => { + if !generics.is_type_parameterized() { let def_id = self.scx.tcx().map.local_def_id(item.id); debug!("RootCollector: ItemFn({})", @@ -1129,9 +1141,8 @@ impl<'b, 'a, 'v> hir_visit::Visitor<'v> for RootCollector<'b, 'a, 'v> { match ii.node { hir::ImplItemKind::Method(hir::MethodSig { ref generics, - constness, .. - }, _) if constness == hir::Constness::NotConst => { + }, _) => { let hir_map = &self.scx.tcx().map; let parent_node_id = hir_map.get_parent_node(ii.id); let is_impl_generic = match hir_map.expect_item(parent_node_id) { @@ -1260,7 +1271,7 @@ pub fn print_collection_results<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) { let mut item_keys = FnvHashMap(); for (item, item_state) in trans_items.iter() { - let k = item.to_string(scx.tcx()); + let k = item.to_string(scx); if item_keys.contains_key(&k) { let prev: (TransItem, TransItemState) = item_keys[&k]; @@ -1288,7 +1299,7 @@ pub fn print_collection_results<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) { let mut generated = FnvHashSet(); for (item, item_state) in trans_items.iter() { - let item_key = item.to_string(scx.tcx()); + let item_key = item.to_string(scx); match *item_state { TransItemState::PredictedAndGenerated => { diff --git a/src/librustc_trans/inline.rs b/src/librustc_trans/inline.rs index af175fbf882..4077b894d62 100644 --- a/src/librustc_trans/inline.rs +++ b/src/librustc_trans/inline.rs @@ -8,13 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use llvm::{AvailableExternallyLinkage, InternalLinkage, SetLinkage}; use middle::cstore::{FoundAst, InlinedItem}; use rustc::hir::def_id::DefId; -use rustc::ty::subst::Substs; -use base::{push_ctxt, trans_item, trans_fn}; -use callee::Callee; +use base::push_ctxt; use common::*; +use monomorphize::Instance; use rustc::dep_graph::DepNode; use rustc::hir; @@ -52,30 +50,6 @@ fn instantiate_inline(ccx: &CrateContext, fn_id: DefId) -> Option { ccx.external_srcs().borrow_mut().insert(item.id, fn_id); ccx.stats().n_inlines.set(ccx.stats().n_inlines.get() + 1); - trans_item(ccx, item); - - if let hir::ItemFn(_, _, _, _, ref generics, _) = item.node { - // Generics have no symbol, so they can't be given any linkage. - if !generics.is_type_parameterized() { - let linkage = if ccx.sess().opts.cg.codegen_units == 1 { - // We could use AvailableExternallyLinkage here, - // but InternalLinkage allows LLVM to optimize more - // aggressively (at the cost of sometimes - // duplicating code). - InternalLinkage - } else { - // With multiple compilation units, duplicated code - // is more of a problem. Also, `codegen_units > 1` - // means the user is okay with losing some - // performance. - AvailableExternallyLinkage - }; - let empty_substs = tcx.mk_substs(Substs::empty()); - let def_id = tcx.map.local_def_id(item.id); - let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; - SetLinkage(llfn, linkage); - } - } item.id } @@ -135,35 +109,12 @@ fn instantiate_inline(ccx: &CrateContext, fn_id: DefId) -> Option { // don't. trait_item.id } - FoundAst::Found(&InlinedItem::ImplItem(impl_did, ref impl_item)) => { + FoundAst::Found(&InlinedItem::ImplItem(_, ref impl_item)) => { ccx.external().borrow_mut().insert(fn_id, Some(impl_item.id)); ccx.external_srcs().borrow_mut().insert(impl_item.id, fn_id); ccx.stats().n_inlines.set(ccx.stats().n_inlines.get() + 1); - // Translate monomorphic impl methods immediately. - if let hir::ImplItemKind::Method(ref sig, ref body) = impl_item.node { - let impl_tpt = tcx.lookup_item_type(impl_did); - if impl_tpt.generics.types.is_empty() && - sig.generics.ty_params.is_empty() { - let def_id = tcx.map.local_def_id(impl_item.id); - let empty_substs = ccx.empty_substs_for_def_id(def_id); - let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; - trans_fn(ccx, - &sig.decl, - body, - llfn, - empty_substs, - impl_item.id); - // See linkage comments on items. - if ccx.sess().opts.cg.codegen_units == 1 { - SetLinkage(llfn, InternalLinkage); - } else { - SetLinkage(llfn, AvailableExternallyLinkage); - } - } - } - impl_item.id } }; @@ -184,3 +135,12 @@ pub fn get_local_instance(ccx: &CrateContext, fn_id: DefId) pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: DefId) -> DefId { get_local_instance(ccx, fn_id).unwrap_or(fn_id) } + +pub fn maybe_inline_instance<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, + instance: Instance<'tcx>) -> Instance<'tcx> { + let def_id = maybe_instantiate_inline(ccx, instance.def); + Instance { + def: def_id, + substs: instance.substs + } +} diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index ab859b88a85..13d6d432422 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -109,15 +109,16 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, }); match map_node { hir_map::NodeItem(&hir::Item { - ref attrs, node: hir::ItemFn(ref decl, _, _, _, _, ref body), .. - }) | - hir_map::NodeTraitItem(&hir::TraitItem { - ref attrs, node: hir::MethodTraitItem( - hir::MethodSig { ref decl, .. }, Some(ref body)), .. + ref attrs, + node: hir::ItemFn(ref decl, _, _, _, _, ref body), .. }) | hir_map::NodeImplItem(&hir::ImplItem { ref attrs, node: hir::ImplItemKind::Method( hir::MethodSig { ref decl, .. }, ref body), .. + }) | + hir_map::NodeTraitItem(&hir::TraitItem { + ref attrs, node: hir::MethodTraitItem( + hir::MethodSig { ref decl, .. }, Some(ref body)), .. }) => { attributes::from_fn_attrs(ccx, attrs, lldecl); diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 92fddd7d77d..f3c0ddfaf73 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -14,19 +14,22 @@ //! item-path. This is used for unit testing the code that generates //! paths etc in all kinds of annoying scenarios. +use attributes; use base; -use context::CrateContext; +use context::{SharedCrateContext, CrateContext}; use declare; use glue::DropGlueKind; use llvm; -use monomorphize::Instance; +use monomorphize::{self, Instance}; +use inline; use rustc::hir; +use rustc::hir::map as hir_map; use rustc::hir::def_id::DefId; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc::ty::subst; use std::hash::{Hash, Hasher}; use syntax::ast::{self, NodeId}; -use syntax::attr; +use syntax::{attr,errors}; use syntax::parse::token; use type_of; @@ -59,24 +62,27 @@ impl<'tcx> Hash for TransItem<'tcx> { } -impl<'tcx> TransItem<'tcx> { +impl<'a, 'tcx> TransItem<'tcx> { - pub fn predefine<'ccx>(&self, - ccx: &CrateContext<'ccx, 'tcx>, - linkage: llvm::Linkage) { + pub fn predefine(&self, + ccx: &CrateContext<'a, 'tcx>, + linkage: llvm::Linkage) { match *self { TransItem::Static(node_id) => { TransItem::predefine_static(ccx, node_id, linkage); } + TransItem::Fn(instance) => { + TransItem::predefine_fn(ccx, instance, linkage); + } _ => { // Not yet implemented } } } - fn predefine_static<'a>(ccx: &CrateContext<'a, 'tcx>, - node_id: ast::NodeId, - linkage: llvm::Linkage) { + fn predefine_static(ccx: &CrateContext<'a, 'tcx>, + node_id: ast::NodeId, + linkage: llvm::Linkage) { let def_id = ccx.tcx().map.local_def_id(node_id); let ty = ccx.tcx().lookup_item_type(def_id).ty; let llty = type_of::type_of(ccx, ty); @@ -101,7 +107,57 @@ impl<'tcx> TransItem<'tcx> { } } - pub fn requests_inline<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { + fn predefine_fn(ccx: &CrateContext<'a, 'tcx>, + instance: Instance<'tcx>, + linkage: llvm::Linkage) { + let unit = ccx.codegen_unit(); + debug!("predefine_fn[cg={}](instance={:?})", &unit.name[..], instance); + assert!(!instance.substs.types.needs_infer() && + !instance.substs.types.has_param_types()); + + let instance = inline::maybe_inline_instance(ccx, instance); + + let item_ty = ccx.tcx().lookup_item_type(instance.def).ty; + let item_ty = ccx.tcx().erase_regions(&item_ty); + let mono_ty = monomorphize::apply_param_substs(ccx.tcx(), instance.substs, &item_ty); + + let fn_node_id = ccx.tcx().map.as_local_node_id(instance.def).unwrap(); + let map_node = errors::expect( + ccx.sess().diagnostic(), + ccx.tcx().map.find(fn_node_id), + || { + format!("while instantiating `{}`, couldn't find it in \ + the item map (may have attempted to monomorphize \ + an item defined in a different crate?)", + instance) + }); + + match map_node { + hir_map::NodeItem(&hir::Item { + ref attrs, node: hir::ItemFn(..), .. + }) | + hir_map::NodeTraitItem(&hir::TraitItem { + ref attrs, node: hir::MethodTraitItem(..), .. + }) | + hir_map::NodeImplItem(&hir::ImplItem { + ref attrs, node: hir::ImplItemKind::Method(..), .. + }) => { + let symbol = instance.symbol_name(ccx.shared()); + let lldecl = declare::declare_fn(ccx, &symbol, mono_ty); + + attributes::from_fn_attrs(ccx, attrs, lldecl); + llvm::SetLinkage(lldecl, linkage); + base::set_link_section(ccx, lldecl, attrs); + + ccx.instances().borrow_mut().insert(instance, lldecl); + } + _ => bug!("Invalid item for TransItem::Fn: `{:?}`", map_node) + }; + + } + + + pub fn requests_inline(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { match *self { TransItem::Fn(ref instance) => { let attributes = tcx.get_attrs(instance.def); @@ -128,7 +184,7 @@ impl<'tcx> TransItem<'tcx> { } } - pub fn explicit_linkage<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option { + pub fn explicit_linkage(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option { let def_id = match *self { TransItem::Fn(ref instance) => instance.def, TransItem::Static(node_id) => tcx.map.local_def_id(node_id), @@ -152,7 +208,8 @@ impl<'tcx> TransItem<'tcx> { } } - pub fn to_string<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { + pub fn to_string(&self, scx: &SharedCrateContext<'a, 'tcx>) -> String { + let tcx = scx.tcx(); let hir_map = &tcx.map; return match *self { @@ -170,13 +227,12 @@ impl<'tcx> TransItem<'tcx> { }, TransItem::Static(node_id) => { let def_id = hir_map.local_def_id(node_id); - let instance = Instance::new(def_id, - tcx.mk_substs(subst::Substs::empty())); + let instance = Instance::mono(scx, def_id); to_string_internal(tcx, "static ", instance) }, }; - fn to_string_internal<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx,'tcx>, + fn to_string_internal<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, prefix: &str, instance: Instance<'tcx>) -> String { From 5f3fefc77dcaeb0f2ca409a7087d0e04ae2e3554 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 13 May 2016 18:14:47 -0400 Subject: [PATCH 03/21] trans: Get rid of the last potential on-demand creation of non-closure functions. --- src/librustc_trans/base.rs | 69 +----------------------------- src/librustc_trans/glue.rs | 2 +- src/librustc_trans/monomorphize.rs | 31 ++++---------- 3 files changed, 11 insertions(+), 91 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 05ae347b534..fc45d73a7ae 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -25,8 +25,6 @@ #![allow(non_camel_case_types)] -pub use self::ValueOrigin::*; - use super::CrateTranslation; use super::ModuleTranslation; @@ -1918,37 +1916,6 @@ pub fn trans_closure<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fcx.finish(bcx, fn_cleanup_debug_loc.debug_loc()); } -/// Creates an LLVM function corresponding to a source language function. -pub fn trans_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, - decl: &hir::FnDecl, - body: &hir::Block, - llfndecl: ValueRef, - param_substs: &'tcx Substs<'tcx>, - id: ast::NodeId) { - let _s = StatRecorder::new(ccx, ccx.tcx().node_path_str(id)); - debug!("trans_fn(param_substs={:?})", param_substs); - let _icx = push_ctxt("trans_fn"); - let def_id = if let Some(&def_id) = ccx.external_srcs().borrow().get(&id) { - def_id - } else { - ccx.tcx().map.local_def_id(id) - }; - let fn_ty = ccx.tcx().lookup_item_type(def_id).ty; - let fn_ty = monomorphize::apply_param_substs(ccx.tcx(), param_substs, &fn_ty); - let sig = ccx.tcx().erase_late_bound_regions(fn_ty.fn_sig()); - let sig = ccx.tcx().normalize_associated_type(&sig); - let abi = fn_ty.fn_abi(); - trans_closure(ccx, - decl, - body, - llfndecl, - Instance::new(def_id, param_substs), - id, - &sig, - abi, - closure::ClosureEnv::NotClosure); -} - pub fn trans_instance<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, instance: Instance<'tcx>) { let instance = inline::maybe_inline_instance(ccx, instance); @@ -2215,46 +2182,14 @@ pub fn llvm_linkage_by_name(name: &str) -> Option { } } - -/// Enum describing the origin of an LLVM `Value`, for linkage purposes. -#[derive(Copy, Clone)] -pub enum ValueOrigin { - /// The LLVM `Value` is in this context because the corresponding item was - /// assigned to the current compilation unit. - OriginalTranslation, - /// The `Value`'s corresponding item was assigned to some other compilation - /// unit, but the `Value` was translated in this context anyway because the - /// item is marked `#[inline]`. - InlinedCopy, -} - /// Set the appropriate linkage for an LLVM `ValueRef` (function or global). /// If the `llval` is the direct translation of a specific Rust item, `id` /// should be set to the `NodeId` of that item. (This mapping should be /// 1-to-1, so monomorphizations and drop/visit glue should have `id` set to -/// `None`.) `llval_origin` indicates whether `llval` is the translation of an -/// item assigned to `ccx`'s compilation unit or an inlined copy of an item -/// assigned to a different compilation unit. +/// `None`.) pub fn update_linkage(ccx: &CrateContext, llval: ValueRef, - id: Option, - llval_origin: ValueOrigin) { - match llval_origin { - InlinedCopy => { - // `llval` is a translation of an item defined in a separate - // compilation unit. This only makes sense if there are at least - // two compilation units. - assert!(ccx.sess().opts.cg.codegen_units > 1 || - ccx.sess().opts.debugging_opts.incremental.is_some()); - // `llval` is a copy of something defined elsewhere, so use - // `AvailableExternallyLinkage` to avoid duplicating code in the - // output. - llvm::SetLinkage(llval, llvm::AvailableExternallyLinkage); - return; - }, - OriginalTranslation => {}, - } - + id: Option) { if let Some(id) = id { let item = ccx.tcx().map.get(id); if let hir_map::NodeItem(i) = item { diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index ac23d713d27..e4519ff82a0 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -285,7 +285,7 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let bcx = fcx.init(false, None); - update_linkage(ccx, llfn, None, OriginalTranslation); + update_linkage(ccx, llfn, None); ccx.stats().n_glues_created.set(ccx.stats().n_glues_created.get() + 1); // All glue functions take values passed *by alias*; this is a diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index 13d6d432422..832acfe14be 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -17,7 +17,6 @@ use rustc::ty::subst::{Subst, Substs}; use rustc::ty::{self, Ty, TypeFoldable, TyCtxt}; use attributes; use base::{push_ctxt}; -use base::trans_fn; use base; use common::*; use declare; @@ -27,17 +26,16 @@ use rustc::util::ppaux; use rustc::hir; -use syntax::attr; use errors; use std::fmt; +use trans_item::TransItem; pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, fn_id: DefId, psubsts: &'tcx subst::Substs<'tcx>) -> (ValueRef, Ty<'tcx>) { debug!("monomorphic_fn(fn_id={:?}, real_substs={:?})", fn_id, psubsts); - assert!(!psubsts.types.needs_infer() && !psubsts.types.has_param_types()); let _icx = push_ctxt("monomorphic_fn"); @@ -53,6 +51,8 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if let Some(&val) = ccx.instances().borrow().get(&instance) { debug!("leaving monomorphic fn {:?}", instance); return (val, mono_ty); + } else { + assert!(!ccx.codegen_unit().items.contains_key(&TransItem::Fn(instance))); } debug!("monomorphic_fn({:?})", instance); @@ -96,6 +96,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ccx.instances().borrow_mut().insert(instance, lldecl); + // we can only monomorphize things in this crate (or inlined into it) let fn_node_id = ccx.tcx().map.as_local_node_id(fn_id).unwrap(); let map_node = errors::expect( @@ -110,34 +111,18 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, match map_node { hir_map::NodeItem(&hir::Item { ref attrs, - node: hir::ItemFn(ref decl, _, _, _, _, ref body), .. + node: hir::ItemFn(..), .. }) | hir_map::NodeImplItem(&hir::ImplItem { ref attrs, node: hir::ImplItemKind::Method( - hir::MethodSig { ref decl, .. }, ref body), .. + hir::MethodSig { .. }, _), .. }) | hir_map::NodeTraitItem(&hir::TraitItem { ref attrs, node: hir::MethodTraitItem( - hir::MethodSig { ref decl, .. }, Some(ref body)), .. + hir::MethodSig { .. }, Some(_)), .. }) => { attributes::from_fn_attrs(ccx, attrs, lldecl); - - let is_first = !ccx.available_monomorphizations().borrow() - .contains(&symbol); - if is_first { - ccx.available_monomorphizations().borrow_mut().insert(symbol.clone()); - } - - let trans_everywhere = attr::requests_inline(attrs); - if trans_everywhere || is_first { - let origin = if is_first { base::OriginalTranslation } else { base::InlinedCopy }; - base::update_linkage(ccx, lldecl, None, origin); - trans_fn(ccx, decl, body, lldecl, psubsts, fn_node_id); - } else { - // We marked the value as using internal linkage earlier, but that is illegal for - // declarations, so switch back to external linkage. - llvm::SetLinkage(lldecl, llvm::ExternalLinkage); - } + llvm::SetLinkage(lldecl, llvm::ExternalLinkage); } hir_map::NodeVariant(_) | hir_map::NodeStructCtor(_) => { From 2cd8cf92fcfa349750b520fb26293ae585656eea Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 13 May 2016 20:47:32 -0400 Subject: [PATCH 04/21] Ignore closure-related translation item collection tests. --- .../codegen-units/item-collection/cross-crate-closures.rs | 5 +++++ .../codegen-units/item-collection/non-generic-closures.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/test/codegen-units/item-collection/cross-crate-closures.rs b/src/test/codegen-units/item-collection/cross-crate-closures.rs index 546bb235a5f..2b5ac7e8d80 100644 --- a/src/test/codegen-units/item-collection/cross-crate-closures.rs +++ b/src/test/codegen-units/item-collection/cross-crate-closures.rs @@ -8,6 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// In the current version of the collector that still has to support +// legacy-trans, closures do not generate their own TransItems, so we are +// ignoring this test until MIR trans has taken over completely +// ignore-test + // ignore-tidy-linelength // compile-flags:-Zprint-trans-items=eager diff --git a/src/test/codegen-units/item-collection/non-generic-closures.rs b/src/test/codegen-units/item-collection/non-generic-closures.rs index ba77266d072..278e9189dd6 100644 --- a/src/test/codegen-units/item-collection/non-generic-closures.rs +++ b/src/test/codegen-units/item-collection/non-generic-closures.rs @@ -8,6 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// In the current version of the collector that still has to support +// legacy-trans, closures do not generate their own TransItems, so we are +// ignoring this test until MIR trans has taken over completely +// ignore-test + // ignore-tidy-linelength // compile-flags:-Zprint-trans-items=eager From 65e8a13441cb4169ce4fb43be7f1369d5c9d71e0 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 13 May 2016 20:48:32 -0400 Subject: [PATCH 05/21] Adapt backend to trans::partitioning dictating the codegen-unit setup. --- src/librustc/session/config.rs | 65 +++++++++-- src/librustc_driver/driver.rs | 2 +- src/librustc_trans/back/link.rs | 15 +-- src/librustc_trans/back/lto.rs | 7 +- src/librustc_trans/back/write.rs | 170 +++++++++++++++++------------ src/librustc_trans/base.rs | 8 +- src/librustc_trans/lib.rs | 3 +- src/librustc_trans/partitioning.rs | 6 +- 8 files changed, 181 insertions(+), 95 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index a3799006192..5ccc96210be 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -197,23 +197,70 @@ pub struct OutputFilenames { pub outputs: HashMap>, } +/// Codegen unit names generated by the numbered naming scheme will contain this +/// marker right before the index of the codegen unit. +pub const NUMBERED_CODEGEN_UNIT_MARKER: &'static str = ".cgu-"; + impl OutputFilenames { pub fn path(&self, flavor: OutputType) -> PathBuf { self.outputs.get(&flavor).and_then(|p| p.to_owned()) .or_else(|| self.single_output_file.clone()) - .unwrap_or_else(|| self.temp_path(flavor)) + .unwrap_or_else(|| self.temp_path(flavor, None)) } - pub fn temp_path(&self, flavor: OutputType) -> PathBuf { + /// Get the path where a compilation artifact of the given type for the + /// given codegen unit should be placed on disk. If codegen_unit_name is + /// None, a path distinct from those of any codegen unit will be generated. + pub fn temp_path(&self, + flavor: OutputType, + codegen_unit_name: Option<&str>) + -> PathBuf { + let extension = match flavor { + OutputType::Bitcode => "bc", + OutputType::Assembly => "s", + OutputType::LlvmAssembly => "ll", + OutputType::Object => "o", + OutputType::DepInfo => "d", + OutputType::Exe => "", + }; + + self.temp_path_ext(extension, codegen_unit_name) + } + + /// Like temp_path, but also supports things where there is no corresponding + /// OutputType, like no-opt-bitcode or lto-bitcode. + pub fn temp_path_ext(&self, + ext: &str, + codegen_unit_name: Option<&str>) + -> PathBuf { let base = self.out_directory.join(&self.filestem()); - match flavor { - OutputType::Bitcode => base.with_extension("bc"), - OutputType::Assembly => base.with_extension("s"), - OutputType::LlvmAssembly => base.with_extension("ll"), - OutputType::Object => base.with_extension("o"), - OutputType::DepInfo => base.with_extension("d"), - OutputType::Exe => base, + + let mut extension = String::new(); + + if let Some(codegen_unit_name) = codegen_unit_name { + if codegen_unit_name.contains(NUMBERED_CODEGEN_UNIT_MARKER) { + // If we use the numbered naming scheme for modules, we don't want + // the files to look like ... + // but simply .. + let marker_offset = codegen_unit_name.rfind(NUMBERED_CODEGEN_UNIT_MARKER) + .unwrap(); + let index_offset = marker_offset + NUMBERED_CODEGEN_UNIT_MARKER.len(); + extension.push_str(&codegen_unit_name[index_offset .. ]); + } else { + extension.push_str(codegen_unit_name); + }; } + + if !ext.is_empty() { + if !extension.is_empty() { + extension.push_str("."); + } + + extension.push_str(ext); + } + + let path = base.with_extension(&extension[..]); + path } pub fn with_extension(&self, extension: &str) -> PathBuf { diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 277789f5312..eef2b6e6f37 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1081,7 +1081,7 @@ pub fn phase_5_run_llvm_passes(sess: &Session, // Remove assembly source, unless --save-temps was specified if !sess.opts.cg.save_temps { - fs::remove_file(&outputs.temp_path(OutputType::Assembly)).unwrap(); + fs::remove_file(&outputs.temp_path(OutputType::Assembly, None)).unwrap(); } } else { time(sess.time_passes(), diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 744712b22b0..a9f3d2f8a17 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -205,7 +205,7 @@ pub fn link_binary(sess: &Session, // Remove the temporary object file and metadata if we aren't saving temps if !sess.opts.cg.save_temps { - for obj in object_filenames(sess, outputs) { + for obj in object_filenames(trans, outputs) { remove(sess, &obj); } remove(sess, &outputs.with_extension("metadata.o")); @@ -316,7 +316,7 @@ fn link_binary_output(sess: &Session, crate_type: config::CrateType, outputs: &OutputFilenames, crate_name: &str) -> PathBuf { - let objects = object_filenames(sess, outputs); + let objects = object_filenames(trans, outputs); let default_filename = filename_for_input(sess, crate_type, crate_name, outputs); let out_filename = outputs.outputs.get(&OutputType::Exe) @@ -356,10 +356,11 @@ fn link_binary_output(sess: &Session, out_filename } -fn object_filenames(sess: &Session, outputs: &OutputFilenames) -> Vec { - (0..sess.opts.cg.codegen_units).map(|i| { - let ext = format!("{}.o", i); - outputs.temp_path(OutputType::Object).with_extension(&ext) +fn object_filenames(trans: &CrateTranslation, + outputs: &OutputFilenames) + -> Vec { + trans.modules.iter().map(|module| { + outputs.temp_path(OutputType::Object, Some(&module.name[..])) }).collect() } @@ -497,7 +498,7 @@ fn link_rlib<'a>(sess: &'a Session, ab.add_file(&bc_deflated_filename); // See the bottom of back::write::run_passes for an explanation - // of when we do and don't keep .0.bc files around. + // of when we do and don't keep .#module-name#.bc files around. let user_wants_numbered_bitcode = sess.opts.output_types.contains_key(&OutputType::Bitcode) && sess.opts.cg.codegen_units > 1; diff --git a/src/librustc_trans/back/lto.rs b/src/librustc_trans/back/lto.rs index 31bc11fb215..69e4a50804f 100644 --- a/src/librustc_trans/back/lto.rs +++ b/src/librustc_trans/back/lto.rs @@ -22,12 +22,12 @@ use libc; use flate; use std::ffi::CString; +use std::path::Path; pub fn run(sess: &session::Session, llmod: ModuleRef, tm: TargetMachineRef, reachable: &[String], config: &ModuleConfig, - name_extra: &str, - output_names: &config::OutputFilenames) { + temp_no_opt_bc_filename: &Path) { if sess.opts.cg.prefer_dynamic { sess.struct_err("cannot prefer dynamic linking when performing LTO") .note("only 'staticlib', 'bin', and 'cdylib' outputs are \ @@ -132,8 +132,7 @@ pub fn run(sess: &session::Session, llmod: ModuleRef, } if sess.opts.cg.save_temps { - let path = output_names.with_extension(&format!("{}.no-opt.lto.bc", name_extra)); - let cstr = path2cstr(&path); + let cstr = path2cstr(temp_no_opt_bc_filename); unsafe { llvm::LLVMWriteBitcodeToFile(llmod, cstr.as_ptr()); } diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs index ec20381d189..28804e4ed9e 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -423,9 +423,9 @@ unsafe extern "C" fn diagnostic_handler(info: DiagnosticInfoRef, user: *mut c_vo unsafe fn optimize_and_codegen(cgcx: &CodegenContext, mtrans: ModuleTranslation, config: ModuleConfig, - name_extra: String, output_names: OutputFilenames) { - let ModuleTranslation { llmod, llcx } = mtrans; + let llmod = mtrans.llmod; + let llcx = mtrans.llcx; let tm = config.tm; // llcx doesn't outlive this function, so we can put this on the stack. @@ -438,9 +438,10 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, llvm::LLVMSetInlineAsmDiagnosticHandler(llcx, inline_asm_handler, fv); llvm::LLVMContextSetDiagnosticHandler(llcx, diagnostic_handler, fv); + let module_name = Some(&mtrans.name[..]); + if config.emit_no_opt_bc { - let ext = format!("{}.no-opt.bc", name_extra); - let out = output_names.with_extension(&ext); + let out = output_names.temp_path_ext("no-opt.bc", module_name); let out = path2cstr(&out); llvm::LLVMWriteBitcodeToFile(llmod, out.as_ptr()); } @@ -512,13 +513,18 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, match cgcx.lto_ctxt { Some((sess, reachable)) if sess.lto() => { - time(sess.time_passes(), "all lto passes", || - lto::run(sess, llmod, tm, reachable, &config, - &name_extra, &output_names)); - + time(sess.time_passes(), "all lto passes", || { + let temp_no_opt_bc_filename = + output_names.temp_path_ext("no-opt.lto.bc", module_name); + lto::run(sess, + llmod, + tm, + reachable, + &config, + &temp_no_opt_bc_filename); + }); if config.emit_lto_bc { - let name = format!("{}.lto.bc", name_extra); - let out = output_names.with_extension(&name); + let out = output_names.temp_path_ext("lto.bc", module_name); let out = path2cstr(&out); llvm::LLVMWriteBitcodeToFile(llmod, out.as_ptr()); } @@ -556,8 +562,8 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, let write_obj = config.emit_obj && !config.obj_is_bitcode; let copy_bc_to_obj = config.emit_obj && config.obj_is_bitcode; - let bc_out = output_names.with_extension(&format!("{}.bc", name_extra)); - let obj_out = output_names.with_extension(&format!("{}.o", name_extra)); + let bc_out = output_names.temp_path(OutputType::Bitcode, module_name); + let obj_out = output_names.temp_path(OutputType::Object, module_name); if write_bc { let bc_out_c = path2cstr(&bc_out); @@ -566,8 +572,7 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, time(config.time_passes, &format!("codegen passes [{}]", cgcx.worker), || { if config.emit_ir { - let ext = format!("{}.ll", name_extra); - let out = output_names.with_extension(&ext); + let out = output_names.temp_path(OutputType::LlvmAssembly, module_name); let out = path2cstr(&out); with_codegen(tm, llmod, config.no_builtins, |cpm| { llvm::LLVMRustPrintModule(cpm, llmod, out.as_ptr()); @@ -576,7 +581,7 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, } if config.emit_asm { - let path = output_names.with_extension(&format!("{}.s", name_extra)); + let path = output_names.temp_path(OutputType::Assembly, module_name); // We can't use the same module for asm and binary output, because that triggers // various errors like invalid IR or broken binaries, so we might have to clone the @@ -713,27 +718,29 @@ pub fn run_passes(sess: &Session, { let work = build_work_item(sess, - trans.metadata_module, + trans.metadata_module.clone(), metadata_config.clone(), - crate_output.clone(), - "metadata".to_string()); + crate_output.clone()); work_items.push(work); } - for (index, mtrans) in trans.modules.iter().enumerate() { + for mtrans in trans.modules.iter() { let work = build_work_item(sess, - *mtrans, + mtrans.clone(), modules_config.clone(), - crate_output.clone(), - format!("{}", index)); + crate_output.clone()); work_items.push(work); } // Process the work items, optionally using worker threads. - if sess.opts.cg.codegen_units == 1 { + // NOTE: This code is not really adapted to incremental compilation where + // the compiler decides the number of codegen units (and will + // potentially create hundreds of them). + let num_workers = work_items.len() - 1; + if num_workers == 1 { run_work_singlethreaded(sess, &trans.reachable, work_items); } else { - run_work_multithreaded(sess, work_items, sess.opts.cg.codegen_units); + run_work_multithreaded(sess, work_items, num_workers); } // All codegen is finished. @@ -748,32 +755,42 @@ pub fn run_passes(sess: &Session, } }; - let copy_if_one_unit = |ext: &str, - output_type: OutputType, + let copy_if_one_unit = |output_type: OutputType, keep_numbered: bool| { - if sess.opts.cg.codegen_units == 1 { + if trans.modules.len() == 1 { // 1) Only one codegen unit. In this case it's no difficulty // to copy `foo.0.x` to `foo.x`. - copy_gracefully(&crate_output.with_extension(ext), + let module_name = Some(&(trans.modules[0].name)[..]); + let path = crate_output.temp_path(output_type, module_name); + copy_gracefully(&path, &crate_output.path(output_type)); if !sess.opts.cg.save_temps && !keep_numbered { - // The user just wants `foo.x`, not `foo.0.x`. - remove(sess, &crate_output.with_extension(ext)); + // The user just wants `foo.x`, not `foo.#module-name#.x`. + remove(sess, &path); } - } else if crate_output.outputs.contains_key(&output_type) { - // 2) Multiple codegen units, with `--emit foo=some_name`. We have - // no good solution for this case, so warn the user. - sess.warn(&format!("ignoring emit path because multiple .{} files \ - were produced", ext)); - } else if crate_output.single_output_file.is_some() { - // 3) Multiple codegen units, with `-o some_name`. We have - // no good solution for this case, so warn the user. - sess.warn(&format!("ignoring -o because multiple .{} files \ - were produced", ext)); } else { - // 4) Multiple codegen units, but no explicit name. We - // just leave the `foo.0.x` files in place. - // (We don't have to do any work in this case.) + let ext = crate_output.temp_path(output_type, None) + .extension() + .unwrap() + .to_str() + .unwrap() + .to_owned(); + + if crate_output.outputs.contains_key(&output_type) { + // 2) Multiple codegen units, with `--emit foo=some_name`. We have + // no good solution for this case, so warn the user. + sess.warn(&format!("ignoring emit path because multiple .{} files \ + were produced", ext)); + } else if crate_output.single_output_file.is_some() { + // 3) Multiple codegen units, with `-o some_name`. We have + // no good solution for this case, so warn the user. + sess.warn(&format!("ignoring -o because multiple .{} files \ + were produced", ext)); + } else { + // 4) Multiple codegen units, but no explicit name. We + // just leave the `foo.0.x` files in place. + // (We don't have to do any work in this case.) + } } }; @@ -789,17 +806,19 @@ pub fn run_passes(sess: &Session, // Copy to .bc, but always keep the .0.bc. There is a later // check to figure out if we should delete .0.bc files, or keep // them for making an rlib. - copy_if_one_unit("0.bc", OutputType::Bitcode, true); + copy_if_one_unit(OutputType::Bitcode, true); } OutputType::LlvmAssembly => { - copy_if_one_unit("0.ll", OutputType::LlvmAssembly, false); + copy_if_one_unit(OutputType::LlvmAssembly, false); } OutputType::Assembly => { - copy_if_one_unit("0.s", OutputType::Assembly, false); + // TODO: These are probably wrong + copy_if_one_unit(OutputType::Assembly, false); } OutputType::Object => { user_wants_objects = true; - copy_if_one_unit("0.o", OutputType::Object, true); + // TODO: These are probably wrong + copy_if_one_unit(OutputType::Object, true); } OutputType::Exe | OutputType::DepInfo => {} @@ -810,51 +829,55 @@ pub fn run_passes(sess: &Session, // Clean up unwanted temporary files. // We create the following files by default: - // - crate.0.bc - // - crate.0.o + // - crate.#module-name#.bc + // - crate.#module-name#.o // - crate.metadata.bc // - crate.metadata.o // - crate.o (linked from crate.##.o) - // - crate.bc (copied from crate.0.bc) + // - crate.bc (copied from crate.##.bc) // We may create additional files if requested by the user (through // `-C save-temps` or `--emit=` flags). if !sess.opts.cg.save_temps { - // Remove the temporary .0.o objects. If the user didn't + // Remove the temporary .#module-name#.o objects. If the user didn't // explicitly request bitcode (with --emit=bc), and the bitcode is not - // needed for building an rlib, then we must remove .0.bc as well. + // needed for building an rlib, then we must remove .#module-name#.bc as + // well. - // Specific rules for keeping .0.bc: + // Specific rules for keeping .#module-name#.bc: // - If we're building an rlib (`needs_crate_bitcode`), then keep // it. // - If the user requested bitcode (`user_wants_bitcode`), and // codegen_units > 1, then keep it. // - If the user requested bitcode but codegen_units == 1, then we - // can toss .0.bc because we copied it to .bc earlier. + // can toss .#module-name#.bc because we copied it to .bc earlier. // - If we're not building an rlib and the user didn't request - // bitcode, then delete .0.bc. + // bitcode, then delete .#module-name#.bc. // If you change how this works, also update back::link::link_rlib, - // where .0.bc files are (maybe) deleted after making an rlib. + // where .#module-name#.bc files are (maybe) deleted after making an + // rlib. let keep_numbered_bitcode = needs_crate_bitcode || (user_wants_bitcode && sess.opts.cg.codegen_units > 1); let keep_numbered_objects = needs_crate_object || (user_wants_objects && sess.opts.cg.codegen_units > 1); - for i in 0..trans.modules.len() { + for module_name in trans.modules.iter().map(|m| Some(&m.name[..])) { if modules_config.emit_obj && !keep_numbered_objects { - let ext = format!("{}.o", i); - remove(sess, &crate_output.with_extension(&ext)); + let path = crate_output.temp_path(OutputType::Object, module_name); + remove(sess, &path); } if modules_config.emit_bc && !keep_numbered_bitcode { - let ext = format!("{}.bc", i); - remove(sess, &crate_output.with_extension(&ext)); + let path = crate_output.temp_path(OutputType::Bitcode, module_name); + remove(sess, &path); } } if metadata_config.emit_bc && !user_wants_bitcode { - remove(sess, &crate_output.with_extension("metadata.bc")); + let path = crate_output.temp_path(OutputType::Bitcode, + Some(&trans.metadata_module.name[..])); + remove(sess, &path); } } @@ -874,28 +897,31 @@ pub fn run_passes(sess: &Session, struct WorkItem { mtrans: ModuleTranslation, config: ModuleConfig, - output_names: OutputFilenames, - name_extra: String + output_names: OutputFilenames } fn build_work_item(sess: &Session, mtrans: ModuleTranslation, config: ModuleConfig, - output_names: OutputFilenames, - name_extra: String) + output_names: OutputFilenames) -> WorkItem { let mut config = config; config.tm = create_target_machine(sess); - WorkItem { mtrans: mtrans, config: config, output_names: output_names, - name_extra: name_extra } + WorkItem { + mtrans: mtrans, + config: config, + output_names: output_names + } } fn execute_work_item(cgcx: &CodegenContext, work_item: WorkItem) { unsafe { - optimize_and_codegen(cgcx, work_item.mtrans, work_item.config, - work_item.name_extra, work_item.output_names); + optimize_and_codegen(cgcx, + work_item.mtrans, + work_item.config, + work_item.output_names); } } @@ -914,6 +940,8 @@ fn run_work_singlethreaded(sess: &Session, fn run_work_multithreaded(sess: &Session, work_items: Vec, num_workers: usize) { + assert!(num_workers > 0); + // Run some workers to process the work items. let work_items_arc = Arc::new(Mutex::new(work_items)); let mut diag_emitter = SharedEmitter::new(); @@ -981,7 +1009,7 @@ pub fn run_assembler(sess: &Session, outputs: &OutputFilenames) { let (pname, mut cmd, _) = get_linker(sess); cmd.arg("-c").arg("-o").arg(&outputs.path(OutputType::Object)) - .arg(&outputs.temp_path(OutputType::Assembly)); + .arg(&outputs.temp_path(OutputType::Assembly, None)); debug!("{:?}", cmd); match cmd.output() { diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index fc45d73a7ae..41c214fe1b3 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2630,6 +2630,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }); let metadata_module = ModuleTranslation { + name: "metadata".to_string(), llcx: shared_ccx.metadata_llcx(), llmod: shared_ccx.metadata_llmod(), }; @@ -2644,7 +2645,11 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let crate_context_list = CrateContextList::new(&shared_ccx, codegen_units); let modules = crate_context_list.iter() - .map(|ccx| ModuleTranslation { llcx: ccx.llcx(), llmod: ccx.llmod() }) + .map(|ccx| ModuleTranslation { + name: String::from(&ccx.codegen_unit().name[..]), + llcx: ccx.llcx(), + llmod: ccx.llmod() + }) .collect(); // Skip crate items and just output metadata in -Z no-trans mode. @@ -2790,6 +2795,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } let linker_info = LinkerInfo::new(&shared_ccx, &reachable_symbols); + CrateTranslation { modules: modules, metadata_module: metadata_module, diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index 9cb5d8b6ad6..c465edc7311 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -129,8 +129,9 @@ mod type_; mod type_of; mod value; -#[derive(Copy, Clone)] +#[derive(Clone)] pub struct ModuleTranslation { + pub name: String, pub llcx: llvm::ContextRef, pub llmod: llvm::ModuleRef, } diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 2ded643ef4f..d15519a87d9 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -121,6 +121,7 @@ use llvm; use monomorphize; use rustc::hir::def_id::DefId; use rustc::hir::map::DefPathData; +use rustc::session::config::NUMBERED_CODEGEN_UNIT_MARKER; use rustc::ty::TyCtxt; use rustc::ty::item_path::characteristic_def_id_of_type; use syntax::parse::token::{self, InternedString}; @@ -283,7 +284,10 @@ fn merge_codegen_units<'tcx>(initial_partitioning: &mut PreInliningPartitioning< } fn numbered_codegen_unit_name(crate_name: &str, index: usize) -> InternedString { - token::intern_and_get_ident(&format!("{}.{}", crate_name, index)[..]) + token::intern_and_get_ident(&format!("{}{}{}", + crate_name, + NUMBERED_CODEGEN_UNIT_MARKER, + index)[..]) } } From 6c8c94b8488405ac3ade9c358e00fe96da9c72b8 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 19 May 2016 12:35:36 -0400 Subject: [PATCH 06/21] Improve linkage assignment in trans::partitioning. --- src/librustc_trans/base.rs | 23 +++-- src/librustc_trans/partitioning.rs | 94 ++++++++++++++++--- src/librustc_trans/trans_item.rs | 8 ++ .../partitioning/extern-drop-glue.rs | 4 +- .../partitioning/extern-generic.rs | 10 +- .../inlining-from-extern-crate.rs | 10 +- .../partitioning/local-drop-glue.rs | 6 +- .../partitioning/local-generic.rs | 8 +- .../partitioning/local-inlining.rs | 8 +- .../partitioning/local-transitive-inlining.rs | 8 +- .../methods-are-with-self-type.rs | 4 +- .../partitioning/regular-modules.rs | 28 +++--- .../codegen-units/partitioning/statics.rs | 4 +- 13 files changed, 146 insertions(+), 69 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 41c214fe1b3..3ee53a4d95e 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2548,8 +2548,8 @@ fn iter_functions(llmod: llvm::ModuleRef) -> ValueIter { /// /// This list is later used by linkers to determine the set of symbols needed to /// be exposed from a dynamic library and it's also encoded into the metadata. -pub fn filter_reachable_ids(scx: &SharedCrateContext) -> NodeSet { - scx.reachable().iter().map(|x| *x).filter(|&id| { +pub fn filter_reachable_ids(tcx: TyCtxt, reachable: NodeSet) -> NodeSet { + reachable.into_iter().filter(|&id| { // Next, we want to ignore some FFI functions that are not exposed from // this crate. Reachable FFI functions can be lumped into two // categories: @@ -2563,9 +2563,9 @@ pub fn filter_reachable_ids(scx: &SharedCrateContext) -> NodeSet { // // As a result, if this id is an FFI item (foreign item) then we only // let it through if it's included statically. - match scx.tcx().map.get(id) { + match tcx.map.get(id) { hir_map::NodeForeignItem(..) => { - scx.sess().cstore.is_statically_included_foreign_item(id) + tcx.sess.cstore.is_statically_included_foreign_item(id) } // Only consider nodes that actually have exported symbols. @@ -2575,8 +2575,8 @@ pub fn filter_reachable_ids(scx: &SharedCrateContext) -> NodeSet { node: hir::ItemFn(..), .. }) | hir_map::NodeImplItem(&hir::ImplItem { node: hir::ImplItemKind::Method(..), .. }) => { - let def_id = scx.tcx().map.local_def_id(id); - let scheme = scx.tcx().lookup_item_type(def_id); + let def_id = tcx.map.local_def_id(id); + let scheme = tcx.lookup_item_type(def_id); scheme.generics.types.is_empty() } @@ -2598,6 +2598,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let krate = tcx.map.krate(); let ty::CrateAnalysis { export_map, reachable, name, .. } = analysis; + let reachable = filter_reachable_ids(tcx, reachable); let check_overflow = if let Some(v) = tcx.sess.opts.debugging_opts.force_overflow_checks { v @@ -2621,12 +2622,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, reachable, check_overflow, check_dropflag); - - let reachable_symbol_ids = filter_reachable_ids(&shared_ccx); - // Translate the metadata. let metadata = time(tcx.sess.time_passes(), "write metadata", || { - write_metadata(&shared_ccx, &reachable_symbol_ids) + write_metadata(&shared_ccx, shared_ccx.reachable()) }); let metadata_module = ModuleTranslation { @@ -2755,7 +2753,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } let sess = shared_ccx.sess(); - let mut reachable_symbols = reachable_symbol_ids.iter().map(|&id| { + let mut reachable_symbols = shared_ccx.reachable().iter().map(|&id| { let def_id = shared_ccx.tcx().map.local_def_id(id); Instance::mono(&shared_ccx, def_id).symbol_name(&shared_ccx) }).collect::>(); @@ -2911,7 +2909,8 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a partitioning::partition(scx.tcx(), items.iter().cloned(), strategy, - &inlining_map) + &inlining_map, + scx.reachable()) }); if scx.sess().opts.debugging_opts.print_trans_items.is_some() { diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index d15519a87d9..cd08fe68f0c 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -126,7 +126,7 @@ use rustc::ty::TyCtxt; use rustc::ty::item_path::characteristic_def_id_of_type; use syntax::parse::token::{self, InternedString}; use trans_item::TransItem; -use util::nodemap::{FnvHashMap, FnvHashSet}; +use util::nodemap::{FnvHashMap, FnvHashSet, NodeSet}; pub struct CodegenUnit<'tcx> { pub name: InternedString, @@ -147,14 +147,23 @@ const FALLBACK_CODEGEN_UNIT: &'static str = "__rustc_fallback_codegen_unit"; pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_items: I, strategy: PartitioningStrategy, - inlining_map: &InliningMap<'tcx>) + inlining_map: &InliningMap<'tcx>, + reachable: &NodeSet) -> Vec> where I: Iterator> { + if let PartitioningStrategy::FixedUnitCount(1) = strategy { + // If there is only a single codegen-unit, we can use a very simple + // scheme and don't have to bother with doing much analysis. + return vec![single_codegen_unit(tcx, trans_items, reachable)]; + } + // In the first step, we place all regular translation items into their // 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(tcx, trans_items); + let mut initial_partitioning = place_root_translation_items(tcx, + trans_items, + reachable); // If the partitioning should produce a fixed count of codegen units, merge // until that count is reached. @@ -179,7 +188,8 @@ struct PreInliningPartitioning<'tcx> { struct PostInliningPartitioning<'tcx>(Vec>); fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - trans_items: I) + trans_items: I, + _reachable: &NodeSet) -> PreInliningPartitioning<'tcx> where I: Iterator> { @@ -219,7 +229,18 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, TransItem::Static(..) => llvm::ExternalLinkage, TransItem::DropGlue(..) => unreachable!(), // Is there any benefit to using ExternalLinkage?: - TransItem::Fn(..) => llvm::WeakODRLinkage, + TransItem::Fn(ref instance) => { + if instance.substs.types.is_empty() { + // This is a non-generic functions, we always + // make it visible externally on the chance that + // it might be used in another codegen unit. + llvm::ExternalLinkage + } else { + // Monomorphizations of generic functions are + // always weak-odr + llvm::WeakODRLinkage + } + } } } }; @@ -282,13 +303,6 @@ fn merge_codegen_units<'tcx>(initial_partitioning: &mut PreInliningPartitioning< items: FnvHashMap() }); } - - fn numbered_codegen_unit_name(crate_name: &str, index: usize) -> InternedString { - token::intern_and_get_ident(&format!("{}{}{}", - crate_name, - NUMBERED_CODEGEN_UNIT_MARKER, - index)[..]) - } } fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartitioning<'tcx>, @@ -319,6 +333,11 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit // so we just add it here with AvailableExternallyLinkage new_codegen_unit.items.insert(trans_item, llvm::AvailableExternallyLinkage); + } else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() { + // An instantiation of this item is always available in the + // crate it was imported from. + new_codegen_unit.items.insert(trans_item, + llvm::AvailableExternallyLinkage); } else { // We can't be sure if this will also be instantiated // somewhere else, so we add an instance here with @@ -414,3 +433,54 @@ fn compute_codegen_unit_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, return token::intern_and_get_ident(&mod_path[..]); } + +fn single_codegen_unit<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + trans_items: I, + reachable: &NodeSet) + -> CodegenUnit<'tcx> + where I: Iterator> +{ + let mut items = FnvHashMap(); + + for trans_item in trans_items { + let linkage = trans_item.explicit_linkage(tcx).unwrap_or_else(|| { + match trans_item { + TransItem::Static(node_id) => { + if reachable.contains(&node_id) { + llvm::ExternalLinkage + } else { + llvm::InternalLinkage + } + } + TransItem::DropGlue(_) => { + llvm::InternalLinkage + } + TransItem::Fn(instance) => { + if trans_item.is_generic_fn() || + trans_item.is_from_extern_crate() || + !reachable.contains(&tcx.map + .as_local_node_id(instance.def) + .unwrap()) { + llvm::InternalLinkage + } else { + llvm::ExternalLinkage + } + } + } + }); + + items.insert(trans_item, linkage); + } + + CodegenUnit { + name: numbered_codegen_unit_name(&tcx.crate_name[..], 0), + items: items + } +} + +fn numbered_codegen_unit_name(crate_name: &str, index: usize) -> InternedString { + token::intern_and_get_ident(&format!("{}{}{}", + crate_name, + NUMBERED_CODEGEN_UNIT_MARKER, + index)[..]) +} diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index f3c0ddfaf73..ae6e095d142 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -184,6 +184,14 @@ impl<'a, 'tcx> TransItem<'tcx> { } } + pub fn is_generic_fn(&self) -> bool { + match *self { + TransItem::Fn(ref instance) => !instance.substs.types.is_empty(), + TransItem::DropGlue(..) | + TransItem::Static(..) => false, + } + } + pub fn explicit_linkage(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Option { let def_id = match *self { TransItem::Fn(ref instance) => instance.def, diff --git a/src/test/codegen-units/partitioning/extern-drop-glue.rs b/src/test/codegen-units/partitioning/extern-drop-glue.rs index 5262d31ae0d..7072a211d24 100644 --- a/src/test/codegen-units/partitioning/extern-drop-glue.rs +++ b/src/test/codegen-units/partitioning/extern-drop-glue.rs @@ -25,7 +25,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[WeakODR] +//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External] fn user() { //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[OnceODR] @@ -37,7 +37,7 @@ mod mod1 { struct LocalStruct(cgu_extern_drop_glue::Struct); - //~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[WeakODR] + //~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External] fn user() { //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[OnceODR] diff --git a/src/test/codegen-units/partitioning/extern-generic.rs b/src/test/codegen-units/partitioning/extern-generic.rs index 6beed231df9..5801727494f 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[WeakODR] +//~ TRANS_ITEM fn extern_generic::user[0] @@ extern_generic[External] 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[WeakODR] + //~ TRANS_ITEM fn extern_generic::mod1[0]::user[0] @@ extern_generic-mod1[External] 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[WeakODR] + //~ TRANS_ITEM fn extern_generic::mod1[0]::mod1[0]::user[0] @@ extern_generic-mod1-mod1[External] fn user() { let _ = cgu_generic_function::foo("abc"); } @@ -45,14 +45,14 @@ mod mod1 { mod mod2 { use cgu_generic_function; - //~ TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[WeakODR] + //~ TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[External] fn user() { let _ = cgu_generic_function::foo("abc"); } } mod mod3 { - //~ TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[WeakODR] + //~ TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[External] fn non_user() {} } 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 967824f31d0..285b068fe1c 100644 --- a/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs +++ b/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs @@ -21,10 +21,10 @@ extern crate cgu_explicit_inlining; // This test makes sure that items inlined from external crates are privately // instantiated in every codegen unit they are used in. -//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[OnceODR] inlining_from_extern_crate-mod1[OnceODR] -//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[OnceODR] inlining_from_extern_crate-mod2[OnceODR] +//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod1[Available] +//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod2[Available] -//~ TRANS_ITEM fn inlining_from_extern_crate::user[0] @@ inlining_from_extern_crate[WeakODR] +//~ TRANS_ITEM fn inlining_from_extern_crate::user[0] @@ inlining_from_extern_crate[External] pub fn user() { cgu_explicit_inlining::inlined(); @@ -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[WeakODR] + //~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[External] 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[WeakODR] + //~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[External] 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 04ebef645ec..dc50650de6d 100644 --- a/src/test/codegen-units/partitioning/local-drop-glue.rs +++ b/src/test/codegen-units/partitioning/local-drop-glue.rs @@ -23,7 +23,7 @@ struct Struct { } impl Drop for Struct { - //~ TRANS_ITEM fn local_drop_glue::{{impl}}[0]::drop[0] @@ local_drop_glue[WeakODR] + //~ TRANS_ITEM fn local_drop_glue::{{impl}}[0]::drop[0] @@ local_drop_glue[External] fn drop(&mut self) {} } @@ -32,7 +32,7 @@ struct Outer { _a: Struct } -//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[WeakODR] +//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[External] fn user() { let _ = Outer { @@ -53,7 +53,7 @@ mod mod1 _b: (u32, Struct), } - //~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[WeakODR] + //~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[External] 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 f5641f1f2ed..bfc911e36e9 100644 --- a/src/test/codegen-units/partitioning/local-generic.rs +++ b/src/test/codegen-units/partitioning/local-generic.rs @@ -25,7 +25,7 @@ //~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[WeakODR] pub fn generic(x: T) -> T { x } -//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[WeakODR] +//~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External] fn user() { let _ = generic(0u32); } @@ -33,7 +33,7 @@ fn user() { mod mod1 { pub use super::generic; - //~ TRANS_ITEM fn local_generic::mod1[0]::user[0] @@ local_generic-mod1[WeakODR] + //~ TRANS_ITEM fn local_generic::mod1[0]::user[0] @@ local_generic-mod1[External] fn user() { let _ = generic(0u64); } @@ -41,7 +41,7 @@ mod mod1 { mod mod1 { use super::generic; - //~ TRANS_ITEM fn local_generic::mod1[0]::mod1[0]::user[0] @@ local_generic-mod1-mod1[WeakODR] + //~ TRANS_ITEM fn local_generic::mod1[0]::mod1[0]::user[0] @@ local_generic-mod1-mod1[External] fn user() { let _ = generic('c'); } @@ -51,7 +51,7 @@ mod mod1 { mod mod2 { use super::generic; - //~ TRANS_ITEM fn local_generic::mod2[0]::user[0] @@ local_generic-mod2[WeakODR] + //~ TRANS_ITEM fn local_generic::mod2[0]::user[0] @@ local_generic-mod2[External] 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 880cc0a4fb4..5eb1cbc2199 100644 --- a/src/test/codegen-units/partitioning/local-inlining.rs +++ b/src/test/codegen-units/partitioning/local-inlining.rs @@ -19,7 +19,7 @@ mod inline { // Important: This function should show up in all codegen units where it is inlined - //~ TRANS_ITEM fn local_inlining::inline[0]::inlined_function[0] @@ local_inlining-inline[WeakODR] local_inlining-user1[Available] local_inlining-user2[Available] + //~ TRANS_ITEM fn local_inlining::inline[0]::inlined_function[0] @@ local_inlining-inline[External] local_inlining-user1[Available] local_inlining-user2[Available] #[inline(always)] pub fn inlined_function() { @@ -30,7 +30,7 @@ mod inline { mod user1 { use super::inline; - //~ TRANS_ITEM fn local_inlining::user1[0]::foo[0] @@ local_inlining-user1[WeakODR] + //~ TRANS_ITEM fn local_inlining::user1[0]::foo[0] @@ local_inlining-user1[External] 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[WeakODR] + //~ TRANS_ITEM fn local_inlining::user2[0]::bar[0] @@ local_inlining-user2[External] 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[WeakODR] + //~ TRANS_ITEM fn local_inlining::non_user[0]::baz[0] @@ local_inlining-non_user[External] 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 f3efa2587d3..28c4698eabd 100644 --- a/src/test/codegen-units/partitioning/local-transitive-inlining.rs +++ b/src/test/codegen-units/partitioning/local-transitive-inlining.rs @@ -18,7 +18,7 @@ mod inline { - //~ TRANS_ITEM fn local_transitive_inlining::inline[0]::inlined_function[0] @@ local_transitive_inlining-inline[WeakODR] local_transitive_inlining-direct_user[Available] local_transitive_inlining-indirect_user[Available] + //~ TRANS_ITEM fn local_transitive_inlining::inline[0]::inlined_function[0] @@ local_transitive_inlining-inline[External] local_transitive_inlining-direct_user[Available] local_transitive_inlining-indirect_user[Available] #[inline(always)] pub fn inlined_function() { @@ -29,7 +29,7 @@ mod inline { mod direct_user { use super::inline; - //~ TRANS_ITEM fn local_transitive_inlining::direct_user[0]::foo[0] @@ local_transitive_inlining-direct_user[WeakODR] local_transitive_inlining-indirect_user[Available] + //~ TRANS_ITEM fn local_transitive_inlining::direct_user[0]::foo[0] @@ local_transitive_inlining-direct_user[External] local_transitive_inlining-indirect_user[Available] #[inline(always)] pub fn foo() { inline::inlined_function(); @@ -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[WeakODR] + //~ TRANS_ITEM fn local_transitive_inlining::indirect_user[0]::bar[0] @@ local_transitive_inlining-indirect_user[External] 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[WeakODR] + //~ TRANS_ITEM fn local_transitive_inlining::non_user[0]::baz[0] @@ local_transitive_inlining-non_user[External] fn baz() { } diff --git a/src/test/codegen-units/partitioning/methods-are-with-self-type.rs b/src/test/codegen-units/partitioning/methods-are-with-self-type.rs index 99dda0e38ba..f8e7d8d2554 100644 --- a/src/test/codegen-units/partitioning/methods-are-with-self-type.rs +++ b/src/test/codegen-units/partitioning/methods-are-with-self-type.rs @@ -25,10 +25,10 @@ mod mod1 { // Even though the impl is in `mod1`, the methods should end up in the // parent module, since that is where their self-type is. impl SomeType { - //~ TRANS_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[0]::method[0] @@ methods_are_with_self_type[WeakODR] + //~ TRANS_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[0]::method[0] @@ methods_are_with_self_type[External] fn method(&self) {} - //~ TRANS_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[0]::associated_fn[0] @@ methods_are_with_self_type[WeakODR] + //~ TRANS_ITEM fn methods_are_with_self_type::mod1[0]::{{impl}}[0]::associated_fn[0] @@ methods_are_with_self_type[External] fn associated_fn() {} } diff --git a/src/test/codegen-units/partitioning/regular-modules.rs b/src/test/codegen-units/partitioning/regular-modules.rs index c3af86f820f..4da64110321 100644 --- a/src/test/codegen-units/partitioning/regular-modules.rs +++ b/src/test/codegen-units/partitioning/regular-modules.rs @@ -16,10 +16,10 @@ #![allow(dead_code)] #![crate_type="lib"] -//~ TRANS_ITEM fn regular_modules::foo[0] @@ regular_modules[WeakODR] +//~ TRANS_ITEM fn regular_modules::foo[0] @@ regular_modules[External] fn foo() {} -//~ TRANS_ITEM fn regular_modules::bar[0] @@ regular_modules[WeakODR] +//~ TRANS_ITEM fn regular_modules::bar[0] @@ regular_modules[External] fn bar() {} //~ TRANS_ITEM static regular_modules::BAZ[0] @@ regular_modules[External] @@ -27,26 +27,26 @@ static BAZ: u64 = 0; mod mod1 { - //~ TRANS_ITEM fn regular_modules::mod1[0]::foo[0] @@ regular_modules-mod1[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod1[0]::foo[0] @@ regular_modules-mod1[External] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod1[0]::bar[0] @@ regular_modules-mod1[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod1[0]::bar[0] @@ regular_modules-mod1[External] fn bar() {} //~ TRANS_ITEM static regular_modules::mod1[0]::BAZ[0] @@ regular_modules-mod1[External] static BAZ: u64 = 0; mod mod1 { - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::foo[0] @@ regular_modules-mod1-mod1[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::foo[0] @@ regular_modules-mod1-mod1[External] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::bar[0] @@ regular_modules-mod1-mod1[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod1[0]::bar[0] @@ regular_modules-mod1-mod1[External] fn bar() {} //~ TRANS_ITEM static regular_modules::mod1[0]::mod1[0]::BAZ[0] @@ regular_modules-mod1-mod1[External] static BAZ: u64 = 0; } mod mod2 { - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::foo[0] @@ regular_modules-mod1-mod2[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::foo[0] @@ regular_modules-mod1-mod2[External] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::bar[0] @@ regular_modules-mod1-mod2[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod1[0]::mod2[0]::bar[0] @@ regular_modules-mod1-mod2[External] fn bar() {} //~ TRANS_ITEM static regular_modules::mod1[0]::mod2[0]::BAZ[0] @@ regular_modules-mod1-mod2[External] static BAZ: u64 = 0; @@ -55,26 +55,26 @@ mod mod1 { mod mod2 { - //~ TRANS_ITEM fn regular_modules::mod2[0]::foo[0] @@ regular_modules-mod2[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod2[0]::foo[0] @@ regular_modules-mod2[External] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod2[0]::bar[0] @@ regular_modules-mod2[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod2[0]::bar[0] @@ regular_modules-mod2[External] fn bar() {} //~ TRANS_ITEM static regular_modules::mod2[0]::BAZ[0] @@ regular_modules-mod2[External] static BAZ: u64 = 0; mod mod1 { - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::foo[0] @@ regular_modules-mod2-mod1[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::foo[0] @@ regular_modules-mod2-mod1[External] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::bar[0] @@ regular_modules-mod2-mod1[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod1[0]::bar[0] @@ regular_modules-mod2-mod1[External] fn bar() {} //~ TRANS_ITEM static regular_modules::mod2[0]::mod1[0]::BAZ[0] @@ regular_modules-mod2-mod1[External] static BAZ: u64 = 0; } mod mod2 { - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::foo[0] @@ regular_modules-mod2-mod2[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::foo[0] @@ regular_modules-mod2-mod2[External] fn foo() {} - //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::bar[0] @@ regular_modules-mod2-mod2[WeakODR] + //~ TRANS_ITEM fn regular_modules::mod2[0]::mod2[0]::bar[0] @@ regular_modules-mod2-mod2[External] fn bar() {} //~ TRANS_ITEM static regular_modules::mod2[0]::mod2[0]::BAZ[0] @@ regular_modules-mod2-mod2[External] static BAZ: u64 = 0; diff --git a/src/test/codegen-units/partitioning/statics.rs b/src/test/codegen-units/partitioning/statics.rs index 9e878b95a36..ffe1ec278b8 100644 --- a/src/test/codegen-units/partitioning/statics.rs +++ b/src/test/codegen-units/partitioning/statics.rs @@ -21,7 +21,7 @@ static FOO: u32 = 0; //~ TRANS_ITEM static statics::BAR[0] @@ statics[External] static BAR: u32 = 0; -//~ TRANS_ITEM fn statics::function[0] @@ statics[WeakODR] +//~ TRANS_ITEM fn statics::function[0] @@ statics[External] fn function() { //~ TRANS_ITEM static statics::function[0]::FOO[0] @@ statics[External] static FOO: u32 = 0; @@ -37,7 +37,7 @@ mod mod1 { //~ TRANS_ITEM static statics::mod1[0]::BAR[0] @@ statics-mod1[External] static BAR: u32 = 0; - //~ TRANS_ITEM fn statics::mod1[0]::function[0] @@ statics-mod1[WeakODR] + //~ TRANS_ITEM fn statics::mod1[0]::function[0] @@ statics-mod1[External] fn function() { //~ TRANS_ITEM static statics::mod1[0]::function[0]::FOO[0] @@ statics-mod1[External] static FOO: u32 = 0; From 87c1c87dd7389466333a976d9c536c083c311443 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 9 May 2016 23:56:49 -0400 Subject: [PATCH 07/21] Make drop-glue translation collector-driven. --- src/librustc_trans/abi.rs | 1 + src/librustc_trans/back/symbol_names.rs | 13 +++ src/librustc_trans/base.rs | 68 +-------------- src/librustc_trans/collector.rs | 11 +-- src/librustc_trans/context.rs | 20 ++--- src/librustc_trans/declare.rs | 24 ++++-- src/librustc_trans/glue.rs | 56 +++--------- src/librustc_trans/partitioning.rs | 27 ++++++ src/librustc_trans/trans_item.rs | 108 +++++++++++++++++++++--- 9 files changed, 178 insertions(+), 150 deletions(-) diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs index df3d2d149b9..6c2a09f8060 100644 --- a/src/librustc_trans/abi.rs +++ b/src/librustc_trans/abi.rs @@ -229,6 +229,7 @@ impl ArgType { /// /// I will do my best to describe this structure, but these /// comments are reverse-engineered and may be inaccurate. -NDM +#[derive(Clone)] pub struct FnType { /// The LLVM types of each argument. pub args: Vec, diff --git a/src/librustc_trans/back/symbol_names.rs b/src/librustc_trans/back/symbol_names.rs index 170c8f75b50..ebb6e0baf20 100644 --- a/src/librustc_trans/back/symbol_names.rs +++ b/src/librustc_trans/back/symbol_names.rs @@ -304,6 +304,19 @@ impl ItemPathBuffer for SymbolPathBuffer { } } +pub fn exported_name_from_type_and_prefix<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, + t: ty::Ty<'tcx>, + prefix: &str) + -> String { + let empty_def_path = DefPath { + data: vec![], + krate: cstore::LOCAL_CRATE, + }; + let hash = get_symbol_hash(scx, &empty_def_path, t, &[]); + let path = [token::intern_and_get_ident(prefix)]; + mangle(path.iter().cloned(), Some(&hash[..])) +} + /// Only symbols that are invisible outside their compilation unit should use a /// name generated by this function. pub fn internal_name_from_type_and_suffix<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 3ee53a4d95e..5dc319aa5f9 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2182,52 +2182,6 @@ pub fn llvm_linkage_by_name(name: &str) -> Option { } } -/// Set the appropriate linkage for an LLVM `ValueRef` (function or global). -/// If the `llval` is the direct translation of a specific Rust item, `id` -/// should be set to the `NodeId` of that item. (This mapping should be -/// 1-to-1, so monomorphizations and drop/visit glue should have `id` set to -/// `None`.) -pub fn update_linkage(ccx: &CrateContext, - llval: ValueRef, - id: Option) { - if let Some(id) = id { - let item = ccx.tcx().map.get(id); - if let hir_map::NodeItem(i) = item { - if let Some(name) = attr::first_attr_value_str_by_name(&i.attrs, "linkage") { - if let Some(linkage) = llvm_linkage_by_name(&name) { - llvm::SetLinkage(llval, linkage); - } else { - ccx.sess().span_fatal(i.span, "invalid linkage specified"); - } - return; - } - } - } - - let (is_reachable, is_generic) = if let Some(id) = id { - (ccx.reachable().contains(&id), false) - } else { - (false, true) - }; - - // We need external linkage for items reachable from other translation units, this include - // other codegen units in case of parallel compilations. - if is_reachable || ccx.sess().opts.cg.codegen_units > 1 { - if is_generic { - // This only happens with multiple codegen units, in which case we need to use weak_odr - // linkage because other crates might expose the same symbol. We cannot use - // linkonce_odr here because the symbol might then get dropped before the other codegen - // units get to link it. - llvm::SetUniqueComdat(ccx.llmod(), llval); - llvm::SetLinkage(llval, llvm::WeakODRLinkage); - } else { - llvm::SetLinkage(llval, llvm::ExternalLinkage); - } - } else { - llvm::SetLinkage(llval, llvm::InternalLinkage); - } -} - pub fn set_link_section(ccx: &CrateContext, llval: ValueRef, attrs: &[ast::Attribute]) { @@ -2673,24 +2627,8 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // ... and now that we have everything pre-defined, fill out those definitions. for ccx in crate_context_list.iter() { - for (&trans_item, _) in &ccx.codegen_unit().items { - match trans_item { - TransItem::Static(node_id) => { - let item = ccx.tcx().map.expect_item(node_id); - if let hir::ItemStatic(_, m, ref expr) = item.node { - match consts::trans_static(&ccx, m, expr, item.id, &item.attrs) { - Ok(_) => { /* Cool, everything's alright. */ }, - Err(err) => ccx.tcx().sess.span_fatal(expr.span, &err.description()), - }; - } else { - span_bug!(item.span, "Mismatch between hir::Item type and TransItem type") - } - } - TransItem::Fn(instance) => { - trans_instance(&ccx, instance); - } - _ => { } - } + for (trans_item, _) in &ccx.codegen_unit().items { + trans_item.define(&ccx); } } @@ -2927,7 +2865,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a let mut item_keys: Vec<_> = items .iter() .map(|i| { - let mut output = i.to_string(scx); + let mut output = i.to_string(scx.tcx()); output.push_str(" @@"); let mut empty = Vec::new(); let mut cgus = item_to_cgus.get_mut(i).unwrap_or(&mut empty); diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 8cb9fd23e49..aff50e5e1f4 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -325,7 +325,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, // We've been here already, no need to search again. return; } - debug!("BEGIN collect_items_rec({})", starting_point.to_string(scx)); + debug!("BEGIN collect_items_rec({})", starting_point.to_string(scx.tcx())); let mut neighbors = Vec::new(); let recursion_depth_reset; @@ -396,7 +396,7 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, recursion_depths.insert(def_id, depth); } - debug!("END collect_items_rec({})", starting_point.to_string(scx)); + debug!("END collect_items_rec({})", starting_point.to_string(scx.tcx())); } fn record_inlining_canditates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -637,7 +637,8 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { let operand_ty = monomorphize::apply_param_substs(tcx, self.param_substs, &mt.ty); - self.output.push(TransItem::DropGlue(DropGlueKind::Ty(operand_ty))); + let ty = glue::get_drop_glue_type(tcx, operand_ty); + self.output.push(TransItem::DropGlue(DropGlueKind::Ty(ty))); } else { bug!("Has the drop_in_place() intrinsic's signature changed?") } @@ -1271,7 +1272,7 @@ pub fn print_collection_results<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) { let mut item_keys = FnvHashMap(); for (item, item_state) in trans_items.iter() { - let k = item.to_string(scx); + let k = item.to_string(scx.tcx()); if item_keys.contains_key(&k) { let prev: (TransItem, TransItemState) = item_keys[&k]; @@ -1299,7 +1300,7 @@ pub fn print_collection_results<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) { let mut generated = FnvHashSet(); for (item, item_state) in trans_items.iter() { - let item_key = item.to_string(scx); + let item_key = item.to_string(scx.tcx()); match *item_state { TransItemState::PredictedAndGenerated => { diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index bfcb1ae33b3..7f6e8fa035a 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -36,7 +36,7 @@ use rustc::ty::{self, Ty, TyCtxt}; use session::config::NoDebugInfo; use session::Session; use util::sha2::Sha256; -use util::nodemap::{NodeMap, NodeSet, DefIdMap, FnvHashMap, FnvHashSet}; +use util::nodemap::{NodeMap, NodeSet, DefIdMap, FnvHashMap}; use std::ffi::{CStr, CString}; use std::cell::{Cell, RefCell}; @@ -46,6 +46,7 @@ use std::rc::Rc; use std::str; use syntax::ast; use syntax::parse::token::InternedString; +use abi::FnType; pub struct Stats { pub n_glues_created: Cell, @@ -80,8 +81,6 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { mir_map: &'a MirMap<'tcx>, mir_cache: RefCell>>>, - available_monomorphizations: RefCell>, - available_drop_glues: RefCell, String>>, use_dll_storage_attrs: bool, translation_items: RefCell, TransItemState>>, @@ -99,7 +98,7 @@ pub struct LocalCrateContext<'tcx> { codegen_unit: CodegenUnit<'tcx>, needs_unwind_cleanup_cache: RefCell, bool>>, fn_pointer_shims: RefCell, ValueRef>>, - drop_glues: RefCell, ValueRef>>, + drop_glues: RefCell, (ValueRef, FnType)>>, /// Track mapping of external ids to local items imported for inlining external: RefCell>>, /// Backwards version of the `external` map (inlined items to where they @@ -413,8 +412,6 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { }, check_overflow: check_overflow, check_drop_flag_for_sanity: check_drop_flag_for_sanity, - available_monomorphizations: RefCell::new(FnvHashSet()), - available_drop_glues: RefCell::new(FnvHashMap()), use_dll_storage_attrs: use_dll_storage_attrs, translation_items: RefCell::new(FnvHashMap()), trait_cache: RefCell::new(DepTrackingMap::new(tcx.dep_graph.clone())), @@ -730,7 +727,8 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.local().fn_pointer_shims } - pub fn drop_glues<'a>(&'a self) -> &'a RefCell, ValueRef>> { + pub fn drop_glues<'a>(&'a self) + -> &'a RefCell, (ValueRef, FnType)>> { &self.local().drop_glues } @@ -816,14 +814,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &self.shared.stats } - pub fn available_monomorphizations<'a>(&'a self) -> &'a RefCell> { - &self.shared.available_monomorphizations - } - - pub fn available_drop_glues(&self) -> &RefCell, String>> { - &self.shared.available_drop_glues - } - pub fn int_type(&self) -> Type { self.local().int_type } diff --git a/src/librustc_trans/declare.rs b/src/librustc_trans/declare.rs index e6db695943b..2746d3fb6b0 100644 --- a/src/librustc_trans/declare.rs +++ b/src/librustc_trans/declare.rs @@ -138,6 +138,20 @@ pub fn define_global(ccx: &CrateContext, name: &str, ty: Type) -> Option(ccx: &CrateContext<'a, 'tcx>, + name: &str, + fn_type: ty::Ty<'tcx>) -> ValueRef { + if get_defined_value(ccx, name).is_some() { + ccx.sess().fatal(&format!("symbol `{}` already defined", name)) + } else { + declare_fn(ccx, name, fn_type) + } +} /// Declare a Rust function with an intention to define it. /// @@ -147,13 +161,9 @@ pub fn define_global(ccx: &CrateContext, name: &str, ty: Type) -> Option(ccx: &CrateContext<'a, 'tcx>, name: &str, fn_type: ty::Ty<'tcx>) -> ValueRef { - if get_defined_value(ccx, name).is_some() { - ccx.sess().fatal(&format!("symbol `{}` already defined", name)) - } else { - let llfn = declare_fn(ccx, name, fn_type); - llvm::SetLinkage(llfn, llvm::InternalLinkage); - llfn - } + let llfn = define_fn(ccx, name, fn_type); + llvm::SetLinkage(llfn, llvm::InternalLinkage); + llfn } diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index e4519ff82a0..30eb71a9006 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -14,15 +14,12 @@ use std; -use attributes; -use back::symbol_names; use llvm; use llvm::{ValueRef, get_param}; use middle::lang_items::ExchangeFreeFnLangItem; use rustc::ty::subst::{Substs}; use rustc::traits; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; -use abi::{Abi, FnType}; use adt; use adt::GetDtorType; // for tcx.dtor_type() use base::*; @@ -33,7 +30,6 @@ use cleanup::CleanupMethods; use collector; use common::*; use debuginfo::DebugLoc; -use declare; use expr; use machine::*; use monomorphize; @@ -236,48 +232,21 @@ impl<'tcx> DropGlueKind<'tcx> { fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, g: DropGlueKind<'tcx>) -> ValueRef { - debug!("make drop glue for {:?}", g); let g = g.map_ty(|t| get_drop_glue_type(ccx.tcx(), t)); - debug!("drop glue type {:?}", g); match ccx.drop_glues().borrow().get(&g) { - Some(&glue) => return glue, - _ => { } + Some(&(glue, _)) => glue, + None => { bug!("Could not find drop glue for {:?} -- {} -- {}", + g, + TransItem::DropGlue(g).to_raw_string(), + ccx.codegen_unit().name) } } - let t = g.ty(); +} +pub fn implement_drop_glue<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, + g: DropGlueKind<'tcx>) { let tcx = ccx.tcx(); - let sig = ty::FnSig { - inputs: vec![tcx.mk_mut_ptr(tcx.types.i8)], - output: ty::FnOutput::FnConverging(tcx.mk_nil()), - variadic: false, - }; - // Create a FnType for fn(*mut i8) and substitute the real type in - // later - that prevents FnType from splitting fat pointers up. - let mut fn_ty = FnType::new(ccx, Abi::Rust, &sig, &[]); - fn_ty.args[0].original_ty = type_of(ccx, t).ptr_to(); - let llfnty = fn_ty.llvm_type(ccx); - - // To avoid infinite recursion, don't `make_drop_glue` until after we've - // added the entry to the `drop_glues` cache. - if let Some(old_sym) = ccx.available_drop_glues().borrow().get(&g) { - let llfn = declare::declare_cfn(ccx, &old_sym, llfnty); - ccx.drop_glues().borrow_mut().insert(g, llfn); - return llfn; - }; - - let suffix = match g { - DropGlueKind::Ty(_) => "drop", - DropGlueKind::TyContents(_) => "drop_contents", - }; - - let fn_nm = symbol_names::internal_name_from_type_and_suffix(ccx, t, suffix); - assert!(declare::get_defined_value(ccx, &fn_nm).is_none()); - let llfn = declare::declare_cfn(ccx, &fn_nm, llfnty); - attributes::set_frame_pointer_elimination(ccx, llfn); - ccx.available_drop_glues().borrow_mut().insert(g, fn_nm); - ccx.drop_glues().borrow_mut().insert(g, llfn); - - let _s = StatRecorder::new(ccx, format!("drop {:?}", t)); + assert_eq!(g.ty(), get_drop_glue_type(tcx, g.ty())); + let (llfn, fn_ty) = ccx.drop_glues().borrow().get(&g).unwrap().clone(); let (arena, fcx): (TypedArena<_>, FunctionContext); arena = TypedArena::new(); @@ -285,8 +254,6 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let bcx = fcx.init(false, None); - update_linkage(ccx, llfn, None); - ccx.stats().n_glues_created.set(ccx.stats().n_glues_created.get() + 1); // All glue functions take values passed *by alias*; this is a // requirement since in many contexts glue is invoked indirectly and @@ -298,10 +265,9 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let bcx = make_drop_glue(bcx, get_param(llfn, 0), g); fcx.finish(bcx, DebugLoc::None); - - llfn } + fn trans_struct_drop_flag<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, t: Ty<'tcx>, struct_data: ValueRef) diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index cd08fe68f0c..a0360a8ed22 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -165,10 +165,14 @@ pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trans_items, reachable); + debug_dump(tcx, "INITIAL PARTITONING:", initial_partitioning.codegen_units.iter()); + // If the partitioning should produce a fixed count of codegen units, merge // until that count is reached. if let PartitioningStrategy::FixedUnitCount(count) = strategy { merge_codegen_units(&mut initial_partitioning, count, &tcx.crate_name[..]); + + debug_dump(tcx, "POST MERGING:", initial_partitioning.codegen_units.iter()); } // In the next step, we use the inlining map to determine which addtional @@ -177,6 +181,9 @@ pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // local functions the definition of which is marked with #[inline]. let post_inlining = place_inlined_translation_items(initial_partitioning, inlining_map); + + debug_dump(tcx, "POST INLINING:", post_inlining.0.iter()); + post_inlining.0 } @@ -484,3 +491,23 @@ fn numbered_codegen_unit_name(crate_name: &str, index: usize) -> InternedString NUMBERED_CODEGEN_UNIT_MARKER, index)[..]) } + +fn debug_dump<'a, 'b, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + label: &str, + cgus: I) + where I: Iterator>, + 'tcx: 'a + 'b +{ + if cfg!(debug_assertions) { + debug!("{}", label); + for cgu in cgus { + debug!("CodegenUnit {}:", cgu.name); + + for (trans_item, linkage) in &cgu.items { + debug!(" - {} [{:?}]", trans_item.to_string(tcx), linkage); + } + + debug!(""); + } + } +} diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index ae6e095d142..a07deda2b1e 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -16,7 +16,8 @@ use attributes; use base; -use context::{SharedCrateContext, CrateContext}; +use consts; +use context::CrateContext; use declare; use glue::DropGlueKind; use llvm; @@ -32,7 +33,9 @@ use syntax::ast::{self, NodeId}; use syntax::{attr,errors}; use syntax::parse::token; use type_of; - +use glue; +use abi::{Abi, FnType}; +use back::symbol_names; #[derive(PartialEq, Eq, Clone, Copy, Debug)] pub enum TransItem<'tcx> { @@ -64,9 +67,47 @@ impl<'tcx> Hash for TransItem<'tcx> { impl<'a, 'tcx> TransItem<'tcx> { + pub fn define(&self, ccx: &CrateContext<'a, 'tcx>) { + + debug!("BEGIN IMPLEMENTING '{} ({})' in cgu {}", + self.to_string(ccx.tcx()), + self.to_raw_string(), + ccx.codegen_unit().name); + + match *self { + TransItem::Static(node_id) => { + let item = ccx.tcx().map.expect_item(node_id); + if let hir::ItemStatic(_, m, ref expr) = item.node { + match consts::trans_static(&ccx, m, expr, item.id, &item.attrs) { + Ok(_) => { /* Cool, everything's alright. */ }, + Err(err) => ccx.tcx().sess.span_fatal(expr.span, &err.description()), + }; + } else { + span_bug!(item.span, "Mismatch between hir::Item type and TransItem type") + } + } + TransItem::Fn(instance) => { + base::trans_instance(&ccx, instance); + } + TransItem::DropGlue(dg) => { + glue::implement_drop_glue(&ccx, dg); + } + } + + debug!("END IMPLEMENTING '{} ({})' in cgu {}", + self.to_string(ccx.tcx()), + self.to_raw_string(), + ccx.codegen_unit().name); + } + pub fn predefine(&self, ccx: &CrateContext<'a, 'tcx>, linkage: llvm::Linkage) { + debug!("BEGIN PREDEFINING '{} ({})' in cgu {}", + self.to_string(ccx.tcx()), + self.to_raw_string(), + ccx.codegen_unit().name); + match *self { TransItem::Static(node_id) => { TransItem::predefine_static(ccx, node_id, linkage); @@ -74,10 +115,15 @@ impl<'a, 'tcx> TransItem<'tcx> { TransItem::Fn(instance) => { TransItem::predefine_fn(ccx, instance, linkage); } - _ => { - // Not yet implemented + TransItem::DropGlue(dg) => { + TransItem::predefine_drop_glue(ccx, dg, linkage); } } + + debug!("END PREDEFINING '{} ({})' in cgu {}", + self.to_string(ccx.tcx()), + self.to_raw_string(), + ccx.codegen_unit().name); } fn predefine_static(ccx: &CrateContext<'a, 'tcx>, @@ -93,7 +139,7 @@ impl<'a, 'tcx> TransItem<'tcx> { }) => { let instance = Instance::mono(ccx.shared(), def_id); let sym = instance.symbol_name(ccx.shared()); - debug!("making {}", sym); + debug!("symbol {}", sym); let g = declare::define_global(ccx, &sym, llty).unwrap_or_else(|| { ccx.sess().span_fatal(span, @@ -110,8 +156,6 @@ impl<'a, 'tcx> TransItem<'tcx> { fn predefine_fn(ccx: &CrateContext<'a, 'tcx>, instance: Instance<'tcx>, linkage: llvm::Linkage) { - let unit = ccx.codegen_unit(); - debug!("predefine_fn[cg={}](instance={:?})", &unit.name[..], instance); assert!(!instance.substs.types.needs_infer() && !instance.substs.types.has_param_types()); @@ -143,10 +187,11 @@ impl<'a, 'tcx> TransItem<'tcx> { ref attrs, node: hir::ImplItemKind::Method(..), .. }) => { let symbol = instance.symbol_name(ccx.shared()); - let lldecl = declare::declare_fn(ccx, &symbol, mono_ty); + debug!("symbol {}", symbol); - attributes::from_fn_attrs(ccx, attrs, lldecl); + let lldecl = declare::declare_fn(ccx, &symbol, mono_ty); llvm::SetLinkage(lldecl, linkage); + attributes::from_fn_attrs(ccx, attrs, lldecl); base::set_link_section(ccx, lldecl, attrs); ccx.instances().borrow_mut().insert(instance, lldecl); @@ -156,6 +201,39 @@ impl<'a, 'tcx> TransItem<'tcx> { } + fn predefine_drop_glue(ccx: &CrateContext<'a, 'tcx>, + dg: glue::DropGlueKind<'tcx>, + linkage: llvm::Linkage) { + let tcx = ccx.tcx(); + assert_eq!(dg.ty(), glue::get_drop_glue_type(tcx, dg.ty())); + let t = dg.ty(); + + let sig = ty::FnSig { + inputs: vec![tcx.mk_mut_ptr(tcx.types.i8)], + output: ty::FnOutput::FnConverging(tcx.mk_nil()), + variadic: false, + }; + + // Create a FnType for fn(*mut i8) and substitute the real type in + // later - that prevents FnType from splitting fat pointers up. + let mut fn_ty = FnType::new(ccx, Abi::Rust, &sig, &[]); + fn_ty.args[0].original_ty = type_of::type_of(ccx, t).ptr_to(); + let llfnty = fn_ty.llvm_type(ccx); + + let prefix = match dg { + DropGlueKind::Ty(_) => "drop", + DropGlueKind::TyContents(_) => "drop_contents", + }; + + let symbol = + symbol_names::exported_name_from_type_and_prefix(ccx.shared(), t, prefix); + debug!(" symbol: {}", symbol); + assert!(declare::get_defined_value(ccx, &symbol).is_none()); + let llfn = declare::declare_cfn(ccx, &symbol, llfnty); + attributes::set_frame_pointer_elimination(ccx, llfn); + llvm::SetLinkage(llfn, linkage); + ccx.drop_glues().borrow_mut().insert(dg, (llfn, fn_ty)); + } pub fn requests_inline(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { match *self { @@ -216,8 +294,7 @@ impl<'a, 'tcx> TransItem<'tcx> { } } - pub fn to_string(&self, scx: &SharedCrateContext<'a, 'tcx>) -> String { - let tcx = scx.tcx(); + pub fn to_string(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { let hir_map = &tcx.map; return match *self { @@ -235,7 +312,8 @@ impl<'a, 'tcx> TransItem<'tcx> { }, TransItem::Static(node_id) => { let def_id = hir_map.local_def_id(node_id); - let instance = Instance::mono(scx, def_id); + let instance = Instance::new(def_id, + tcx.mk_substs(subst::Substs::empty())); to_string_internal(tcx, "static ", instance) }, }; @@ -254,7 +332,11 @@ impl<'a, 'tcx> TransItem<'tcx> { pub fn to_raw_string(&self) -> String { match *self { TransItem::DropGlue(dg) => { - format!("DropGlue({})", dg.ty() as *const _ as usize) + let prefix = match dg { + DropGlueKind::Ty(_) => "Ty", + DropGlueKind::TyContents(_) => "TyContents", + }; + format!("DropGlue({}: {})", prefix, dg.ty() as *const _ as usize) } TransItem::Fn(instance) => { format!("Fn({:?}, {})", From b38e0d0d44c6de6f0c72a4a1f8f9aa8c74ea842e Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 26 May 2016 08:59:58 -0400 Subject: [PATCH 08/21] Build SymbolMap for symbol name conflict checking and caching. --- src/librustc_trans/base.rs | 45 +++++++++-- src/librustc_trans/callee.rs | 16 +++- src/librustc_trans/consts.rs | 17 +++-- src/librustc_trans/context.rs | 16 +++- src/librustc_trans/lib.rs | 1 + src/librustc_trans/monomorphize.rs | 13 +++- src/librustc_trans/symbol_map.rs | 115 +++++++++++++++++++++++++++++ src/librustc_trans/trans_item.rs | 68 +++++++++-------- 8 files changed, 237 insertions(+), 54 deletions(-) create mode 100644 src/librustc_trans/symbol_map.rs diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 5dc319aa5f9..3fc7fe51288 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -80,6 +80,7 @@ use meth; use mir; use monomorphize::{self, Instance}; use partitioning::{self, PartitioningStrategy, CodegenUnit}; +use symbol_map::SymbolMap; use symbol_names_test; use trans_item::TransItem; use tvec; @@ -97,6 +98,7 @@ use libc::c_uint; use std::ffi::{CStr, CString}; use std::cell::{Cell, RefCell}; use std::collections::{HashMap, HashSet}; +use std::rc::Rc; use std::str; use std::{i8, i16, i32, i64}; use syntax_pos::{Span, DUMMY_SP}; @@ -2588,14 +2590,18 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; let no_builtins = attr::contains_name(&krate.attrs, "no_builtins"); - let codegen_units = collect_and_partition_translation_items(&shared_ccx); + let (codegen_units, symbol_map) = + collect_and_partition_translation_items(&shared_ccx); let codegen_unit_count = codegen_units.len(); assert!(tcx.sess.opts.cg.codegen_units == codegen_unit_count || tcx.sess.opts.debugging_opts.incremental.is_some()); - let crate_context_list = CrateContextList::new(&shared_ccx, codegen_units); + let symbol_map = Rc::new(symbol_map); + let crate_context_list = CrateContextList::new(&shared_ccx, + codegen_units, + symbol_map.clone()); let modules = crate_context_list.iter() .map(|ccx| ModuleTranslation { name: String::from(&ccx.codegen_unit().name[..]), @@ -2693,8 +2699,9 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let sess = shared_ccx.sess(); let mut reachable_symbols = shared_ccx.reachable().iter().map(|&id| { let def_id = shared_ccx.tcx().map.local_def_id(id); - Instance::mono(&shared_ccx, def_id).symbol_name(&shared_ccx) + symbol_for_def_id(def_id, &shared_ccx, &symbol_map) }).collect::>(); + if sess.entry_fn.borrow().is_some() { reachable_symbols.push("main".to_string()); } @@ -2716,7 +2723,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, reachable_symbols.extend(syms.into_iter().filter(|did| { sess.cstore.is_extern_item(shared_ccx.tcx(), *did) }).map(|did| { - Instance::mono(&shared_ccx, did).symbol_name(&shared_ccx) + symbol_for_def_id(did, &shared_ccx, &symbol_map) })); } @@ -2810,7 +2817,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for TransItemsWithinModVisitor<'a, 'tcx> { } fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) - -> Vec> { + -> (Vec>, SymbolMap<'tcx>) { let time_passes = scx.sess().time_passes(); let collection_mode = match scx.sess().opts.debugging_opts.print_trans_items { @@ -2833,10 +2840,13 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a None => TransItemCollectionMode::Lazy }; - let (items, inlining_map) = time(time_passes, "translation item collection", || { - collector::collect_crate_translation_items(&scx, collection_mode) + let (items, inlining_map) = + time(time_passes, "translation item collection", || { + collector::collect_crate_translation_items(&scx, collection_mode) }); + let symbol_map = SymbolMap::build(scx, items.iter().cloned()); + let strategy = if scx.sess().opts.debugging_opts.incremental.is_some() { PartitioningStrategy::PerModule } else { @@ -2910,5 +2920,24 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a } } - codegen_units + (codegen_units, symbol_map) +} + +fn symbol_for_def_id<'a, 'tcx>(def_id: DefId, + scx: &SharedCrateContext<'a, 'tcx>, + symbol_map: &SymbolMap<'tcx>) + -> String { + // Just try to look things up in the symbol map. If nothing's there, we + // recompute. + if let Some(node_id) = scx.tcx().map.as_local_node_id(def_id) { + if let Some(sym) = symbol_map.get(TransItem::Static(node_id)) { + return sym.to_owned(); + } + } + + let instance = Instance::mono(scx, def_id); + + symbol_map.get(TransItem::Fn(instance)) + .map(str::to_owned) + .unwrap_or_else(|| instance.symbol_name(scx)) } diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index 9ea65532b35..b7cf43a4371 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -46,6 +46,7 @@ use intrinsic; use machine::llalign_of_min; use meth; use monomorphize::{self, Instance}; +use trans_item::TransItem; use type_::Type; use type_of; use value::Value; @@ -536,11 +537,18 @@ fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // reference. It also occurs when testing libcore and in some // other weird situations. Annoying. - let sym = instance.symbol_name(ccx.shared()); + // Let's see if we can get the symbol name from the symbol_map, so we don't + // have to recompute it. + let mut sym_data = String::new(); + let sym = ccx.symbol_map().get(TransItem::Fn(instance)).unwrap_or_else(|| { + sym_data = instance.symbol_name(ccx.shared()); + &sym_data[..] + }); + let llptrty = type_of::type_of(ccx, fn_ptr_ty); - let llfn = if let Some(llfn) = declare::get_declared_value(ccx, &sym) { + let llfn = if let Some(llfn) = declare::get_declared_value(ccx, sym) { if let Some(span) = local_item { - if declare::get_defined_value(ccx, &sym).is_some() { + if declare::get_defined_value(ccx, sym).is_some() { ccx.sess().span_fatal(span, &format!("symbol `{}` is already defined", sym)); } @@ -558,7 +566,7 @@ fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, llfn } } else { - let llfn = declare::declare_fn(ccx, &sym, ty); + let llfn = declare::declare_fn(ccx, sym, ty); assert_eq!(common::val_ty(llfn), llptrty); debug!("get_fn: not casting pointer!"); diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index 1a38baeff37..3f18c61fbd2 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -1013,14 +1013,16 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) return Datum::new(g, ty, Lvalue::new("static")); } - let sym = instance.symbol_name(ccx.shared()); - let g = if let Some(id) = ccx.tcx().map.as_local_node_id(def_id) { + let llty = type_of::type_of(ccx, ty); let (g, attrs) = match ccx.tcx().map.get(id) { hir_map::NodeItem(&hir::Item { ref attrs, span, node: hir::ItemStatic(..), .. }) => { + let sym = ccx.symbol_map() + .get(TransItem::Static(id)) + .expect("Local statics should always be in the SymbolMap"); // Make sure that this is never executed for something inlined. assert!(!ccx.external_srcs().borrow().contains_key(&id)); @@ -1028,16 +1030,16 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) .items .contains_key(&TransItem::Static(id)); if defined_in_current_codegen_unit { - if declare::get_declared_value(ccx, &sym).is_none() { + if declare::get_declared_value(ccx, sym).is_none() { span_bug!(span, "trans: Static not properly pre-defined?"); } } else { - if declare::get_declared_value(ccx, &sym).is_some() { + if declare::get_declared_value(ccx, sym).is_some() { span_bug!(span, "trans: Conflicting symbol names for static?"); } } - let g = declare::define_global(ccx, &sym, llty).unwrap(); + let g = declare::define_global(ccx, sym, llty).unwrap(); (g, attrs) } @@ -1045,6 +1047,7 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) hir_map::NodeForeignItem(&hir::ForeignItem { ref attrs, span, node: hir::ForeignItemStatic(..), .. }) => { + let sym = instance.symbol_name(ccx.shared()); let g = if let Some(name) = attr::first_attr_value_str_by_name(&attrs, "linkage") { // If this is a static with a linkage specified, then we need to handle @@ -1079,7 +1082,7 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) real_name.push_str(&sym); let g2 = declare::define_global(ccx, &real_name, llty).unwrap_or_else(||{ ccx.sess().span_fatal(span, - &format!("symbol `{}` is already defined", sym)) + &format!("symbol `{}` is already defined", &sym)) }); llvm::SetLinkage(g2, llvm::InternalLinkage); llvm::LLVMSetInitializer(g2, g1); @@ -1104,6 +1107,8 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) g } else { + let sym = instance.symbol_name(ccx.shared()); + // FIXME(nagisa): perhaps the map of externs could be offloaded to llvm somehow? // FIXME(nagisa): investigate whether it can be changed into define_global let g = declare::declare_global(ccx, &sym, type_of::type_of(ccx, ty)); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 7f6e8fa035a..64e0351610f 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -35,6 +35,7 @@ use rustc::ty::subst::{Substs, VecPerParamSpace}; use rustc::ty::{self, Ty, TyCtxt}; use session::config::NoDebugInfo; use session::Session; +use symbol_map::SymbolMap; use util::sha2::Sha256; use util::nodemap::{NodeMap, NodeSet, DefIdMap, FnvHashMap}; @@ -171,6 +172,8 @@ pub struct LocalCrateContext<'tcx> { /// Depth of the current type-of computation - used to bail out type_of_depth: Cell, + + symbol_map: Rc>, } // Implement DepTrackingMapConfig for `trait_cache` @@ -197,12 +200,13 @@ pub struct CrateContextList<'a, 'tcx: 'a> { impl<'a, 'tcx: 'a> CrateContextList<'a, 'tcx> { pub fn new(shared_ccx: &'a SharedCrateContext<'a, 'tcx>, - codegen_units: Vec>) + codegen_units: Vec>, + symbol_map: Rc>) -> CrateContextList<'a, 'tcx> { CrateContextList { shared: shared_ccx, local_ccxs: codegen_units.into_iter().map(|codegen_unit| { - LocalCrateContext::new(shared_ccx, codegen_unit) + LocalCrateContext::new(shared_ccx, codegen_unit, symbol_map.clone()) }).collect() } } @@ -512,7 +516,8 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { impl<'tcx> LocalCrateContext<'tcx> { fn new<'a>(shared: &SharedCrateContext<'a, 'tcx>, - codegen_unit: CodegenUnit<'tcx>) + codegen_unit: CodegenUnit<'tcx>, + symbol_map: Rc>) -> LocalCrateContext<'tcx> { unsafe { // Append ".rs" to LLVM module identifier. @@ -571,6 +576,7 @@ impl<'tcx> LocalCrateContext<'tcx> { intrinsics: RefCell::new(FnvHashMap()), n_llvm_insns: Cell::new(0), type_of_depth: Cell::new(0), + symbol_map: symbol_map, }; let (int_type, opaque_vec_type, str_slice_ty, mut local_ccx) = { @@ -890,6 +896,10 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { self.shared.get_mir(def_id) } + pub fn symbol_map(&self) -> &SymbolMap<'tcx> { + &*self.local().symbol_map + } + pub fn translation_items(&self) -> &RefCell, TransItemState>> { &self.shared.translation_items } diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index c465edc7311..fa0a1fdc375 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -122,6 +122,7 @@ mod meth; mod mir; mod monomorphize; mod partitioning; +mod symbol_map; mod symbol_names_test; mod trans_item; mod tvec; diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index 832acfe14be..8a2cc53432d 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -84,19 +84,24 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, monomorphizing.insert(fn_id, depth + 1); } - let symbol = instance.symbol_name(ccx.shared()); + // Let's see if we can get the symbol name from the symbol_map, so we don't + // have to recompute it. + let mut sym_data = String::new(); + let symbol = ccx.symbol_map().get(TransItem::Fn(instance)).unwrap_or_else(|| { + sym_data = instance.symbol_name(ccx.shared()); + &sym_data[..] + }); debug!("monomorphize_fn mangled to {}", symbol); - assert!(declare::get_defined_value(ccx, &symbol).is_none()); + assert!(declare::get_defined_value(ccx, symbol).is_none()); // FIXME(nagisa): perhaps needs a more fine grained selection? - let lldecl = declare::define_internal_fn(ccx, &symbol, mono_ty); + let lldecl = declare::define_internal_fn(ccx, symbol, mono_ty); // FIXME(eddyb) Doubt all extern fn should allow unwinding. attributes::unwind(lldecl, true); ccx.instances().borrow_mut().insert(instance, lldecl); - // we can only monomorphize things in this crate (or inlined into it) let fn_node_id = ccx.tcx().map.as_local_node_id(fn_id).unwrap(); let map_node = errors::expect( diff --git a/src/librustc_trans/symbol_map.rs b/src/librustc_trans/symbol_map.rs new file mode 100644 index 00000000000..4f82b54c76b --- /dev/null +++ b/src/librustc_trans/symbol_map.rs @@ -0,0 +1,115 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use context::SharedCrateContext; +use monomorphize::Instance; +use rustc::ty::TyCtxt; +use syntax::codemap::Span; +use trans_item::TransItem; +use util::nodemap::FnvHashMap; + + +// In the SymbolMap we collect the symbol names of all translation items of +// the current crate. + +pub struct SymbolMap<'tcx> { + index: FnvHashMap, (usize, usize)>, + arena: String, +} + +impl<'tcx> SymbolMap<'tcx> { + + pub fn build<'a, I>(scx: &SharedCrateContext<'a, 'tcx>, + trans_items: I) + -> SymbolMap<'tcx> + where I: Iterator> + { + // Check for duplicate symbol names + let mut symbols: Vec<_> = trans_items.map(|trans_item| { + (trans_item, trans_item.compute_symbol_name(scx)) + }).collect(); + + (&mut symbols[..]).sort_by(|&(_, ref sym1), &(_, ref sym2)|{ + sym1.cmp(sym2) + }); + + for pair in (&symbols[..]).windows(2) { + let sym1 = &pair[0].1; + let sym2 = &pair[1].1; + + if *sym1 == *sym2 { + let trans_item1 = pair[0].0; + let trans_item2 = pair[1].0; + + let span1 = get_span(scx.tcx(), trans_item1); + let span2 = get_span(scx.tcx(), trans_item2); + + // Deterministically select one of the spans for error reporting + let span = match (span1, span2) { + (Some(span1), Some(span2)) => { + Some(if span1.lo.0 > span2.lo.0 { + span1 + } else { + span2 + }) + } + (Some(span), None) | + (None, Some(span)) => Some(span), + _ => None + }; + + let error_message = format!("symbol `{}` is already defined", sym1); + + if let Some(span) = span { + scx.sess().span_fatal(span, &error_message) + } else { + scx.sess().fatal(&error_message) + } + } + } + + let mut symbol_map = SymbolMap { + index: FnvHashMap(), + arena: String::with_capacity(1024), + }; + + for (trans_item, symbol) in symbols { + let start_index = symbol_map.arena.len(); + symbol_map.arena.push_str(&symbol[..]); + let end_index = symbol_map.arena.len(); + let prev_entry = symbol_map.index.insert(trans_item, + (start_index, end_index)); + if prev_entry.is_some() { + bug!("TransItem encountered twice?") + } + } + + fn get_span<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + trans_item: TransItem<'tcx>) -> Option { + match trans_item { + TransItem::Fn(Instance { def, .. }) => { + tcx.map.as_local_node_id(def) + } + TransItem::Static(node_id) => Some(node_id), + TransItem::DropGlue(_) => None, + }.map(|node_id| { + tcx.map.span(node_id) + }) + } + + symbol_map + } + + pub fn get(&self, trans_item: TransItem<'tcx>) -> Option<&str> { + self.index.get(&trans_item).map(|&(start_index, end_index)| { + &self.arena[start_index .. end_index] + }) + } +} diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index a07deda2b1e..4fa0ba03005 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -17,7 +17,7 @@ use attributes; use base; use consts; -use context::CrateContext; +use context::{CrateContext, SharedCrateContext}; use declare; use glue::DropGlueKind; use llvm; @@ -64,7 +64,6 @@ impl<'tcx> Hash for TransItem<'tcx> { } } - impl<'a, 'tcx> TransItem<'tcx> { pub fn define(&self, ccx: &CrateContext<'a, 'tcx>) { @@ -108,15 +107,20 @@ impl<'a, 'tcx> TransItem<'tcx> { self.to_raw_string(), ccx.codegen_unit().name); + let symbol_name = ccx.symbol_map() + .get(*self) + .expect("Name not present in SymbolMap?"); + debug!("symbol {}", symbol_name); + match *self { TransItem::Static(node_id) => { - TransItem::predefine_static(ccx, node_id, linkage); + TransItem::predefine_static(ccx, node_id, linkage, symbol_name); } TransItem::Fn(instance) => { - TransItem::predefine_fn(ccx, instance, linkage); + TransItem::predefine_fn(ccx, instance, linkage, symbol_name); } TransItem::DropGlue(dg) => { - TransItem::predefine_drop_glue(ccx, dg, linkage); + TransItem::predefine_drop_glue(ccx, dg, linkage, symbol_name); } } @@ -128,7 +132,8 @@ impl<'a, 'tcx> TransItem<'tcx> { fn predefine_static(ccx: &CrateContext<'a, 'tcx>, node_id: ast::NodeId, - linkage: llvm::Linkage) { + linkage: llvm::Linkage, + symbol_name: &str) { let def_id = ccx.tcx().map.local_def_id(node_id); let ty = ccx.tcx().lookup_item_type(def_id).ty; let llty = type_of::type_of(ccx, ty); @@ -137,13 +142,9 @@ impl<'a, 'tcx> TransItem<'tcx> { hir::map::NodeItem(&hir::Item { span, node: hir::ItemStatic(..), .. }) => { - let instance = Instance::mono(ccx.shared(), def_id); - let sym = instance.symbol_name(ccx.shared()); - debug!("symbol {}", sym); - - let g = declare::define_global(ccx, &sym, llty).unwrap_or_else(|| { + let g = declare::define_global(ccx, symbol_name, llty).unwrap_or_else(|| { ccx.sess().span_fatal(span, - &format!("symbol `{}` is already defined", sym)) + &format!("symbol `{}` is already defined", symbol_name)) }); llvm::SetLinkage(g, linkage); @@ -155,7 +156,8 @@ impl<'a, 'tcx> TransItem<'tcx> { fn predefine_fn(ccx: &CrateContext<'a, 'tcx>, instance: Instance<'tcx>, - linkage: llvm::Linkage) { + linkage: llvm::Linkage, + symbol_name: &str) { assert!(!instance.substs.types.needs_infer() && !instance.substs.types.has_param_types()); @@ -186,10 +188,7 @@ impl<'a, 'tcx> TransItem<'tcx> { hir_map::NodeImplItem(&hir::ImplItem { ref attrs, node: hir::ImplItemKind::Method(..), .. }) => { - let symbol = instance.symbol_name(ccx.shared()); - debug!("symbol {}", symbol); - - let lldecl = declare::declare_fn(ccx, &symbol, mono_ty); + let lldecl = declare::declare_fn(ccx, symbol_name, mono_ty); llvm::SetLinkage(lldecl, linkage); attributes::from_fn_attrs(ccx, attrs, lldecl); base::set_link_section(ccx, lldecl, attrs); @@ -203,7 +202,8 @@ impl<'a, 'tcx> TransItem<'tcx> { fn predefine_drop_glue(ccx: &CrateContext<'a, 'tcx>, dg: glue::DropGlueKind<'tcx>, - linkage: llvm::Linkage) { + linkage: llvm::Linkage, + symbol_name: &str) { let tcx = ccx.tcx(); assert_eq!(dg.ty(), glue::get_drop_glue_type(tcx, dg.ty())); let t = dg.ty(); @@ -220,21 +220,31 @@ impl<'a, 'tcx> TransItem<'tcx> { fn_ty.args[0].original_ty = type_of::type_of(ccx, t).ptr_to(); let llfnty = fn_ty.llvm_type(ccx); - let prefix = match dg { - DropGlueKind::Ty(_) => "drop", - DropGlueKind::TyContents(_) => "drop_contents", - }; - - let symbol = - symbol_names::exported_name_from_type_and_prefix(ccx.shared(), t, prefix); - debug!(" symbol: {}", symbol); - assert!(declare::get_defined_value(ccx, &symbol).is_none()); - let llfn = declare::declare_cfn(ccx, &symbol, llfnty); + assert!(declare::get_defined_value(ccx, symbol_name).is_none()); + let llfn = declare::declare_cfn(ccx, symbol_name, llfnty); + llvm::SetLinkage(llfn, linkage); attributes::set_frame_pointer_elimination(ccx, llfn); - llvm::SetLinkage(llfn, linkage); ccx.drop_glues().borrow_mut().insert(dg, (llfn, fn_ty)); } + pub fn compute_symbol_name(&self, + scx: &SharedCrateContext<'a, 'tcx>) -> String { + match *self { + TransItem::Fn(instance) => instance.symbol_name(scx), + TransItem::Static(node_id) => { + let def_id = scx.tcx().map.local_def_id(node_id); + Instance::mono(scx, def_id).symbol_name(scx) + } + TransItem::DropGlue(dg) => { + let prefix = match dg { + DropGlueKind::Ty(_) => "drop", + DropGlueKind::TyContents(_) => "drop_contents", + }; + symbol_names::exported_name_from_type_and_prefix(scx, dg.ty(), prefix) + } + } + } + pub fn requests_inline(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { match *self { TransItem::Fn(ref instance) => { From 37a10ecbe82c040ae307c48a0a0cdbcd9f05f6a3 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 26 May 2016 11:43:53 -0400 Subject: [PATCH 09/21] Make item translation order deterministic by sorting by symbol name. --- src/librustc_trans/base.rs | 10 ++++++-- src/librustc_trans/partitioning.rs | 37 +++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 3fc7fe51288..4bbbae85fb2 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2626,14 +2626,20 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Instantiate translation items without filling out definitions yet... for ccx in crate_context_list.iter() { - for (&trans_item, &linkage) in &ccx.codegen_unit().items { + let trans_items = ccx.codegen_unit() + .items_in_deterministic_order(&symbol_map); + + for (trans_item, linkage) in trans_items { trans_item.predefine(&ccx, linkage); } } // ... and now that we have everything pre-defined, fill out those definitions. for ccx in crate_context_list.iter() { - for (trans_item, _) in &ccx.codegen_unit().items { + let trans_items = ccx.codegen_unit() + .items_in_deterministic_order(&symbol_map); + + for (trans_item, _) in trans_items { trans_item.define(&ccx); } } diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index a0360a8ed22..0e06fd6235e 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -124,15 +124,11 @@ use rustc::hir::map::DefPathData; use rustc::session::config::NUMBERED_CODEGEN_UNIT_MARKER; use rustc::ty::TyCtxt; use rustc::ty::item_path::characteristic_def_id_of_type; +use symbol_map::SymbolMap; use syntax::parse::token::{self, InternedString}; use trans_item::TransItem; use util::nodemap::{FnvHashMap, FnvHashSet, NodeSet}; -pub struct CodegenUnit<'tcx> { - pub name: InternedString, - pub items: FnvHashMap, llvm::Linkage>, -} - pub enum PartitioningStrategy { /// Generate one codegen unit per source-level module. PerModule, @@ -141,6 +137,29 @@ pub enum PartitioningStrategy { FixedUnitCount(usize) } +pub struct CodegenUnit<'tcx> { + pub name: InternedString, + pub items: FnvHashMap, llvm::Linkage>, +} + +impl<'tcx> CodegenUnit<'tcx> { + pub fn items_in_deterministic_order(&self, + symbol_map: &SymbolMap) + -> Vec<(TransItem<'tcx>, llvm::Linkage)> { + let mut items: Vec<(TransItem<'tcx>, llvm::Linkage)> = + self.items.iter().map(|(item, linkage)| (*item, *linkage)).collect(); + + items.as_mut_slice().sort_by(|&(trans_item1, _), &(trans_item2, _)| { + let symbol_name1 = symbol_map.get(trans_item1).unwrap(); + let symbol_name2 = symbol_map.get(trans_item2).unwrap(); + symbol_name1.cmp(symbol_name2) + }); + + items + } +} + + // Anything we can't find a proper codegen unit for goes into this. const FALLBACK_CODEGEN_UNIT: &'static str = "__rustc_fallback_codegen_unit"; @@ -184,7 +203,13 @@ pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, debug_dump(tcx, "POST INLINING:", post_inlining.0.iter()); - post_inlining.0 + // Finally, sort by codegen unit name, so that we get deterministic results + let mut result = post_inlining.0; + result.as_mut_slice().sort_by(|cgu1, cgu2| { + (&cgu1.name[..]).cmp(&cgu2.name[..]) + }); + + result } struct PreInliningPartitioning<'tcx> { From 283c94cd49c8b7cf0b565902686db5fb0cf3e6fd Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 26 May 2016 12:18:39 -0400 Subject: [PATCH 10/21] Clean up trans::trans_crate() after making things collector driven. --- src/librustc_trans/base.rs | 117 +++++++++++++----------- src/librustc_trans/symbol_names_test.rs | 16 ++-- 2 files changed, 70 insertions(+), 63 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 4bbbae85fb2..234ed800c47 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2198,34 +2198,17 @@ pub fn set_link_section(ccx: &CrateContext, } } -pub fn trans_item(ccx: &CrateContext, item: &hir::Item) { +fn trans_item(ccx: &CrateContext, item: &hir::Item) { let _icx = push_ctxt("trans_item"); - let tcx = ccx.tcx(); match item.node { - hir::ItemFn(_, _, _, _, _, _) => { - let def_id = tcx.map.local_def_id(item.id); - // check for the #[rustc_error] annotation, which forces an - // error in trans. This is used to write compile-fail tests - // that actually test that compilation succeeds without - // reporting an error. - if is_entry_fn(ccx.sess(), item.id) { - let empty_substs = ccx.empty_substs_for_def_id(def_id); - let llfn = Callee::def(ccx, def_id, empty_substs).reify(ccx).val; - create_entry_wrapper(ccx, item.span, llfn); - if tcx.has_attr(def_id, "rustc_error") { - tcx.sess.span_fatal(item.span, "compilation successful"); - } - } - - // Function is actually translated in trans_instance - } hir::ItemEnum(ref enum_definition, ref gens) => { if gens.ty_params.is_empty() { // sizes only make sense for non-generic types enum_variant_size_lint(ccx, enum_definition, item.span, item.id); } } + hir::ItemFn(..) | hir::ItemImpl(..) | hir::ItemStatic(..) => { // Don't do anything here. Translation has been moved to @@ -2235,22 +2218,40 @@ pub fn trans_item(ccx: &CrateContext, item: &hir::Item) { } } -pub fn is_entry_fn(sess: &Session, node_id: ast::NodeId) -> bool { - match *sess.entry_fn.borrow() { - Some((entry_id, _)) => node_id == entry_id, - None => false, - } -} +/// Create the `main` function which will initialise the rust runtime and call +/// users’ main function. +pub fn maybe_create_entry_wrapper(ccx: &CrateContext) { + let (main_def_id, span) = match *ccx.sess().entry_fn.borrow() { + Some((id, span)) => { + (ccx.tcx().map.local_def_id(id), span) + } + None => return, + }; + + // check for the #[rustc_error] annotation, which forces an + // error in trans. This is used to write compile-fail tests + // that actually test that compilation succeeds without + // reporting an error. + if ccx.tcx().has_attr(main_def_id, "rustc_error") { + ccx.tcx().sess.span_fatal(span, "compilation successful"); + } + + let instance = Instance::mono(ccx.shared(), main_def_id); + + if !ccx.codegen_unit().items.contains_key(&TransItem::Fn(instance)) { + // We want to create the wrapper in the same codegen unit as Rust's main + // function. + return; + } + + let main_llfn = Callee::def(ccx, main_def_id, instance.substs).reify(ccx).val; -/// Create the `main` function which will initialise the rust runtime and call users’ main -/// function. -pub fn create_entry_wrapper(ccx: &CrateContext, sp: Span, main_llfn: ValueRef) { let et = ccx.sess().entry_type.get().unwrap(); match et { config::EntryMain => { - create_entry_fn(ccx, sp, main_llfn, true); + create_entry_fn(ccx, span, main_llfn, true); } - config::EntryStart => create_entry_fn(ccx, sp, main_llfn, false), + config::EntryStart => create_entry_fn(ccx, span, main_llfn, false), config::EntryNone => {} // Do nothing. } @@ -2590,13 +2591,11 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; let no_builtins = attr::contains_name(&krate.attrs, "no_builtins"); - let (codegen_units, symbol_map) = - collect_and_partition_translation_items(&shared_ccx); + // Run the translation item collector and partition the collected items into + // codegen units. + let (codegen_units, symbol_map) = collect_and_partition_translation_items(&shared_ccx); let codegen_unit_count = codegen_units.len(); - assert!(tcx.sess.opts.cg.codegen_units == codegen_unit_count || - tcx.sess.opts.debugging_opts.incremental.is_some()); - let symbol_map = Rc::new(symbol_map); let crate_context_list = CrateContextList::new(&shared_ccx, @@ -2642,28 +2641,12 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, for (trans_item, _) in trans_items { trans_item.define(&ccx); } - } - { - let ccx = crate_context_list.get_ccx(0); + // If this codegen unit contains the main function, also create the + // wrapper here + maybe_create_entry_wrapper(&ccx); - // Translate all items. See `TransModVisitor` for - // details on why we walk in this particular way. - { - let _icx = push_ctxt("text"); - intravisit::walk_mod(&mut TransItemsWithinModVisitor { ccx: &ccx }, &krate.module); - krate.visit_all_items(&mut TransModVisitor { ccx: &ccx }); - } - - collector::print_collection_results(ccx.shared()); - - symbol_names_test::report_symbol_names(&ccx); - } - - for ccx in crate_context_list.iter() { - if ccx.sess().opts.debuginfo != NoDebugInfo { - debuginfo::finalize(&ccx); - } + // Run replace-all-uses-with for statics that need it for &(old_g, new_g) in ccx.statics_to_rauw().borrow().iter() { unsafe { let bitcast = llvm::LLVMConstPointerCast(new_g, llvm::LLVMTypeOf(old_g)); @@ -2671,6 +2654,26 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, llvm::LLVMDeleteGlobal(old_g); } } + + // Finalize debuginfo + if ccx.sess().opts.debuginfo != NoDebugInfo { + debuginfo::finalize(&ccx); + } + } + + collector::print_collection_results(&shared_ccx); + symbol_names_test::report_symbol_names(&shared_ccx); + + { + let ccx = crate_context_list.get_ccx(0); + + // At this point, we only walk the HIR for running + // enum_variant_size_lint(). This should arguably be moved somewhere + // else + { + intravisit::walk_mod(&mut TransItemsWithinModVisitor { ccx: &ccx }, &krate.module); + krate.visit_all_items(&mut TransModVisitor { ccx: &ccx }); + } } if shared_ccx.sess().trans_stats() { @@ -2696,6 +2699,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } } + if shared_ccx.sess().count_llvm_insns() { for (k, v) in shared_ccx.stats().llvm_insns.borrow().iter() { println!("{:7} {}", *v, *k); @@ -2867,6 +2871,9 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a scx.reachable()) }); + assert!(scx.tcx().sess.opts.cg.codegen_units == codegen_units.len() || + scx.tcx().sess.opts.debugging_opts.incremental.is_some()); + if scx.sess().opts.debugging_opts.print_trans_items.is_some() { let mut item_to_cgus = HashMap::new(); diff --git a/src/librustc_trans/symbol_names_test.rs b/src/librustc_trans/symbol_names_test.rs index 11e9e9f3204..9a7fe54e0d9 100644 --- a/src/librustc_trans/symbol_names_test.rs +++ b/src/librustc_trans/symbol_names_test.rs @@ -19,40 +19,40 @@ use rustc::hir::intravisit::{self, Visitor}; use syntax::ast; use syntax::attr::AttrMetaMethods; -use common::CrateContext; +use common::SharedCrateContext; use monomorphize::Instance; const SYMBOL_NAME: &'static str = "rustc_symbol_name"; const ITEM_PATH: &'static str = "rustc_item_path"; -pub fn report_symbol_names(ccx: &CrateContext) { +pub fn report_symbol_names(scx: &SharedCrateContext) { // if the `rustc_attrs` feature is not enabled, then the // attributes we are interested in cannot be present anyway, so // skip the walk. - let tcx = ccx.tcx(); + let tcx = scx.tcx(); if !tcx.sess.features.borrow().rustc_attrs { return; } let _ignore = tcx.dep_graph.in_ignore(); - let mut visitor = SymbolNamesTest { ccx: ccx }; + let mut visitor = SymbolNamesTest { scx: scx }; tcx.map.krate().visit_all_items(&mut visitor); } struct SymbolNamesTest<'a, 'tcx:'a> { - ccx: &'a CrateContext<'a, 'tcx>, + scx: &'a SharedCrateContext<'a, 'tcx>, } impl<'a, 'tcx> SymbolNamesTest<'a, 'tcx> { fn process_attrs(&mut self, node_id: ast::NodeId) { - let tcx = self.ccx.tcx(); + let tcx = self.scx.tcx(); let def_id = tcx.map.local_def_id(node_id); for attr in tcx.get_attrs(def_id).iter() { if attr.check_name(SYMBOL_NAME) { // for now, can only use on monomorphic names - let instance = Instance::mono(self.ccx.shared(), def_id); - let name = instance.symbol_name(self.ccx.shared()); + let instance = Instance::mono(self.scx, def_id); + let name = instance.symbol_name(self.scx); tcx.sess.span_err(attr.span, &format!("symbol-name({})", name)); } else if attr.check_name(ITEM_PATH) { let path = tcx.item_path_str(def_id); From 3a47103f1d2bbf144d3866abc9fe74a9143d2988 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 26 May 2016 13:04:35 -0400 Subject: [PATCH 11/21] Fix codegen tests by make sure items are translated in AST order. --- src/librustc_trans/base.rs | 4 +-- src/librustc_trans/partitioning.rs | 44 +++++++++++++++++++++++++++--- src/test/codegen/drop.rs | 14 +++++----- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 234ed800c47..e13eea6f8ce 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2626,7 +2626,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Instantiate translation items without filling out definitions yet... for ccx in crate_context_list.iter() { let trans_items = ccx.codegen_unit() - .items_in_deterministic_order(&symbol_map); + .items_in_deterministic_order(tcx, &symbol_map); for (trans_item, linkage) in trans_items { trans_item.predefine(&ccx, linkage); @@ -2636,7 +2636,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // ... and now that we have everything pre-defined, fill out those definitions. for ccx in crate_context_list.iter() { let trans_items = ccx.codegen_unit() - .items_in_deterministic_order(&symbol_map); + .items_in_deterministic_order(tcx, &symbol_map); for (trans_item, _) in trans_items { trans_item.define(&ccx); diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 0e06fd6235e..2f1961ac9f8 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -124,7 +124,9 @@ use rustc::hir::map::DefPathData; use rustc::session::config::NUMBERED_CODEGEN_UNIT_MARKER; use rustc::ty::TyCtxt; use rustc::ty::item_path::characteristic_def_id_of_type; +use std::cmp::Ordering; use symbol_map::SymbolMap; +use syntax::ast::NodeId; use syntax::parse::token::{self, InternedString}; use trans_item::TransItem; use util::nodemap::{FnvHashMap, FnvHashSet, NodeSet}; @@ -144,18 +146,52 @@ pub struct CodegenUnit<'tcx> { impl<'tcx> CodegenUnit<'tcx> { pub fn items_in_deterministic_order(&self, + tcx: TyCtxt, symbol_map: &SymbolMap) -> Vec<(TransItem<'tcx>, llvm::Linkage)> { let mut items: Vec<(TransItem<'tcx>, llvm::Linkage)> = self.items.iter().map(|(item, linkage)| (*item, *linkage)).collect(); + // 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 items.as_mut_slice().sort_by(|&(trans_item1, _), &(trans_item2, _)| { - let symbol_name1 = symbol_map.get(trans_item1).unwrap(); - let symbol_name2 = symbol_map.get(trans_item2).unwrap(); - symbol_name1.cmp(symbol_name2) + + let node_id1 = local_node_id(tcx, trans_item1); + let node_id2 = local_node_id(tcx, trans_item2); + + match (node_id1, node_id2) { + (None, None) => { + let symbol_name1 = symbol_map.get(trans_item1).unwrap(); + let symbol_name2 = symbol_map.get(trans_item2).unwrap(); + symbol_name1.cmp(symbol_name2) + } + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some(node_id1), Some(node_id2)) => { + let ordering = node_id1.cmp(&node_id2); + + if ordering != Ordering::Equal { + return ordering; + } + + let symbol_name1 = symbol_map.get(trans_item1).unwrap(); + let symbol_name2 = symbol_map.get(trans_item2).unwrap(); + symbol_name1.cmp(symbol_name2) + } + } }); - items + return items; + + fn local_node_id(tcx: TyCtxt, trans_item: TransItem) -> Option { + match trans_item { + TransItem::Fn(instance) => { + tcx.map.as_local_node_id(instance.def) + } + TransItem::Static(node_id) => Some(node_id), + TransItem::DropGlue(_) => None, + } + } } } diff --git a/src/test/codegen/drop.rs b/src/test/codegen/drop.rs index 83dd6a3b002..25f8c130469 100644 --- a/src/test/codegen/drop.rs +++ b/src/test/codegen/drop.rs @@ -31,13 +31,13 @@ pub fn droppy() { // that's one new drop call per call to possibly_unwinding(), and finally 3 drop calls for the // regular function exit. We used to have problems with quadratic growths of drop calls in such // functions. -// CHECK: call{{.*}}SomeUniqueName{{.*}}drop -// CHECK: call{{.*}}SomeUniqueName{{.*}}drop -// CHECK: call{{.*}}SomeUniqueName{{.*}}drop -// CHECK: call{{.*}}SomeUniqueName{{.*}}drop -// CHECK: call{{.*}}SomeUniqueName{{.*}}drop -// CHECK: call{{.*}}SomeUniqueName{{.*}}drop -// CHECK-NOT: call{{.*}}SomeUniqueName{{.*}}drop +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName +// CHECK-NOT: call{{.*}}drop{{.*}}SomeUniqueName // The next line checks for the } that ends the function definition // CHECK-LABEL: {{^[}]}} let _s = SomeUniqueName; From 00226fc0c8f9d3579db32c6b46fe2a3e4e12cf29 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 26 May 2016 14:21:08 -0400 Subject: [PATCH 12/21] Pacify make tidy. --- src/librustc_trans/back/write.rs | 2 -- src/librustc_trans/collector.rs | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs index 28804e4ed9e..071960f1944 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -812,12 +812,10 @@ pub fn run_passes(sess: &Session, copy_if_one_unit(OutputType::LlvmAssembly, false); } OutputType::Assembly => { - // TODO: These are probably wrong copy_if_one_unit(OutputType::Assembly, false); } OutputType::Object => { user_wants_objects = true; - // TODO: These are probably wrong copy_if_one_unit(OutputType::Object, true); } OutputType::Exe | diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index aff50e5e1f4..048e95f31b4 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -456,8 +456,11 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { match *rvalue { mir::Rvalue::Aggregate(mir::AggregateKind::Closure(def_id, ref substs), _) => { - let mir = errors::expect(self.scx.sess().diagnostic(), self.scx.get_mir(def_id), - || format!("Could not find MIR for closure: {:?}", def_id)); + let mir = errors::expect(self.scx.sess().diagnostic(), + self.scx.get_mir(def_id), + || { + format!("Could not find MIR for closure: {:?}", def_id) + }); let concrete_substs = monomorphize::apply_param_substs(self.scx.tcx(), self.param_substs, From ab80f7467087289931bf5eda07cadc70922ccc94 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 2 Jun 2016 12:28:29 -0400 Subject: [PATCH 13/21] collector-driven-trans: Take care of nits. --- src/librustc_trans/base.rs | 3 ++- src/librustc_trans/callee.rs | 17 ++++++----------- src/librustc_trans/glue.rs | 3 ++- src/librustc_trans/monomorphize.rs | 15 +++++---------- src/librustc_trans/partitioning.rs | 8 ++++---- src/librustc_trans/symbol_map.rs | 17 +++++++++++++++-- src/librustc_trans/trans_item.rs | 2 +- 7 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index e13eea6f8ce..071d9d68dbb 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2667,9 +2667,10 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, { let ccx = crate_context_list.get_ccx(0); + // FIXME: #34018 // At this point, we only walk the HIR for running // enum_variant_size_lint(). This should arguably be moved somewhere - // else + // else. { intravisit::walk_mod(&mut TransItemsWithinModVisitor { ccx: &ccx }, &krate.module); krate.visit_all_items(&mut TransModVisitor { ccx: &ccx }); diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index b7cf43a4371..c4a5a1864f6 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -537,20 +537,15 @@ fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // reference. It also occurs when testing libcore and in some // other weird situations. Annoying. - // Let's see if we can get the symbol name from the symbol_map, so we don't - // have to recompute it. - let mut sym_data = String::new(); - let sym = ccx.symbol_map().get(TransItem::Fn(instance)).unwrap_or_else(|| { - sym_data = instance.symbol_name(ccx.shared()); - &sym_data[..] - }); + let sym = ccx.symbol_map().get_or_compute(ccx.shared(), + TransItem::Fn(instance)); let llptrty = type_of::type_of(ccx, fn_ptr_ty); - let llfn = if let Some(llfn) = declare::get_declared_value(ccx, sym) { + let llfn = if let Some(llfn) = declare::get_declared_value(ccx, &sym) { if let Some(span) = local_item { - if declare::get_defined_value(ccx, sym).is_some() { + if declare::get_defined_value(ccx, &sym).is_some() { ccx.sess().span_fatal(span, - &format!("symbol `{}` is already defined", sym)); + &format!("symbol `{}` is already defined", &sym)); } } @@ -566,7 +561,7 @@ fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, llfn } } else { - let llfn = declare::declare_fn(ccx, sym, ty); + let llfn = declare::declare_fn(ccx, &sym, ty); assert_eq!(common::val_ty(llfn), llptrty); debug!("get_fn: not casting pointer!"); diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 30eb71a9006..3b4a9499a11 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -235,7 +235,8 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let g = g.map_ty(|t| get_drop_glue_type(ccx.tcx(), t)); match ccx.drop_glues().borrow().get(&g) { Some(&(glue, _)) => glue, - None => { bug!("Could not find drop glue for {:?} -- {} -- {}", + None => { bug!("Could not find drop glue for {:?} -- {} -- {}. \ + It should have be instantiated during the pre-definition phase", g, TransItem::DropGlue(g).to_raw_string(), ccx.codegen_unit().name) } diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index 8a2cc53432d..f12eb1cd8e1 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -84,19 +84,14 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, monomorphizing.insert(fn_id, depth + 1); } - // Let's see if we can get the symbol name from the symbol_map, so we don't - // have to recompute it. - let mut sym_data = String::new(); - let symbol = ccx.symbol_map().get(TransItem::Fn(instance)).unwrap_or_else(|| { - sym_data = instance.symbol_name(ccx.shared()); - &sym_data[..] - }); + let symbol = ccx.symbol_map().get_or_compute(ccx.shared(), + TransItem::Fn(instance)); - debug!("monomorphize_fn mangled to {}", symbol); - assert!(declare::get_defined_value(ccx, symbol).is_none()); + debug!("monomorphize_fn mangled to {}", &symbol); + assert!(declare::get_defined_value(ccx, &symbol).is_none()); // FIXME(nagisa): perhaps needs a more fine grained selection? - let lldecl = declare::define_internal_fn(ccx, symbol, mono_ty); + let lldecl = declare::define_internal_fn(ccx, &symbol, mono_ty); // FIXME(eddyb) Doubt all extern fn should allow unwinding. attributes::unwind(lldecl, true); diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 2f1961ac9f8..785c193c855 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -154,8 +154,7 @@ impl<'tcx> CodegenUnit<'tcx> { // 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 - items.as_mut_slice().sort_by(|&(trans_item1, _), &(trans_item2, _)| { - + items.sort_by(|&(trans_item1, _), &(trans_item2, _)| { let node_id1 = local_node_id(tcx, trans_item1); let node_id2 = local_node_id(tcx, trans_item2); @@ -165,6 +164,7 @@ impl<'tcx> CodegenUnit<'tcx> { let symbol_name2 = symbol_map.get(trans_item2).unwrap(); symbol_name1.cmp(symbol_name2) } + // In the following two cases we can avoid looking up the symbol (None, Some(_)) => Ordering::Less, (Some(_), None) => Ordering::Greater, (Some(node_id1), Some(node_id2)) => { @@ -241,7 +241,7 @@ pub fn partition<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Finally, sort by codegen unit name, so that we get deterministic results let mut result = post_inlining.0; - result.as_mut_slice().sort_by(|cgu1, cgu2| { + result.sort_by(|cgu1, cgu2| { (&cgu1.name[..]).cmp(&cgu2.name[..]) }); @@ -348,7 +348,7 @@ fn merge_codegen_units<'tcx>(initial_partitioning: &mut PreInliningPartitioning< // translation items in a given unit. This could be improved on. while codegen_units.len() > target_cgu_count { // Sort small cgus to the back - codegen_units.as_mut_slice().sort_by_key(|cgu| -(cgu.items.len() as i64)); + codegen_units.sort_by_key(|cgu| -(cgu.items.len() as i64)); let smallest = codegen_units.pop().unwrap(); let second_smallest = codegen_units.last_mut().unwrap(); diff --git a/src/librustc_trans/symbol_map.rs b/src/librustc_trans/symbol_map.rs index 4f82b54c76b..3faaa085dce 100644 --- a/src/librustc_trans/symbol_map.rs +++ b/src/librustc_trans/symbol_map.rs @@ -11,13 +11,15 @@ use context::SharedCrateContext; use monomorphize::Instance; use rustc::ty::TyCtxt; +use std::borrow::Cow; use syntax::codemap::Span; use trans_item::TransItem; use util::nodemap::FnvHashMap; - // In the SymbolMap we collect the symbol names of all translation items of -// the current crate. +// the current crate. This map exists as a performance optimization. Symbol +// names of translation items are deterministic and fully defined by the item. +// Thus they could also always be recomputed if needed. pub struct SymbolMap<'tcx> { index: FnvHashMap, (usize, usize)>, @@ -112,4 +114,15 @@ impl<'tcx> SymbolMap<'tcx> { &self.arena[start_index .. end_index] }) } + + pub fn get_or_compute<'map, 'scx>(&'map self, + scx: &SharedCrateContext<'scx, 'tcx>, + trans_item: TransItem<'tcx>) + -> Cow<'map, str> { + if let Some(sym) = self.get(trans_item) { + Cow::from(sym) + } else { + Cow::from(trans_item.compute_symbol_name(scx)) + } + } } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 4fa0ba03005..b4f8a116662 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -222,7 +222,7 @@ impl<'a, 'tcx> TransItem<'tcx> { assert!(declare::get_defined_value(ccx, symbol_name).is_none()); let llfn = declare::declare_cfn(ccx, symbol_name, llfnty); - llvm::SetLinkage(llfn, linkage); + llvm::SetLinkage(llfn, linkage); attributes::set_frame_pointer_elimination(ccx, llfn); ccx.drop_glues().borrow_mut().insert(dg, (llfn, fn_ty)); } From b33240e2cc08cc62511e6c996e68c87ad8bd4951 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 2 Jun 2016 17:25:03 -0400 Subject: [PATCH 14/21] trans::collector: Also consider initializers of const items. --- src/librustc_trans/collector.rs | 61 ++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 048e95f31b4..f37c117cb0f 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -205,6 +205,7 @@ use rustc::mir::visit::Visitor as MirVisitor; use syntax::abi::Abi; use errors; use syntax_pos::DUMMY_SP; +use syntax::ast::NodeId; use base::custom_coerce_unsize_info; use context::SharedCrateContext; use common::{fulfill_obligation, normalize_and_test_predicates, type_is_sized}; @@ -349,17 +350,14 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, || format!("Could not find MIR for static: {:?}", def_id)); let empty_substs = scx.empty_substs_for_def_id(def_id); - let mut visitor = MirNeighborCollector { + let visitor = MirNeighborCollector { scx: scx, mir: &mir, output: &mut neighbors, param_substs: empty_substs }; - visitor.visit_mir(&mir); - for promoted in &mir.promoted { - visitor.visit_mir(promoted); - } + visit_mir_and_promoted(visitor, &mir); } TransItem::Fn(instance) => { // Keep track of the monomorphization recursion depth @@ -372,17 +370,14 @@ fn collect_items_rec<'a, 'tcx: 'a>(scx: &SharedCrateContext<'a, 'tcx>, let mir = errors::expect(scx.sess().diagnostic(), scx.get_mir(instance.def), || format!("Could not find MIR for function: {}", instance)); - let mut visitor = MirNeighborCollector { + let visitor = MirNeighborCollector { scx: scx, mir: &mir, output: &mut neighbors, param_substs: instance.substs }; - visitor.visit_mir(&mir); - for promoted in &mir.promoted { - visitor.visit_mir(promoted); - } + visit_mir_and_promoted(visitor, &mir); } } @@ -467,17 +462,14 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { &substs.func_substs); let concrete_substs = self.scx.tcx().erase_regions(&concrete_substs); - let mut visitor = MirNeighborCollector { + let visitor = MirNeighborCollector { scx: self.scx, mir: &mir, output: self.output, param_substs: concrete_substs }; - visitor.visit_mir(&mir); - for promoted in &mir.promoted { - visitor.visit_mir(promoted); - } + visit_mir_and_promoted(visitor, &mir); } // When doing an cast from a regular pointer to a fat pointer, we // have to instantiate all methods of the trait being cast to, so we @@ -1087,7 +1079,6 @@ impl<'b, 'a, 'v> hir_visit::Visitor<'v> for RootCollector<'b, 'a, 'v> { hir::ItemTy(..) | hir::ItemDefaultImpl(..) | hir::ItemTrait(..) | - hir::ItemConst(..) | hir::ItemMod(..) => { // Nothing to do, just keep recursing... } @@ -1124,6 +1115,12 @@ impl<'b, 'a, 'v> hir_visit::Visitor<'v> for RootCollector<'b, 'a, 'v> { self.scx.tcx().map.local_def_id(item.id))); self.output.push(TransItem::Static(item.id)); } + hir::ItemConst(..) => { + debug!("RootCollector: ItemConst({})", + def_id_to_string(self.scx.tcx(), + self.scx.tcx().map.local_def_id(item.id))); + add_roots_for_const_item(self.scx, item.id, self.output); + } hir::ItemFn(_, _, _, _, ref generics, _) => { if !generics.is_type_parameterized() { let def_id = self.scx.tcx().map.local_def_id(item.id); @@ -1243,6 +1240,38 @@ fn create_trans_items_for_default_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } +// There are no translation items for constants themselves but their +// initializers might still contain something that produces translation items, +// such as cast that introduce a new vtable. +fn add_roots_for_const_item<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, + const_item_node_id: NodeId, + output: &mut Vec>) +{ + let def_id = scx.tcx().map.local_def_id(const_item_node_id); + + // Scan the MIR in order to find function calls, closures, and + // drop-glue + let mir = errors::expect(scx.sess().diagnostic(), scx.get_mir(def_id), + || format!("Could not find MIR for const: {:?}", def_id)); + + let empty_substs = scx.empty_substs_for_def_id(def_id); + let visitor = MirNeighborCollector { + scx: scx, + mir: &mir, + output: output, + param_substs: empty_substs + }; + + visit_mir_and_promoted(visitor, &mir); +} + +fn visit_mir_and_promoted<'tcx, V: MirVisitor<'tcx>>(mut visitor: V, mir: &mir::Mir<'tcx>) { + visitor.visit_mir(&mir); + for promoted in &mir.promoted { + visitor.visit_mir(promoted); + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum TransItemState { PredictedAndGenerated, From 4a3f9b8962fb3946f155a39c745297e8e3126b05 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 3 Jun 2016 13:24:46 -0400 Subject: [PATCH 15/21] hir-trans: Don't generate code for unreachable operands in short-circuiting logical operations. --- src/librustc_trans/expr.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/librustc_trans/expr.rs b/src/librustc_trans/expr.rs index 71c6cba9cc2..b8dd7273a83 100644 --- a/src/librustc_trans/expr.rs +++ b/src/librustc_trans/expr.rs @@ -1695,11 +1695,13 @@ fn trans_scalar_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, } // refinement types would obviate the need for this +#[derive(Clone, Copy)] enum lazy_binop_ty { lazy_and, lazy_or, } + fn trans_lazy_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, binop_expr: &hir::Expr, op: lazy_binop_ty, @@ -1717,6 +1719,17 @@ fn trans_lazy_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, return immediate_rvalue_bcx(past_lhs, lhs, binop_ty).to_expr_datumblock(); } + // If the rhs can never be reached, don't generate code for it. + if let Some(cond_val) = const_to_opt_uint(lhs) { + match (cond_val, op) { + (0, lazy_and) | + (1, lazy_or) => { + return immediate_rvalue_bcx(past_lhs, lhs, binop_ty).to_expr_datumblock(); + } + _ => { /* continue */ } + } + } + let join = fcx.new_id_block("join", binop_expr.id); let before_rhs = fcx.new_id_block("before_rhs", b.id); From a7bc0b920f27f943bb841f3ace093a5449394497 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 7 Jun 2016 21:12:40 -0400 Subject: [PATCH 16/21] trans: Add missing normalize_associated_type() call to callee::get_fn(). --- src/librustc_trans/callee.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_trans/callee.rs b/src/librustc_trans/callee.rs index c4a5a1864f6..983ee564c35 100644 --- a/src/librustc_trans/callee.rs +++ b/src/librustc_trans/callee.rs @@ -303,7 +303,7 @@ pub fn trans_fn_pointer_shim<'a, 'tcx>( let tcx = ccx.tcx(); // Normalize the type for better caching. - let bare_fn_ty = tcx.erase_regions(&bare_fn_ty); + let bare_fn_ty = tcx.normalize_associated_type(&bare_fn_ty); // If this is an impl of `Fn` or `FnMut` trait, the receiver is `&self`. let is_by_ref = match closure_kind { @@ -469,7 +469,7 @@ fn get_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Should be either intra-crate or inlined. assert_eq!(def_id.krate, LOCAL_CRATE); - let substs = tcx.mk_substs(substs.clone().erase_regions()); + let substs = tcx.normalize_associated_type(&substs); let (val, fn_ty) = monomorphize::monomorphic_fn(ccx, def_id, substs); let fn_ptr_ty = match fn_ty.sty { ty::TyFnDef(_, _, fty) => { From 4c27a3c6d597ce5ecd1b71e99e48075e625e7b27 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 7 Jun 2016 21:14:51 -0400 Subject: [PATCH 17/21] trans: Enable falling back to on-demand instantiation for drop-glue and monomorphizations. See issue #34151 for more information. --- src/librustc_trans/base.rs | 16 +++++++++------ src/librustc_trans/context.rs | 2 ++ src/librustc_trans/glue.rs | 33 ++++++++++++++++++++++++------ src/librustc_trans/monomorphize.rs | 21 +++++++++++++++++-- src/librustc_trans/partitioning.rs | 4 ++-- src/librustc_trans/trans_item.rs | 12 +++++------ 6 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 071d9d68dbb..bcca008c5e9 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2684,6 +2684,8 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, println!("n_null_glues: {}", stats.n_null_glues.get()); println!("n_real_glues: {}", stats.n_real_glues.get()); + println!("n_fallback_instantiations: {}", stats.n_fallback_instantiations.get()); + println!("n_fns: {}", stats.n_fns.get()); println!("n_monos: {}", stats.n_monos.get()); println!("n_inlines: {}", stats.n_inlines.get()); @@ -2875,6 +2877,14 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a assert!(scx.tcx().sess.opts.cg.codegen_units == codegen_units.len() || scx.tcx().sess.opts.debugging_opts.incremental.is_some()); + { + let mut ccx_map = scx.translation_items().borrow_mut(); + + for trans_item in items.iter().cloned() { + ccx_map.insert(trans_item, TransItemState::PredictedButNotGenerated); + } + } + if scx.sess().opts.debugging_opts.print_trans_items.is_some() { let mut item_to_cgus = HashMap::new(); @@ -2926,12 +2936,6 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a for item in item_keys { println!("TRANS_ITEM {}", item); } - - let mut ccx_map = scx.translation_items().borrow_mut(); - - for cgi in items { - ccx_map.insert(cgi, TransItemState::PredictedButNotGenerated); - } } (codegen_units, symbol_map) diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 64e0351610f..8f700e2fe38 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -53,6 +53,7 @@ pub struct Stats { pub n_glues_created: Cell, pub n_null_glues: Cell, pub n_real_glues: Cell, + pub n_fallback_instantiations: Cell, pub n_fns: Cell, pub n_monos: Cell, pub n_inlines: Cell, @@ -406,6 +407,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { n_glues_created: Cell::new(0), n_null_glues: Cell::new(0), n_real_glues: Cell::new(0), + n_fallback_instantiations: Cell::new(0), n_fns: Cell::new(0), n_monos: Cell::new(0), n_inlines: Cell::new(0), diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 3b4a9499a11..468192d7f11 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -234,13 +234,34 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, g: DropGlueKind<'tcx>) -> ValueRef { let g = g.map_ty(|t| get_drop_glue_type(ccx.tcx(), t)); match ccx.drop_glues().borrow().get(&g) { - Some(&(glue, _)) => glue, - None => { bug!("Could not find drop glue for {:?} -- {} -- {}. \ - It should have be instantiated during the pre-definition phase", - g, - TransItem::DropGlue(g).to_raw_string(), - ccx.codegen_unit().name) } + Some(&(glue, _)) => return glue, + None => { + debug!("Could not find drop glue for {:?} -- {} -- {}. \ + Falling back to on-demand instantiation.", + g, + TransItem::DropGlue(g).to_raw_string(), + ccx.codegen_unit().name); + + ccx.stats().n_fallback_instantiations.set(ccx.stats() + .n_fallback_instantiations + .get() + 1); + } } + + // FIXME: #34151 + // Normally, getting here would indicate a bug in trans::collector, + // since it seems to have missed a translation item. When we are + // translating with non-MIR-based trans, however, the results of the + // collector are not entirely reliable since it bases its analysis + // on MIR. Thus, we'll instantiate the missing function on demand in + // this codegen unit, so that things keep working. + + TransItem::DropGlue(g).predefine(ccx, llvm::LinkOnceODRLinkage); + TransItem::DropGlue(g).define(ccx); + + // Now that we made sure that the glue function is in ccx.drop_glues, + // give it another try + get_drop_glue_core(ccx, g) } pub fn implement_drop_glue<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index f12eb1cd8e1..71aacfdfe58 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -121,8 +121,25 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ref attrs, node: hir::MethodTraitItem( hir::MethodSig { .. }, Some(_)), .. }) => { - attributes::from_fn_attrs(ccx, attrs, lldecl); - llvm::SetLinkage(lldecl, llvm::ExternalLinkage); + let trans_item = TransItem::Fn(instance); + + if ccx.shared().translation_items().borrow().contains_key(&trans_item) { + attributes::from_fn_attrs(ccx, attrs, lldecl); + llvm::SetLinkage(lldecl, llvm::ExternalLinkage); + } else { + // FIXME: #34151 + // Normally, getting here would indicate a bug in trans::collector, + // since it seems to have missed a translation item. When we are + // translating with non-MIR based trans, however, the results of + // the collector are not entirely reliable since it bases its + // analysis on MIR. Thus, we'll instantiate the missing function + // privately in this codegen unit, so that things keep working. + ccx.stats().n_fallback_instantiations.set(ccx.stats() + .n_fallback_instantiations + .get() + 1); + trans_item.predefine(ccx, llvm::PrivateLinkage); + trans_item.define(ccx); + } } hir_map::NodeVariant(_) | hir_map::NodeStructCtor(_) => { diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 785c193c855..edf5db81b18 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -517,11 +517,11 @@ fn single_codegen_unit<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, if reachable.contains(&node_id) { llvm::ExternalLinkage } else { - llvm::InternalLinkage + llvm::PrivateLinkage } } TransItem::DropGlue(_) => { - llvm::InternalLinkage + llvm::PrivateLinkage } TransItem::Fn(instance) => { if trans_item.is_generic_fn() || diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index b4f8a116662..2fc90b821fe 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -108,19 +108,19 @@ impl<'a, 'tcx> TransItem<'tcx> { ccx.codegen_unit().name); let symbol_name = ccx.symbol_map() - .get(*self) - .expect("Name not present in SymbolMap?"); - debug!("symbol {}", symbol_name); + .get_or_compute(ccx.shared(), *self); + + debug!("symbol {}", &symbol_name); match *self { TransItem::Static(node_id) => { - TransItem::predefine_static(ccx, node_id, linkage, symbol_name); + TransItem::predefine_static(ccx, node_id, linkage, &symbol_name); } TransItem::Fn(instance) => { - TransItem::predefine_fn(ccx, instance, linkage, symbol_name); + TransItem::predefine_fn(ccx, instance, linkage, &symbol_name); } TransItem::DropGlue(dg) => { - TransItem::predefine_drop_glue(ccx, dg, linkage, symbol_name); + TransItem::predefine_drop_glue(ccx, dg, linkage, &symbol_name); } } From b149b9d19bb4dfbda4145b2667d87168da1e57d6 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 9 Jun 2016 13:39:21 -0400 Subject: [PATCH 18/21] trans: Set COMDAT section for weak symbols so that Windows can handle them. --- src/librustc_trans/closure.rs | 1 + src/librustc_trans/trans_item.rs | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustc_trans/closure.rs b/src/librustc_trans/closure.rs index bf9cde9a441..b992ba362a9 100644 --- a/src/librustc_trans/closure.rs +++ b/src/librustc_trans/closure.rs @@ -212,6 +212,7 @@ pub fn trans_closure_expr<'a, 'tcx>(dest: Dest<'a, 'tcx>, let llfn = get_or_create_closure_declaration(ccx, closure_def_id, closure_substs); llvm::SetLinkage(llfn, llvm::WeakODRLinkage); + llvm::SetUniqueComdat(ccx.llmod(), llfn); // Get the type of this closure. Use the current `param_substs` as // the closure substitutions. This makes sense because the closure diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 2fc90b821fe..b9700f2cacf 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -190,9 +190,13 @@ impl<'a, 'tcx> TransItem<'tcx> { }) => { let lldecl = declare::declare_fn(ccx, symbol_name, mono_ty); llvm::SetLinkage(lldecl, linkage); - attributes::from_fn_attrs(ccx, attrs, lldecl); base::set_link_section(ccx, lldecl, attrs); + if linkage == llvm::LinkOnceODRLinkage || + linkage == llvm::WeakODRLinkage { + llvm::SetUniqueComdat(ccx.llmod(), lldecl); + } + attributes::from_fn_attrs(ccx, attrs, lldecl); ccx.instances().borrow_mut().insert(instance, lldecl); } _ => bug!("Invalid item for TransItem::Fn: `{:?}`", map_node) @@ -223,6 +227,10 @@ impl<'a, 'tcx> TransItem<'tcx> { assert!(declare::get_defined_value(ccx, symbol_name).is_none()); let llfn = declare::declare_cfn(ccx, symbol_name, llfnty); llvm::SetLinkage(llfn, linkage); + if linkage == llvm::LinkOnceODRLinkage || + linkage == llvm::WeakODRLinkage { + llvm::SetUniqueComdat(ccx.llmod(), llfn); + } attributes::set_frame_pointer_elimination(ccx, llfn); ccx.drop_glues().borrow_mut().insert(dg, (llfn, fn_ty)); } From ac80d4117562aa070717fb2ebbbc069be712d906 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 16 Jun 2016 18:56:14 -0400 Subject: [PATCH 19/21] trans: Remove tracking of translation item state. The data tracked here was meant to compare the output of the translation item collector to the set of translation items found by the on-demand translator. --- src/librustc_trans/base.rs | 9 +-- src/librustc_trans/collector.rs | 109 ----------------------------- src/librustc_trans/consts.rs | 6 -- src/librustc_trans/context.rs | 25 ++----- src/librustc_trans/glue.rs | 6 -- src/librustc_trans/monomorphize.rs | 2 +- 6 files changed, 8 insertions(+), 149 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index bcca008c5e9..c080d1f06d0 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -58,7 +58,7 @@ use callee::{Callee, CallArgs, ArgExprs, ArgVals}; use cleanup::{self, CleanupMethods, DropHint}; use closure; use common::{Block, C_bool, C_bytes_in_context, C_i32, C_int, C_uint, C_integral}; -use collector::{self, TransItemState, TransItemCollectionMode}; +use collector::{self, TransItemCollectionMode}; use common::{C_null, C_struct_in_context, C_u64, C_u8, C_undef}; use common::{CrateContext, DropFlagHintsMap, Field, FunctionContext}; use common::{Result, NodeIdAndSpan, VariantInfo}; @@ -1830,10 +1830,6 @@ pub fn trans_closure<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, closure_env: closure::ClosureEnv) { ccx.stats().n_closures.set(ccx.stats().n_closures.get() + 1); - if collector::collecting_debug_information(ccx.shared()) { - ccx.record_translation_item_as_generated(TransItem::Fn(instance)); - } - let _icx = push_ctxt("trans_closure"); if !ccx.sess().no_landing_pads() { attributes::emit_uwtable(llfndecl, true); @@ -2661,7 +2657,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } - collector::print_collection_results(&shared_ccx); symbol_names_test::report_symbol_names(&shared_ccx); { @@ -2881,7 +2876,7 @@ fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a let mut ccx_map = scx.translation_items().borrow_mut(); for trans_item in items.iter().cloned() { - ccx_map.insert(trans_item, TransItemState::PredictedButNotGenerated); + ccx_map.insert(trans_item); } } diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index f37c117cb0f..ba2cd2ba699 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -1271,112 +1271,3 @@ fn visit_mir_and_promoted<'tcx, V: MirVisitor<'tcx>>(mut visitor: V, mir: &mir:: visitor.visit_mir(promoted); } } - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum TransItemState { - PredictedAndGenerated, - PredictedButNotGenerated, - NotPredictedButGenerated, -} - -pub fn collecting_debug_information(scx: &SharedCrateContext) -> bool { - return cfg!(debug_assertions) && - scx.sess().opts.debugging_opts.print_trans_items.is_some(); -} - -pub fn print_collection_results<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>) { - use std::hash::{Hash, SipHasher, Hasher}; - - if !collecting_debug_information(scx) { - return; - } - - fn hash(t: &T) -> u64 { - let mut s = SipHasher::new(); - t.hash(&mut s); - s.finish() - } - - let trans_items = scx.translation_items().borrow(); - - { - // Check for duplicate item keys - let mut item_keys = FnvHashMap(); - - for (item, item_state) in trans_items.iter() { - let k = item.to_string(scx.tcx()); - - if item_keys.contains_key(&k) { - let prev: (TransItem, TransItemState) = item_keys[&k]; - debug!("DUPLICATE KEY: {}", k); - debug!(" (1) {:?}, {:?}, hash: {}, raw: {}", - prev.0, - prev.1, - hash(&prev.0), - prev.0.to_raw_string()); - - debug!(" (2) {:?}, {:?}, hash: {}, raw: {}", - *item, - *item_state, - hash(item), - item.to_raw_string()); - } else { - item_keys.insert(k, (*item, *item_state)); - } - } - } - - let mut predicted_but_not_generated = FnvHashSet(); - let mut not_predicted_but_generated = FnvHashSet(); - let mut predicted = FnvHashSet(); - let mut generated = FnvHashSet(); - - for (item, item_state) in trans_items.iter() { - let item_key = item.to_string(scx.tcx()); - - match *item_state { - TransItemState::PredictedAndGenerated => { - predicted.insert(item_key.clone()); - generated.insert(item_key); - } - TransItemState::PredictedButNotGenerated => { - predicted_but_not_generated.insert(item_key.clone()); - predicted.insert(item_key); - } - TransItemState::NotPredictedButGenerated => { - not_predicted_but_generated.insert(item_key.clone()); - generated.insert(item_key); - } - } - } - - debug!("Total number of translation items predicted: {}", predicted.len()); - debug!("Total number of translation items generated: {}", generated.len()); - debug!("Total number of translation items predicted but not generated: {}", - predicted_but_not_generated.len()); - debug!("Total number of translation items not predicted but generated: {}", - not_predicted_but_generated.len()); - - if generated.len() > 0 { - debug!("Failed to predict {}% of translation items", - (100 * not_predicted_but_generated.len()) / generated.len()); - } - if generated.len() > 0 { - debug!("Predict {}% too many translation items", - (100 * predicted_but_not_generated.len()) / generated.len()); - } - - debug!(""); - debug!("Not predicted but generated:"); - debug!("============================"); - for item in not_predicted_but_generated { - debug!(" - {}", item); - } - - debug!(""); - debug!("Predicted but not generated:"); - debug!("============================"); - for item in predicted_but_not_generated { - debug!(" - {}", item); - } -} diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index 3f18c61fbd2..5732fded362 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -21,7 +21,6 @@ use rustc::hir::map as hir_map; use {abi, adt, closure, debuginfo, expr, machine}; use base::{self, push_ctxt}; use callee::Callee; -use collector; use trans_item::TransItem; use common::{type_is_sized, C_nil, const_get_elt}; use common::{CrateContext, C_integral, C_floating, C_bool, C_str_slice, C_bytes, val_ty}; @@ -1140,11 +1139,6 @@ pub fn trans_static(ccx: &CrateContext, id: ast::NodeId, attrs: &[ast::Attribute]) -> Result { - - if collector::collecting_debug_information(ccx.shared()) { - ccx.record_translation_item_as_generated(TransItem::Static(id)); - } - unsafe { let _icx = push_ctxt("trans_static"); let def_id = ccx.tcx().map.local_def_id(id); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 8f700e2fe38..b8d231db40a 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -28,7 +28,6 @@ use mir::CachedMir; use monomorphize::Instance; use partitioning::CodegenUnit; -use collector::TransItemState; use trans_item::TransItem; use type_::{Type, TypeNames}; use rustc::ty::subst::{Substs, VecPerParamSpace}; @@ -37,7 +36,7 @@ use session::config::NoDebugInfo; use session::Session; use symbol_map::SymbolMap; use util::sha2::Sha256; -use util::nodemap::{NodeMap, NodeSet, DefIdMap, FnvHashMap}; +use util::nodemap::{NodeMap, NodeSet, DefIdMap, FnvHashMap, FnvHashSet}; use std::ffi::{CStr, CString}; use std::cell::{Cell, RefCell}; @@ -85,7 +84,7 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { use_dll_storage_attrs: bool, - translation_items: RefCell, TransItemState>>, + translation_items: RefCell>>, trait_cache: RefCell>>, } @@ -419,7 +418,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { check_overflow: check_overflow, check_drop_flag_for_sanity: check_drop_flag_for_sanity, use_dll_storage_attrs: use_dll_storage_attrs, - translation_items: RefCell::new(FnvHashMap()), + translation_items: RefCell::new(FnvHashSet()), trait_cache: RefCell::new(DepTrackingMap::new(tcx.dep_graph.clone())), } } @@ -482,7 +481,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { } } - pub fn translation_items(&self) -> &RefCell, TransItemState>> { + pub fn translation_items(&self) -> &RefCell>> { &self.translation_items } @@ -902,24 +901,10 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { &*self.local().symbol_map } - pub fn translation_items(&self) -> &RefCell, TransItemState>> { + pub fn translation_items(&self) -> &RefCell>> { &self.shared.translation_items } - pub fn record_translation_item_as_generated(&self, cgi: TransItem<'tcx>) { - if self.sess().opts.debugging_opts.print_trans_items.is_none() { - return; - } - - let mut codegen_items = self.translation_items().borrow_mut(); - - if codegen_items.contains_key(&cgi) { - codegen_items.insert(cgi, TransItemState::PredictedAndGenerated); - } else { - codegen_items.insert(cgi, TransItemState::NotPredictedButGenerated); - } - } - /// Given the def-id of some item that has no type parameters, make /// a suitable "empty substs" for it. pub fn empty_substs_for_def_id(&self, item_def_id: DefId) -> &'tcx Substs<'tcx> { diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 468192d7f11..87a1d584433 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -27,7 +27,6 @@ use build::*; use callee::{Callee, ArgVals}; use cleanup; use cleanup::CleanupMethods; -use collector; use common::*; use debuginfo::DebugLoc; use expr; @@ -482,11 +481,6 @@ pub fn size_and_align_of_dst<'blk, 'tcx>(bcx: &BlockAndBuilder<'blk, 'tcx>, fn make_drop_glue<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, v0: ValueRef, g: DropGlueKind<'tcx>) -> Block<'blk, 'tcx> { - if collector::collecting_debug_information(bcx.ccx().shared()) { - bcx.ccx() - .record_translation_item_as_generated(TransItem::DropGlue(g)); - } - let t = g.ty(); let skip_dtor = match g { DropGlueKind::Ty(_) => false, DropGlueKind::TyContents(_) => true }; diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index 71aacfdfe58..a97e3e5a78d 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -123,7 +123,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, }) => { let trans_item = TransItem::Fn(instance); - if ccx.shared().translation_items().borrow().contains_key(&trans_item) { + if ccx.shared().translation_items().borrow().contains(&trans_item) { attributes::from_fn_attrs(ccx, attrs, lldecl); llvm::SetLinkage(lldecl, llvm::ExternalLinkage); } else { From 051d391f2d45c8e7e59bf1bff6cab3b276b76335 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 22 Jun 2016 11:59:34 -0400 Subject: [PATCH 20/21] Update LLVM. --- src/llvm | 2 +- src/rustllvm/llvm-auto-clean-trigger | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/llvm b/src/llvm index 80ad955b60b..7ca76af03bb 160000 --- a/src/llvm +++ b/src/llvm @@ -1 +1 @@ -Subproject commit 80ad955b60b3ac02d0462a4a65fcea597d0ebfb1 +Subproject commit 7ca76af03bb04659562890d6b4f223fffe0d748f diff --git a/src/rustllvm/llvm-auto-clean-trigger b/src/rustllvm/llvm-auto-clean-trigger index 4017c3856c4..1953fc5a6b4 100644 --- a/src/rustllvm/llvm-auto-clean-trigger +++ b/src/rustllvm/llvm-auto-clean-trigger @@ -1,4 +1,4 @@ # If this file is modified, then llvm will be forcibly cleaned and then rebuilt. # The actual contents of this file do not matter, but to trigger a change on the # build bots then the contents should be changed so git updates the mtime. -2016-04-28 +2016-06-23 From 1c03bfe3b43c06bc439c5369a180958eb4360361 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 10 Jun 2016 19:06:21 -0400 Subject: [PATCH 21/21] trans: Adjust linkage assignment so that we don't need weak linkage. --- src/librustc_trans/glue.rs | 2 +- src/librustc_trans/monomorphize.rs | 2 +- src/librustc_trans/partitioning.rs | 88 ++++++++++++------- src/librustc_trans/trans_item.rs | 11 ++- .../partitioning/extern-drop-glue.rs | 8 +- .../partitioning/extern-generic.rs | 4 +- .../inlining-from-extern-crate.rs | 4 +- .../partitioning/local-drop-glue.rs | 10 +-- .../partitioning/local-generic.rs | 11 +-- .../methods-are-with-self-type.rs | 5 ++ 10 files changed, 85 insertions(+), 60 deletions(-) diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index 87a1d584433..ef7d0ea165d 100644 --- a/src/librustc_trans/glue.rs +++ b/src/librustc_trans/glue.rs @@ -255,7 +255,7 @@ fn get_drop_glue_core<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // on MIR. Thus, we'll instantiate the missing function on demand in // this codegen unit, so that things keep working. - TransItem::DropGlue(g).predefine(ccx, llvm::LinkOnceODRLinkage); + TransItem::DropGlue(g).predefine(ccx, llvm::InternalLinkage); TransItem::DropGlue(g).define(ccx); // Now that we made sure that the glue function is in ccx.drop_glues, diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index a97e3e5a78d..00c0e911035 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -137,7 +137,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ccx.stats().n_fallback_instantiations.set(ccx.stats() .n_fallback_instantiations .get() + 1); - trans_item.predefine(ccx, llvm::PrivateLinkage); + trans_item.predefine(ccx, llvm::InternalLinkage); trans_item.define(ccx); } } diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index edf5db81b18..8073359ede8 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -265,15 +265,11 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let mut codegen_units = FnvHashMap(); for trans_item in trans_items { - let is_root = match trans_item { - TransItem::Static(..) => true, - TransItem::DropGlue(..) => false, - TransItem::Fn(_) => !trans_item.is_from_extern_crate(), - }; + let is_root = !trans_item.is_instantiated_only_on_demand(); if is_root { let characteristic_def_id = characteristic_def_id_of_trans_item(tcx, trans_item); - let is_volatile = trans_item.is_lazily_instantiated(); + let is_volatile = trans_item.is_generic_fn(); let codegen_unit_name = match characteristic_def_id { Some(def_id) => compute_codegen_unit_name(tcx, def_id, is_volatile), @@ -304,9 +300,9 @@ fn place_root_translation_items<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // it might be used in another codegen unit. llvm::ExternalLinkage } else { - // Monomorphizations of generic functions are - // always weak-odr - llvm::WeakODRLinkage + // In the current setup, generic functions cannot + // be roots. + unreachable!() } } } @@ -395,25 +391,30 @@ fn place_inlined_translation_items<'tcx>(initial_partitioning: PreInliningPartit if let Some(linkage) = codegen_unit.items.get(&trans_item) { // This is a root, just copy it over new_codegen_unit.items.insert(trans_item, *linkage); + } else if initial_partitioning.roots.contains(&trans_item) { + // This item will be instantiated in some other codegen unit, + // so we just add it here with AvailableExternallyLinkage + // FIXME(mw): I have not seen it happening yet but having + // available_externally here could potentially lead + // to the same problem with exception handling tables + // as in the case below. + new_codegen_unit.items.insert(trans_item, + llvm::AvailableExternallyLinkage); + } else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() { + // FIXME(mw): It would be nice if we could mark these as + // `AvailableExternallyLinkage`, since they should have + // been instantiated in the extern crate. But this + // sometimes leads to crashes on Windows because LLVM + // does not handle exception handling table instantiation + // reliably in that case. + new_codegen_unit.items.insert(trans_item, llvm::InternalLinkage); } else { - if initial_partitioning.roots.contains(&trans_item) { - // This item will be instantiated in some other codegen unit, - // so we just add it here with AvailableExternallyLinkage - new_codegen_unit.items.insert(trans_item, - llvm::AvailableExternallyLinkage); - } else if trans_item.is_from_extern_crate() && !trans_item.is_generic_fn() { - // An instantiation of this item is always available in the - // crate it was imported from. - new_codegen_unit.items.insert(trans_item, - llvm::AvailableExternallyLinkage); - } else { - // We can't be sure if this will also be instantiated - // somewhere else, so we add an instance here with - // LinkOnceODRLinkage. That way the item can be discarded if - // it's not needed (inlined) after all. - new_codegen_unit.items.insert(trans_item, - llvm::LinkOnceODRLinkage); - } + assert!(trans_item.is_instantiated_only_on_demand()); + // We can't be sure if this will also be instantiated + // somewhere else, so we add an instance here with + // InternalLinkage so we don't get any conflicts. + new_codegen_unit.items.insert(trans_item, + llvm::InternalLinkage); } } @@ -521,17 +522,36 @@ fn single_codegen_unit<'a, 'tcx, I>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } TransItem::DropGlue(_) => { - llvm::PrivateLinkage + llvm::InternalLinkage } TransItem::Fn(instance) => { - if trans_item.is_generic_fn() || - trans_item.is_from_extern_crate() || - !reachable.contains(&tcx.map - .as_local_node_id(instance.def) - .unwrap()) { + if trans_item.is_generic_fn() { + // FIXME(mw): Assigning internal linkage to all + // monomorphizations is potentially a waste of space + // since monomorphizations could be shared between + // crates. The main reason for making them internal is + // a limitation in MingW's binutils that cannot deal + // with COFF object that have more than 2^15 sections, + // which is something that can happen for large programs + // when every function gets put into its own COMDAT + // section. llvm::InternalLinkage - } else { + } else if trans_item.is_from_extern_crate() { + // FIXME(mw): It would be nice if we could mark these as + // `AvailableExternallyLinkage`, since they should have + // been instantiated in the extern crate. But this + // sometimes leads to crashes on Windows because LLVM + // does not handle exception handling table instantiation + // reliably in that case. + llvm::InternalLinkage + } else if reachable.contains(&tcx.map + .as_local_node_id(instance.def) + .unwrap()) { llvm::ExternalLinkage + } else { + // Functions that are not visible outside this crate can + // be marked as internal. + llvm::InternalLinkage } } } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index b9700f2cacf..b7b18b2631b 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -256,8 +256,10 @@ impl<'a, 'tcx> TransItem<'tcx> { pub fn requests_inline(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { match *self { TransItem::Fn(ref instance) => { - let attributes = tcx.get_attrs(instance.def); - attr::requests_inline(&attributes[..]) + !instance.substs.types.is_empty() || { + let attributes = tcx.get_attrs(instance.def); + attr::requests_inline(&attributes[..]) + } } TransItem::DropGlue(..) => true, TransItem::Static(..) => false, @@ -272,9 +274,10 @@ impl<'a, 'tcx> TransItem<'tcx> { } } - pub fn is_lazily_instantiated(&self) -> bool { + pub fn is_instantiated_only_on_demand(&self) -> bool { match *self { - TransItem::Fn(ref instance) => !instance.substs.types.is_empty(), + TransItem::Fn(ref instance) => !instance.def.is_local() || + !instance.substs.types.is_empty(), TransItem::DropGlue(..) => true, TransItem::Static(..) => false, } diff --git a/src/test/codegen-units/partitioning/extern-drop-glue.rs b/src/test/codegen-units/partitioning/extern-drop-glue.rs index 7072a211d24..910ffd2959e 100644 --- a/src/test/codegen-units/partitioning/extern-drop-glue.rs +++ b/src/test/codegen-units/partitioning/extern-drop-glue.rs @@ -20,15 +20,15 @@ // aux-build:cgu_extern_drop_glue.rs extern crate cgu_extern_drop_glue; -//~ TRANS_ITEM drop-glue cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[OnceODR] extern_drop_glue-mod1[OnceODR] -//~ TRANS_ITEM drop-glue-contents cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[OnceODR] extern_drop_glue-mod1[OnceODR] +//~ TRANS_ITEM drop-glue cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[Internal] extern_drop_glue-mod1[Internal] +//~ TRANS_ITEM drop-glue-contents cgu_extern_drop_glue::Struct[0] @@ extern_drop_glue[Internal] extern_drop_glue-mod1[Internal] struct LocalStruct(cgu_extern_drop_glue::Struct); //~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External] fn user() { - //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[OnceODR] + //~ TRANS_ITEM drop-glue extern_drop_glue::LocalStruct[0] @@ extern_drop_glue[Internal] let _ = LocalStruct(cgu_extern_drop_glue::Struct(0)); } @@ -40,7 +40,7 @@ mod mod1 { //~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External] fn user() { - //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[OnceODR] + //~ TRANS_ITEM drop-glue extern_drop_glue::mod1[0]::LocalStruct[0] @@ extern_drop_glue-mod1[Internal] let _ = LocalStruct(cgu_extern_drop_glue::Struct(0)); } } diff --git a/src/test/codegen-units/partitioning/extern-generic.rs b/src/test/codegen-units/partitioning/extern-generic.rs index 5801727494f..58f904f48a1 100644 --- a/src/test/codegen-units/partitioning/extern-generic.rs +++ b/src/test/codegen-units/partitioning/extern-generic.rs @@ -58,7 +58,7 @@ mod mod3 { // Make sure the two generic functions from the extern crate get instantiated // privately in every module they are use in. -//~ TRANS_ITEM fn cgu_generic_function::foo[0]<&str> @@ extern_generic[OnceODR] extern_generic-mod1[OnceODR] extern_generic-mod2[OnceODR] extern_generic-mod1-mod1[OnceODR] -//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ extern_generic[OnceODR] extern_generic-mod1[OnceODR] extern_generic-mod2[OnceODR] extern_generic-mod1-mod1[OnceODR] +//~ TRANS_ITEM fn cgu_generic_function::foo[0]<&str> @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod2[Internal] extern_generic-mod1-mod1[Internal] +//~ TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod2[Internal] extern_generic-mod1-mod1[Internal] //~ TRANS_ITEM drop-glue i8 diff --git a/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs b/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs index 285b068fe1c..118513f6554 100644 --- a/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs +++ b/src/test/codegen-units/partitioning/inlining-from-extern-crate.rs @@ -21,8 +21,8 @@ extern crate cgu_explicit_inlining; // This test makes sure that items inlined from external crates are privately // instantiated in every codegen unit they are used in. -//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod1[Available] -//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[Available] inlining_from_extern_crate-mod2[Available] +//~ TRANS_ITEM fn cgu_explicit_inlining::inlined[0] @@ inlining_from_extern_crate[Internal] inlining_from_extern_crate-mod1[Internal] +//~ TRANS_ITEM fn cgu_explicit_inlining::always_inlined[0] @@ inlining_from_extern_crate[Internal] inlining_from_extern_crate-mod2[Internal] //~ TRANS_ITEM fn inlining_from_extern_crate::user[0] @@ inlining_from_extern_crate[External] pub fn user() diff --git a/src/test/codegen-units/partitioning/local-drop-glue.rs b/src/test/codegen-units/partitioning/local-drop-glue.rs index dc50650de6d..f61e3fe1293 100644 --- a/src/test/codegen-units/partitioning/local-drop-glue.rs +++ b/src/test/codegen-units/partitioning/local-drop-glue.rs @@ -16,8 +16,8 @@ #![allow(dead_code)] #![crate_type="lib"] -//~ TRANS_ITEM drop-glue local_drop_glue::Struct[0] @@ local_drop_glue[OnceODR] local_drop_glue-mod1[OnceODR] -//~ TRANS_ITEM drop-glue-contents local_drop_glue::Struct[0] @@ local_drop_glue[OnceODR] local_drop_glue-mod1[OnceODR] +//~ TRANS_ITEM drop-glue local_drop_glue::Struct[0] @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal] +//~ TRANS_ITEM drop-glue-contents local_drop_glue::Struct[0] @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal] struct Struct { _a: u32 } @@ -27,7 +27,7 @@ impl Drop for Struct { fn drop(&mut self) {} } -//~ TRANS_ITEM drop-glue local_drop_glue::Outer[0] @@ local_drop_glue[OnceODR] +//~ TRANS_ITEM drop-glue local_drop_glue::Outer[0] @@ local_drop_glue[Internal] struct Outer { _a: Struct } @@ -46,10 +46,10 @@ mod mod1 { use super::Struct; - //~ TRANS_ITEM drop-glue local_drop_glue::mod1[0]::Struct2[0] @@ local_drop_glue-mod1[OnceODR] + //~ TRANS_ITEM drop-glue local_drop_glue::mod1[0]::Struct2[0] @@ local_drop_glue-mod1[Internal] struct Struct2 { _a: Struct, - //~ TRANS_ITEM drop-glue (u32, local_drop_glue::Struct[0]) @@ local_drop_glue-mod1[OnceODR] + //~ TRANS_ITEM drop-glue (u32, local_drop_glue::Struct[0]) @@ local_drop_glue-mod1[Internal] _b: (u32, Struct), } diff --git a/src/test/codegen-units/partitioning/local-generic.rs b/src/test/codegen-units/partitioning/local-generic.rs index bfc911e36e9..2d744169d3f 100644 --- a/src/test/codegen-units/partitioning/local-generic.rs +++ b/src/test/codegen-units/partitioning/local-generic.rs @@ -16,13 +16,10 @@ #![allow(dead_code)] #![crate_type="lib"] -// Used in different modules/codegen units but always instantiated in the same -// codegen unit. - -//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic.volatile[WeakODR] -//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic.volatile[WeakODR] -//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic.volatile[WeakODR] -//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic.volatile[WeakODR] +//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic[Internal] +//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic-mod1[Internal] +//~ TRANS_ITEM fn local_generic::generic[0] @@ local_generic-mod1-mod1[Internal] +//~ TRANS_ITEM fn local_generic::generic[0]<&str> @@ local_generic-mod2[Internal] pub fn generic(x: T) -> T { x } //~ TRANS_ITEM fn local_generic::user[0] @@ local_generic[External] diff --git a/src/test/codegen-units/partitioning/methods-are-with-self-type.rs b/src/test/codegen-units/partitioning/methods-are-with-self-type.rs index f8e7d8d2554..1ea5aafd401 100644 --- a/src/test/codegen-units/partitioning/methods-are-with-self-type.rs +++ b/src/test/codegen-units/partitioning/methods-are-with-self-type.rs @@ -8,6 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Currently, all generic functions are instantiated in each codegen unit that +// uses them, even those not marked with #[inline], so this test does not make +// much sense at the moment. +// ignore-test + // ignore-tidy-linelength // We specify -Z incremental here because we want to test the partitioning for // incremental compilation