From c75bbd96ac82575db0a27377c6c18882ca29189e 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 | 54 ++++--- src/librustc_trans/trans_item.rs | 258 ++++++++++++++++++------------- 3 files changed, 221 insertions(+), 137 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 5250361cd17ae..59166a52e85db 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2251,8 +2251,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: `{}`", §)); } @@ -2282,7 +2284,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), @@ -2338,13 +2340,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". } _ => {} } @@ -2702,6 +2700,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()); @@ -2725,6 +2724,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 e988d2e6ac314..2782c796005b5 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -1020,22 +1020,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); - - // 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)) - }) + // Make sure that this is never executed for something inlined. + assert!(!ccx.external_srcs().borrow().contains_key(&id)); + + 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 { @@ -1086,17 +1093,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 @@ -1200,6 +1209,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 d7c5c41a156ba..92fddd7d77d91 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 c8eec7753224fad4d815ef47f953e54370447965 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 | 172 +++++++++++++++-------------- 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, 210 insertions(+), 175 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 59166a52e85db..375710649df29 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -1950,6 +1950,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, @@ -2269,70 +2312,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() { @@ -2340,8 +2336,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". } _ => {} @@ -2484,16 +2481,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); } } @@ -2503,21 +2500,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); + + 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 !((linkage == llvm::ExternalLinkage as c_uint || - linkage == llvm::WeakODRLinkage as c_uint) && - llvm::LLVMIsDeclaration(val) == 0) { - continue; - } + 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); + } - 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); } } } @@ -2746,6 +2749,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); + } _ => { } } } @@ -2982,7 +2988,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 9196cfce16feb..bf9cde9a44166 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 bbc01f0935f2b..bcc9ef5adb18f 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 af175fbf88256..4077b894d62d4 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 dad82167a76b3..8be04ea832286 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 92fddd7d77d91..f3c0ddfaf73a9 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 3f9ebe00a1183723688e4c49197a83da286ec953 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 375710649df29..45ed86439b7e6 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; @@ -1919,37 +1917,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); @@ -2216,46 +2183,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 211efeb4e4baa..acde59036da38 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 8be04ea832286..aabc31673590a 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 syntax::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 ac7ad72a1041bc67d372a597c3834c6b33c7cd97 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 546bb235a5f50..2b5ac7e8d80de 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 ba77266d07248..278e9189dd6a7 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 e86d8d10fe309e24c6bf41872f359b53e50a52b0 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 7a1ac7c218c8c..2e256ddcbf634 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -196,23 +196,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 af68b7632dd18..c0c3a591a9546 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1093,7 +1093,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 4676b0a67e4ae..be7960548aa66 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -204,7 +204,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")); @@ -315,7 +315,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) @@ -355,10 +355,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() } @@ -496,7 +497,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 31bc11fb215b0..69e4a50804fad 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 cf81777be261d..9a4cd3046e889 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 @@ -705,27 +710,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. @@ -740,32 +747,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.) + } } }; @@ -781,17 +798,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 => {} @@ -802,51 +821,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); } } @@ -866,28 +889,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); } } @@ -906,6 +932,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(); @@ -973,7 +1001,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 45ed86439b7e6..05f08e55ca130 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2631,6 +2631,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(), }; @@ -2645,7 +2646,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. @@ -2791,6 +2796,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 8724945ed901b..d712cea24c504 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -128,8 +128,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 2ded643ef4fdd..d15519a87d980 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 1621b49aa359f5e11592b6568fedea1f8bd2a7bd 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 05f08e55ca130..5851678c2ea30 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2549,8 +2549,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: @@ -2564,9 +2564,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. @@ -2576,8 +2576,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() } @@ -2599,6 +2599,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 @@ -2622,12 +2623,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 { @@ -2756,7 +2754,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::>(); @@ -2912,7 +2910,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 d15519a87d980..cd08fe68f0cc5 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 f3c0ddfaf73a9..ae6e095d14291 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 5262d31ae0dca..7072a211d2401 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 6beed231df993..5801727494f7c 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 967824f31d045..285b068fe1c6c 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 04ebef645ec98..dc50650de6d43 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 f5641f1f2ed73..bfc911e36e9a8 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 880cc0a4fb47a..5eb1cbc2199f7 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 f3efa2587d3d5..28c4698eabd1f 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 99dda0e38bad7..f8e7d8d255465 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 c3af86f820f18..4da6411032168 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 9e878b95a369a..ffe1ec278b8dd 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 375c52229c1ace8aed1b91f2d79002143d2292ab 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 df3d2d149b99c..6c2a09f8060c5 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 170c8f75b5056..ebb6e0baf20a0 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 5851678c2ea30..14f78dba60424 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2183,52 +2183,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]) { @@ -2674,24 +2628,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); } } @@ -2928,7 +2866,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 bcc9ef5adb18f..26cd0b0998aa1 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 bfcb1ae33b301..7f6e8fa035ab7 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 e6db695943bbe..2746d3fb6b0b6 100644 --- a/src/librustc_trans/declare.rs +++ b/src/librustc_trans/declare.rs @@ -138,24 +138,34 @@ pub fn define_global(ccx: &CrateContext, name: &str, ty: Type) -> Option(ccx: &CrateContext<'a, 'tcx>, - name: &str, - fn_type: ty::Ty<'tcx>) -> ValueRef { +pub fn define_fn<'a, 'tcx>(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 + declare_fn(ccx, name, fn_type) } } +/// Declare a Rust function with an intention to define it. +/// +/// Use this function when you intend to define a function. This function will +/// return panic if the name already has a definition associated with it. This +/// can happen with #[no_mangle] or #[export_name], for example. +pub fn define_internal_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, + name: &str, + fn_type: ty::Ty<'tcx>) -> ValueRef { + let llfn = define_fn(ccx, name, fn_type); + llvm::SetLinkage(llfn, llvm::InternalLinkage); + llfn +} + /// Get declared value by name. pub fn get_declared_value(ccx: &CrateContext, name: &str) -> Option { diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index acde59036da38..2f264594ea601 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 cd08fe68f0cc5..a0360a8ed226e 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 ae6e095d14291..a07deda2b1e5e 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 63c65e3556d3abf6df506b0461aa022770968683 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 14f78dba60424..809e002c53b90 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::codemap::{Span, DUMMY_SP}; @@ -2589,14 +2591,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[..]), @@ -2694,8 +2700,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()); } @@ -2717,7 +2724,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) })); } @@ -2811,7 +2818,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 { @@ -2834,10 +2841,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 { @@ -2911,5 +2921,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 7099246c6abde..00a0229fe4d4c 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 2782c796005b5..e2b55beaab45c 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -1016,14 +1016,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)); @@ -1031,16 +1033,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) } @@ -1048,6 +1050,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 @@ -1082,7 +1085,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); @@ -1107,6 +1110,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 7f6e8fa035ab7..64e0351610f24 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 d712cea24c504..129d0dcf9af99 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -121,6 +121,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 aabc31673590a..10610eec148e9 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 0000000000000..4f82b54c76b02 --- /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 a07deda2b1e5e..4fa0ba03005ab 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 267f72f572c39f845d70183df209dabb31751d3b 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 809e002c53b90..03d9b54e6f1ce 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2627,14 +2627,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 a0360a8ed226e..0e06fd6235e58 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 ba1bdf64682dafaf7f71c0ff8bd849d412844bfd 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 | 115 +++++++++++++----------- src/librustc_trans/symbol_names_test.rs | 16 ++-- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 03d9b54e6f1ce..56cc8c04c2265 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2199,34 +2199,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 @@ -2236,22 +2219,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; } -} -/// 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 main_llfn = Callee::def(ccx, main_def_id, instance.substs).reify(ccx).val; + 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. } @@ -2591,13 +2592,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, @@ -2643,28 +2642,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); - - // 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()); + // If this codegen unit contains the main function, also create the + // wrapper here + maybe_create_entry_wrapper(&ccx); - 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)); @@ -2672,6 +2655,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() { @@ -2697,6 +2700,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); @@ -2868,6 +2872,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 11e9e9f3204a2..9a7fe54e0d9f5 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 6790fc4f4035abd8a77d2c88b5df0afc8040158a 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 56cc8c04c2265..a23aed0217844 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2627,7 +2627,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); @@ -2637,7 +2637,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 0e06fd6235e58..2f1961ac9f830 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 83dd6a3b00258..25f8c13046997 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 ecc381ff027e87ff4a9da686e8bd05cbd7dc50ed 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 9a4cd3046e889..48524e7744369 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -804,12 +804,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 26cd0b0998aa1..22351913637f6 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 57c718cacb7b1fb84f7d361333257854b28db083 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 a23aed0217844..36313c366cf82 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2668,9 +2668,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 00a0229fe4d4c..1adec15698396 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 2f264594ea601..a45df9f272a3c 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 10610eec148e9..e345c0d00785a 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 2f1961ac9f830..785c193c85570 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 4f82b54c76b02..3faaa085dce14 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 4fa0ba03005ab..b4f8a116662ea 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 0b0eb3de5952e4737168d22bf6c673041ebff10e 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 22351913637f6..6c0269b61a0f8 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -203,6 +203,7 @@ use rustc::mir::visit as mir_visit; use rustc::mir::visit::Visitor as MirVisitor; use syntax::abi::Abi; +use syntax::ast::NodeId; use syntax::codemap::DUMMY_SP; use syntax::errors; use base::custom_coerce_unsize_info; @@ -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 01309d35355d0b7639b00ba51a43faa9ccd0097b 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 652886ff2a044..b0fc7a561d058 100644 --- a/src/librustc_trans/expr.rs +++ b/src/librustc_trans/expr.rs @@ -1694,11 +1694,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, @@ -1716,6 +1718,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 933d723314341227685f4a8cee5299469aca0e53 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 1adec15698396..3b39ce708e95f 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 86613cdcaa45c12b10396136c5dcf4ae494ea7ba 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 36313c366cf82..c99ae05d6c1de 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -2685,6 +2685,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()); @@ -2876,6 +2878,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(); @@ -2927,12 +2937,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 64e0351610f24..8f700e2fe389b 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 a45df9f272a3c..6405fd6074ade 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 e345c0d00785a..461dcfd571e5a 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 785c193c85570..edf5db81b1843 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 b4f8a116662ea..2fc90b821feae 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 4cdf365fab61e0a4639afa05dee99f48e0fa5a75 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 bf9cde9a44166..b992ba362a983 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 2fc90b821feae..b9700f2cacfc2 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 56a3bf2db19e0fc9fd5f29ab2e5c71cf66d051e0 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 c99ae05d6c1de..d063270163ed3 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}; @@ -1835,10 +1835,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); @@ -2662,7 +2658,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); { @@ -2882,7 +2877,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 6c0269b61a0f8..7334d853b34e0 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 e2b55beaab45c..c818c497942f1 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}; @@ -1143,11 +1142,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 8f700e2fe389b..b8d231db40a2a 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 6405fd6074ade..ffb65a8efec5c 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 461dcfd571e5a..7472668ed9fa2 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 be9101949fbb71355d4d8efdd4bc9caa53d98d6b Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 10 Jun 2016 19:06:21 -0400 Subject: [PATCH 20/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 | 64 +++++++++---------- src/librustc_trans/trans_item.rs | 11 ++-- .../partitioning/extern-drop-glue.rs | 8 +-- .../partitioning/extern-generic.rs | 4 +- .../partitioning/local-drop-glue.rs | 10 +-- .../partitioning/local-generic.rs | 11 ++-- .../methods-are-with-self-type.rs | 5 ++ 9 files changed, 59 insertions(+), 58 deletions(-) diff --git a/src/librustc_trans/glue.rs b/src/librustc_trans/glue.rs index ffb65a8efec5c..2065d812c922d 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 7472668ed9fa2..66768d8a544dd 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 edf5db81b1843..2e068232c4928 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,23 @@ 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 + 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 { - 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 +515,19 @@ 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() { llvm::InternalLinkage - } else { + } else if trans_item.is_from_extern_crate() { + llvm::AvailableExternallyLinkage + } else if reachable.contains(&tcx.map + .as_local_node_id(instance.def) + .unwrap()) { llvm::ExternalLinkage + } else { + llvm::InternalLinkage } } } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index b9700f2cacfc2..b7b18b2631bee 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 7072a211d2401..910ffd2959ed0 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 5801727494f7c..58f904f48a17d 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/local-drop-glue.rs b/src/test/codegen-units/partitioning/local-drop-glue.rs index dc50650de6d43..f61e3fe12931e 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 bfc911e36e9a8..2d744169d3f8e 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 f8e7d8d255465..1ea5aafd401d2 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 From 06b4eb9cd32f68223ef77128a539123898b2d7ed Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 22 Jun 2016 11:59:34 -0400 Subject: [PATCH 21/21] Update LLVM. --- src/llvm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llvm b/src/llvm index 80ad955b60b3a..e2debac0311c5 160000 --- a/src/llvm +++ b/src/llvm @@ -1 +1 @@ -Subproject commit 80ad955b60b3ac02d0462a4a65fcea597d0ebfb1 +Subproject commit e2debac0311c59a9d5600b251e5a0c210349de0d