From e964ea5bf575d68f164d02e4d1aa45b5bf924c0d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 24 Sep 2023 21:06:39 +1000 Subject: [PATCH 1/3] coverage: Emit mappings for unused functions without generating stubs --- .../src/coverageinfo/mapgen.rs | 35 +++++++-- .../src/coverageinfo/mod.rs | 76 ++----------------- 2 files changed, 37 insertions(+), 74 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index f75add90aa2..4f12f4af175 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -4,7 +4,7 @@ use crate::coverageinfo::ffi::CounterMappingRegion; use crate::coverageinfo::map_data::FunctionCoverage; use crate::llvm; -use rustc_codegen_ssa::traits::ConstMethods; +use rustc_codegen_ssa::traits::{BaseTypeMethods, ConstMethods}; use rustc_data_structures::fx::FxIndexSet; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; @@ -94,8 +94,14 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) { // Generate the LLVM IR representation of the coverage map and store it in a well-known global let cov_data_val = generate_coverage_map(cx, version, filenames_size, filenames_val); + let mut unused_function_names = Vec::new(); + let covfun_section_name = coverageinfo::covfun_section_name(cx); for (mangled_function_name, source_hash, is_used, coverage_mapping_buffer) in function_data { + if !is_used { + unused_function_names.push(mangled_function_name); + } + save_function_record( cx, &covfun_section_name, @@ -107,6 +113,25 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) { ); } + // For unused functions, we need to take their mangled names and store them + // in a specially-named global array. LLVM's `InstrProfiling` pass will + // detect this global and include those names in its `__llvm_prf_names` + // section. (See `llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp`.) + if !unused_function_names.is_empty() { + assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu()); + + let name_globals = unused_function_names + .into_iter() + .map(|mangled_function_name| cx.const_str(mangled_function_name).0) + .collect::>(); + let initializer = cx.const_array(cx.type_ptr(), &name_globals); + + let array = llvm::add_global(cx.llmod, cx.val_ty(initializer), "__llvm_coverage_names"); + llvm::set_global_constant(array, true); + llvm::set_linkage(array, llvm::Linkage::InternalLinkage); + llvm::set_initializer(array, initializer); + } + // Save the coverage data value to LLVM IR coverageinfo::save_cov_data_to_mod(cx, cov_data_val); } @@ -290,10 +315,10 @@ fn save_function_record( /// `DefId`s (`tcx` query `mir_keys`) minus the codegenned `DefId`s (`tcx` query /// `codegened_and_inlined_items`). /// -/// These unused functions are then codegen'd in one of the CGUs which is marked as the -/// "code coverage dead code cgu" during the partitioning process. This prevents us from generating -/// code regions for the same function more than once which can lead to linker errors regarding -/// duplicate symbols. +/// These unused functions don't need to be codegenned, but we do need to add them to the function +/// coverage map (in a single designated CGU) so that we still emit coverage mappings for them. +/// We also end up adding their symbol names to a special global array that LLVM will include in +/// its embedded coverage data. fn add_unused_functions(cx: &CodegenCx<'_, '_>) { assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu()); diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 204a73b788a..5a5f6eb89f6 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -1,6 +1,5 @@ use crate::llvm; -use crate::abi::Abi; use crate::builder::Builder; use crate::common::CodegenCx; use crate::coverageinfo::ffi::{CounterExpression, CounterMappingRegion}; @@ -12,17 +11,13 @@ use rustc_codegen_ssa::traits::{ StaticMethods, }; use rustc_data_structures::fx::FxHashMap; -use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_llvm::RustString; use rustc_middle::bug; -use rustc_middle::mir::coverage::{CounterId, CoverageKind, FunctionCoverageInfo}; +use rustc_middle::mir::coverage::{CoverageKind, FunctionCoverageInfo}; use rustc_middle::mir::Coverage; -use rustc_middle::ty; -use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt}; -use rustc_middle::ty::GenericArgs; -use rustc_middle::ty::Instance; -use rustc_middle::ty::Ty; +use rustc_middle::ty::layout::HasTyCtxt; +use rustc_middle::ty::{self, GenericArgs, Instance, TyCtxt}; use std::cell::RefCell; @@ -30,8 +25,6 @@ pub(crate) mod ffi; pub(crate) mod map_data; pub mod mapgen; -const UNUSED_FUNCTION_COUNTER_ID: CounterId = CounterId::START; - const VAR_ALIGN_BYTES: usize = 8; /// A context object for maintaining all state needed by the coverageinfo module. @@ -77,22 +70,8 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { } } - /// Functions with MIR-based coverage are normally codegenned _only_ if - /// called. LLVM coverage tools typically expect every function to be - /// defined (even if unused), with at least one call to LLVM intrinsic - /// `instrprof.increment`. - /// - /// Codegen a small function that will never be called, with one counter - /// that will never be incremented. - /// - /// For used/called functions, the coverageinfo was already added to the - /// `function_coverage_map` (keyed by function `Instance`) during codegen. - /// But in this case, since the unused function was _not_ previously - /// codegenned, collect the function coverage info from MIR and add an - /// "unused" entry to the function coverage map. fn define_unused_fn(&self, def_id: DefId, function_coverage_info: &'tcx FunctionCoverageInfo) { - let instance = declare_unused_fn(self, def_id); - codegen_unused_fn_and_counter(self, instance); + let instance = declare_unused_fn(self.tcx, def_id); add_unused_function_coverage(self, instance, function_coverage_info); } } @@ -159,10 +138,8 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { } } -fn declare_unused_fn<'tcx>(cx: &CodegenCx<'_, 'tcx>, def_id: DefId) -> Instance<'tcx> { - let tcx = cx.tcx; - - let instance = Instance::new( +fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Instance<'tcx> { + Instance::new( def_id, GenericArgs::for_item(tcx, def_id, |param, _| { if let ty::GenericParamDefKind::Lifetime = param.kind { @@ -171,46 +148,7 @@ fn declare_unused_fn<'tcx>(cx: &CodegenCx<'_, 'tcx>, def_id: DefId) -> Instance< tcx.mk_param_from_def(param) } }), - ); - - let llfn = cx.declare_fn( - tcx.symbol_name(instance).name, - cx.fn_abi_of_fn_ptr( - ty::Binder::dummy(tcx.mk_fn_sig( - [Ty::new_unit(tcx)], - Ty::new_unit(tcx), - false, - hir::Unsafety::Unsafe, - Abi::Rust, - )), - ty::List::empty(), - ), - None, - ); - - llvm::set_linkage(llfn, llvm::Linkage::PrivateLinkage); - llvm::set_visibility(llfn, llvm::Visibility::Default); - - assert!(cx.instances.borrow_mut().insert(instance, llfn).is_none()); - - instance -} - -fn codegen_unused_fn_and_counter<'tcx>(cx: &CodegenCx<'_, 'tcx>, instance: Instance<'tcx>) { - let llfn = cx.get_fn(instance); - let llbb = Builder::append_block(cx, llfn, "unused_function"); - let mut bx = Builder::build(cx, llbb); - let fn_name = bx.get_pgo_func_name_var(instance); - let hash = bx.const_u64(0); - let num_counters = bx.const_u32(1); - let index = bx.const_u32(u32::from(UNUSED_FUNCTION_COUNTER_ID)); - debug!( - "codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, - index={:?}) for unused function: {:?}", - fn_name, hash, num_counters, index, instance - ); - bx.instrprof_increment(fn_name, hash, num_counters, index); - bx.ret_void(); + ) } fn add_unused_function_coverage<'tcx>( From cdeeffde64b100ceb135135aef6879241ab6682f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 24 Sep 2023 22:26:12 +1000 Subject: [PATCH 2/3] coverage: Move unused-function helpers closer to where they are used --- .../src/coverageinfo/mapgen.rs | 43 ++++++++++++++++--- .../src/coverageinfo/mod.rs | 39 +---------------- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 4f12f4af175..4653849a29d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -10,6 +10,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_index::IndexVec; use rustc_middle::bug; +use rustc_middle::mir; use rustc_middle::mir::coverage::CodeRegion; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::Symbol; @@ -351,17 +352,47 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { let codegenned_def_ids = tcx.codegened_and_inlined_items(()); - for non_codegenned_def_id in - eligible_def_ids.into_iter().filter(|id| !codegenned_def_ids.contains(id)) - { + // For each `DefId` that should have coverage instrumentation but wasn't + // codegenned, add it to the function coverage map as an unused function. + for def_id in eligible_def_ids.into_iter().filter(|id| !codegenned_def_ids.contains(id)) { // Skip any function that didn't have coverage data added to it by the // coverage instrumentor. - let body = tcx.instance_mir(ty::InstanceDef::Item(non_codegenned_def_id)); + let body = tcx.instance_mir(ty::InstanceDef::Item(def_id)); let Some(function_coverage_info) = body.function_coverage_info.as_deref() else { continue; }; - debug!("generating unused fn: {:?}", non_codegenned_def_id); - cx.define_unused_fn(non_codegenned_def_id, function_coverage_info); + debug!("generating unused fn: {def_id:?}"); + let instance = declare_unused_fn(tcx, def_id); + add_unused_function_coverage(cx, instance, function_coverage_info); + } +} + +fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::Instance<'tcx> { + ty::Instance::new( + def_id, + ty::GenericArgs::for_item(tcx, def_id, |param, _| { + if let ty::GenericParamDefKind::Lifetime = param.kind { + tcx.lifetimes.re_erased.into() + } else { + tcx.mk_param_from_def(param) + } + }), + ) +} + +fn add_unused_function_coverage<'tcx>( + cx: &CodegenCx<'_, 'tcx>, + instance: ty::Instance<'tcx>, + function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo, +) { + // An unused function's mappings will automatically be rewritten to map to + // zero, because none of its counters/expressions are marked as seen. + let function_coverage = FunctionCoverage::unused(instance, function_coverage_info); + + if let Some(coverage_context) = cx.coverage_context() { + coverage_context.function_coverage_map.borrow_mut().insert(instance, function_coverage); + } else { + bug!("Could not get the `coverage_context`"); } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 5a5f6eb89f6..cf7c7e6be60 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -11,13 +11,12 @@ use rustc_codegen_ssa::traits::{ StaticMethods, }; use rustc_data_structures::fx::FxHashMap; -use rustc_hir::def_id::DefId; use rustc_llvm::RustString; use rustc_middle::bug; -use rustc_middle::mir::coverage::{CoverageKind, FunctionCoverageInfo}; +use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::Coverage; use rustc_middle::ty::layout::HasTyCtxt; -use rustc_middle::ty::{self, GenericArgs, Instance, TyCtxt}; +use rustc_middle::ty::Instance; use std::cell::RefCell; @@ -69,11 +68,6 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { bug!("Could not get the `coverage_context`"); } } - - fn define_unused_fn(&self, def_id: DefId, function_coverage_info: &'tcx FunctionCoverageInfo) { - let instance = declare_unused_fn(self.tcx, def_id); - add_unused_function_coverage(self, instance, function_coverage_info); - } } impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { @@ -138,35 +132,6 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { } } -fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Instance<'tcx> { - Instance::new( - def_id, - GenericArgs::for_item(tcx, def_id, |param, _| { - if let ty::GenericParamDefKind::Lifetime = param.kind { - tcx.lifetimes.re_erased.into() - } else { - tcx.mk_param_from_def(param) - } - }), - ) -} - -fn add_unused_function_coverage<'tcx>( - cx: &CodegenCx<'_, 'tcx>, - instance: Instance<'tcx>, - function_coverage_info: &'tcx FunctionCoverageInfo, -) { - // An unused function's mappings will automatically be rewritten to map to - // zero, because none of its counters/expressions are marked as seen. - let function_coverage = FunctionCoverage::unused(instance, function_coverage_info); - - if let Some(coverage_context) = cx.coverage_context() { - coverage_context.function_coverage_map.borrow_mut().insert(instance, function_coverage); - } else { - bug!("Could not get the `coverage_context`"); - } -} - /// Calls llvm::createPGOFuncNameVar() with the given function instance's /// mangled function name. The LLVM API returns an llvm::GlobalVariable /// containing the function name, with the specific variable name and linkage From 6f1ca8d9ebca7960408584b3b325dca50fcb781d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 28 Sep 2023 15:37:44 +1000 Subject: [PATCH 3/3] coverage: Change query `codegened_and_inlined_items` to a plain function This query has a name that sounds general-purpose, but in fact it has coverage-specific semantics, and (fortunately) is only used by coverage code. Because it is only ever called once (from one designated CGU), it doesn't need to be a query, and we can change it to a regular function instead. --- .../src/coverageinfo/mapgen.rs | 37 +++++++++++++++++-- compiler/rustc_middle/src/query/mod.rs | 6 --- .../rustc_monomorphize/src/partitioning.rs | 31 ---------------- 3 files changed, 34 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 4653849a29d..2f825b801ac 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -13,6 +13,7 @@ use rustc_middle::bug; use rustc_middle::mir; use rustc_middle::mir::coverage::CodeRegion; use rustc_middle::ty::{self, TyCtxt}; +use rustc_span::def_id::DefIdSet; use rustc_span::Symbol; /// Generates and exports the Coverage Map. @@ -313,8 +314,7 @@ fn save_function_record( /// `-Clink-dead-code` will not generate code for unused generic functions.) /// /// We can find the unused functions (including generic functions) by the set difference of all MIR -/// `DefId`s (`tcx` query `mir_keys`) minus the codegenned `DefId`s (`tcx` query -/// `codegened_and_inlined_items`). +/// `DefId`s (`tcx` query `mir_keys`) minus the codegenned `DefId`s (`codegenned_and_inlined_items`). /// /// These unused functions don't need to be codegenned, but we do need to add them to the function /// coverage map (in a single designated CGU) so that we still emit coverage mappings for them. @@ -350,7 +350,7 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { }) .collect(); - let codegenned_def_ids = tcx.codegened_and_inlined_items(()); + let codegenned_def_ids = codegenned_and_inlined_items(tcx); // For each `DefId` that should have coverage instrumentation but wasn't // codegenned, add it to the function coverage map as an unused function. @@ -368,6 +368,37 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { } } +/// All items participating in code generation together with (instrumented) +/// items inlined into them. +fn codegenned_and_inlined_items(tcx: TyCtxt<'_>) -> DefIdSet { + let (items, cgus) = tcx.collect_and_partition_mono_items(()); + let mut visited = DefIdSet::default(); + let mut result = items.clone(); + + for cgu in cgus { + for item in cgu.items().keys() { + if let mir::mono::MonoItem::Fn(ref instance) = item { + let did = instance.def_id(); + if !visited.insert(did) { + continue; + } + let body = tcx.instance_mir(instance.def); + for block in body.basic_blocks.iter() { + for statement in &block.statements { + let mir::StatementKind::Coverage(_) = statement.kind else { continue }; + let scope = statement.source_info.scope; + if let Some(inlined) = scope.inlined_instance(&body.source_scopes) { + result.insert(inlined.def_id()); + } + } + } + } + } + } + + result +} + fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::Instance<'tcx> { ty::Instance::new( def_id, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index b5f873c9257..cd5206a837f 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1882,12 +1882,6 @@ rustc_queries! { desc { |tcx| "determining whether `{}` needs codegen", tcx.def_path_str(def_id) } } - /// All items participating in code generation together with items inlined into them. - query codegened_and_inlined_items(_: ()) -> &'tcx DefIdSet { - eval_always - desc { "collecting codegened and inlined items" } - } - query codegen_unit(sym: Symbol) -> &'tcx CodegenUnit<'tcx> { desc { "getting codegen unit `{sym}`" } } diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 1d8cbe0e21b..4009e289240 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -105,7 +105,6 @@ use rustc_hir::def_id::{DefId, DefIdSet, LOCAL_CRATE}; use rustc_hir::definitions::DefPathDataName; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::exported_symbols::{SymbolExportInfo, SymbolExportLevel}; -use rustc_middle::mir; use rustc_middle::mir::mono::{ CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, Linkage, MonoItem, MonoItemData, Visibility, @@ -1279,38 +1278,8 @@ fn dump_mono_items_stats<'tcx>( Ok(()) } -fn codegened_and_inlined_items(tcx: TyCtxt<'_>, (): ()) -> &DefIdSet { - let (items, cgus) = tcx.collect_and_partition_mono_items(()); - let mut visited = DefIdSet::default(); - let mut result = items.clone(); - - for cgu in cgus { - for item in cgu.items().keys() { - if let MonoItem::Fn(ref instance) = item { - let did = instance.def_id(); - if !visited.insert(did) { - continue; - } - let body = tcx.instance_mir(instance.def); - for block in body.basic_blocks.iter() { - for statement in &block.statements { - let mir::StatementKind::Coverage(_) = statement.kind else { continue }; - let scope = statement.source_info.scope; - if let Some(inlined) = scope.inlined_instance(&body.source_scopes) { - result.insert(inlined.def_id()); - } - } - } - } - } - } - - tcx.arena.alloc(result) -} - pub fn provide(providers: &mut Providers) { providers.collect_and_partition_mono_items = collect_and_partition_mono_items; - providers.codegened_and_inlined_items = codegened_and_inlined_items; providers.is_codegened_item = |tcx, def_id| { let (all_mono_items, _) = tcx.collect_and_partition_mono_items(());