From 847fd88df724280880c705848ba1a120ce15e020 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 24 Mar 2024 20:06:05 -0400 Subject: Always use tcx.coroutine_layout over calling optimized_mir directly --- compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs | 2 +- compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs index 4792b0798df..9a10e1bc129 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs @@ -683,7 +683,7 @@ fn build_union_fields_for_direct_tag_coroutine<'ll, 'tcx>( _ => unreachable!(), }; - let coroutine_layout = cx.tcx.optimized_mir(coroutine_def_id).coroutine_layout().unwrap(); + let coroutine_layout = cx.tcx.coroutine_layout(coroutine_def_id).unwrap(); let common_upvar_names = cx.tcx.closure_saved_names_of_captured_variables(coroutine_def_id); let variant_range = coroutine_args.variant_range(coroutine_def_id, cx.tcx); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs index 3dbe820b8ff..f0f55981760 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs @@ -158,8 +158,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>( DIFlags::FlagZero, ), |cx, coroutine_type_di_node| { - let coroutine_layout = - cx.tcx.optimized_mir(coroutine_def_id).coroutine_layout().unwrap(); + let coroutine_layout = cx.tcx.coroutine_layout(coroutine_def_id).unwrap(); let Variants::Multiple { tag_encoding: TagEncoding::Direct, ref variants, .. } = coroutine_type_and_layout.variants -- cgit 1.4.1-3-g733a5 From b7d67eace78d5e660df93b513326650fe8226a96 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 24 Mar 2024 21:12:49 -0400 Subject: Require coroutine kind type to be passed to TyCtxt::coroutine_layout --- .../src/debuginfo/metadata/enums/cpp_like.rs | 3 +- .../src/debuginfo/metadata/enums/native.rs | 7 ++-- .../rustc_const_eval/src/transform/validate.rs | 13 ++++---- compiler/rustc_middle/src/mir/mod.rs | 3 +- compiler/rustc_middle/src/mir/pretty.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 37 ++++++++++++++++++++-- compiler/rustc_middle/src/ty/sty.rs | 7 ++-- compiler/rustc_ty_utils/src/layout.rs | 4 +-- 8 files changed, 59 insertions(+), 17 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs index 9a10e1bc129..4edef14422e 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs @@ -683,7 +683,8 @@ fn build_union_fields_for_direct_tag_coroutine<'ll, 'tcx>( _ => unreachable!(), }; - let coroutine_layout = cx.tcx.coroutine_layout(coroutine_def_id).unwrap(); + let coroutine_layout = + cx.tcx.coroutine_layout(coroutine_def_id, coroutine_args.kind_ty()).unwrap(); let common_upvar_names = cx.tcx.closure_saved_names_of_captured_variables(coroutine_def_id); let variant_range = coroutine_args.variant_range(coroutine_def_id, cx.tcx); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs index f0f55981760..115d5187eaf 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs @@ -135,7 +135,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>( unique_type_id: UniqueTypeId<'tcx>, ) -> DINodeCreationResult<'ll> { let coroutine_type = unique_type_id.expect_ty(); - let &ty::Coroutine(coroutine_def_id, _) = coroutine_type.kind() else { + let &ty::Coroutine(coroutine_def_id, coroutine_args) = coroutine_type.kind() else { bug!("build_coroutine_di_node() called with non-coroutine type: `{:?}`", coroutine_type) }; @@ -158,7 +158,10 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>( DIFlags::FlagZero, ), |cx, coroutine_type_di_node| { - let coroutine_layout = cx.tcx.coroutine_layout(coroutine_def_id).unwrap(); + let coroutine_layout = cx + .tcx + .coroutine_layout(coroutine_def_id, coroutine_args.as_coroutine().kind_ty()) + .unwrap(); let Variants::Multiple { tag_encoding: TagEncoding::Direct, ref variants, .. } = coroutine_type_and_layout.variants diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 08e3e42a82e..b085e4e76a1 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -101,9 +101,9 @@ impl<'tcx> MirPass<'tcx> for Validator { } // Enforce that coroutine-closure layouts are identical. - if let Some(layout) = body.coroutine_layout() + if let Some(layout) = body.coroutine_layout_raw() && let Some(by_move_body) = body.coroutine_by_move_body() - && let Some(by_move_layout) = by_move_body.coroutine_layout() + && let Some(by_move_layout) = by_move_body.coroutine_layout_raw() { if layout != by_move_layout { // If this turns out not to be true, please let compiler-errors know. @@ -715,13 +715,14 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { // args of the coroutine. Otherwise, we prefer to use this body // since we may be in the process of computing this MIR in the // first place. - let gen_body = if def_id == self.caller_body.source.def_id() { - self.caller_body + let layout = if def_id == self.caller_body.source.def_id() { + // FIXME: This is not right for async closures. + self.caller_body.coroutine_layout_raw() } else { - self.tcx.optimized_mir(def_id) + self.tcx.coroutine_layout(def_id, args.as_coroutine().kind_ty()) }; - let Some(layout) = gen_body.coroutine_layout() else { + let Some(layout) = layout else { self.fail( location, format!("No coroutine layout for {parent_ty:?}"), diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index e4dce2bdc9e..02af55fbf0e 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -652,8 +652,9 @@ impl<'tcx> Body<'tcx> { self.coroutine.as_ref().and_then(|coroutine| coroutine.resume_ty) } + /// Prefer going through [`TyCtxt::coroutine_layout`] rather than using this directly. #[inline] - pub fn coroutine_layout(&self) -> Option<&CoroutineLayout<'tcx>> { + pub fn coroutine_layout_raw(&self) -> Option<&CoroutineLayout<'tcx>> { self.coroutine.as_ref().and_then(|coroutine| coroutine.coroutine_layout.as_ref()) } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index f0499cf344f..41df2e3b587 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -126,7 +126,7 @@ fn dump_matched_mir_node<'tcx, F>( Some(promoted) => write!(file, "::{promoted:?}`")?, } writeln!(file, " {disambiguator} {pass_name}")?; - if let Some(ref layout) = body.coroutine_layout() { + if let Some(ref layout) = body.coroutine_layout_raw() { writeln!(file, "/* coroutine_layout = {layout:#?} */")?; } writeln!(file)?; diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 6ce53ccc8cd..aad2f6a4cf8 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -60,6 +60,7 @@ pub use rustc_target::abi::{ReprFlags, ReprOptions}; pub use rustc_type_ir::{DebugWithInfcx, InferCtxtLike, WithInfcx}; pub use vtable::*; +use std::assert_matches::assert_matches; use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; @@ -1826,8 +1827,40 @@ impl<'tcx> TyCtxt<'tcx> { /// Returns layout of a coroutine. Layout might be unavailable if the /// coroutine is tainted by errors. - pub fn coroutine_layout(self, def_id: DefId) -> Option<&'tcx CoroutineLayout<'tcx>> { - self.optimized_mir(def_id).coroutine_layout() + /// + /// Takes `coroutine_kind` which can be acquired from the `CoroutineArgs::kind_ty`, + /// e.g. `args.as_coroutine().kind_ty()`. + pub fn coroutine_layout( + self, + def_id: DefId, + coroutine_kind_ty: Ty<'tcx>, + ) -> Option<&'tcx CoroutineLayout<'tcx>> { + let mir = self.optimized_mir(def_id); + // Regular coroutine + if coroutine_kind_ty.is_unit() { + mir.coroutine_layout_raw() + } else { + // If we have a `Coroutine` that comes from an coroutine-closure, + // then it may be a by-move or by-ref body. + let ty::Coroutine(_, identity_args) = + *self.type_of(def_id).instantiate_identity().kind() + else { + unreachable!(); + }; + let identity_kind_ty = identity_args.as_coroutine().kind_ty(); + // If the types differ, then we must be getting the by-move body of + // a by-ref coroutine. + if identity_kind_ty == coroutine_kind_ty { + mir.coroutine_layout_raw() + } else { + assert_matches!(coroutine_kind_ty.to_opt_closure_kind(), Some(ClosureKind::FnOnce)); + assert_matches!( + identity_kind_ty.to_opt_closure_kind(), + Some(ClosureKind::Fn | ClosureKind::FnMut) + ); + mir.coroutine_by_move_body().unwrap().coroutine_layout_raw() + } + } } /// Given the `DefId` of an impl, returns the `DefId` of the trait it implements. diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index c85ee140fa4..82a2423cbc6 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -694,7 +694,10 @@ impl<'tcx> CoroutineArgs<'tcx> { #[inline] pub fn variant_range(&self, def_id: DefId, tcx: TyCtxt<'tcx>) -> Range { // FIXME requires optimized MIR - FIRST_VARIANT..tcx.coroutine_layout(def_id).unwrap().variant_fields.next_index() + // FIXME(async_closures): We should assert all coroutine layouts have + // the same number of variants. + FIRST_VARIANT + ..tcx.coroutine_layout(def_id, tcx.types.unit).unwrap().variant_fields.next_index() } /// The discriminant for the given variant. Panics if the `variant_index` is @@ -754,7 +757,7 @@ impl<'tcx> CoroutineArgs<'tcx> { def_id: DefId, tcx: TyCtxt<'tcx>, ) -> impl Iterator> + Captures<'tcx>> { - let layout = tcx.coroutine_layout(def_id).unwrap(); + let layout = tcx.coroutine_layout(def_id, self.kind_ty()).unwrap(); layout.variant_fields.iter().map(move |variant| { variant.iter().map(move |field| { ty::EarlyBinder::bind(layout.field_tys[*field].ty).instantiate(tcx, self.args) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 85ac6071a08..331970ac362 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -745,7 +745,7 @@ fn coroutine_layout<'tcx>( let tcx = cx.tcx; let instantiate_field = |ty: Ty<'tcx>| EarlyBinder::bind(ty).instantiate(tcx, args); - let Some(info) = tcx.coroutine_layout(def_id) else { + let Some(info) = tcx.coroutine_layout(def_id, args.as_coroutine().kind_ty()) else { return Err(error(cx, LayoutError::Unknown(ty))); }; let (ineligible_locals, assignments) = coroutine_saved_local_eligibility(info); @@ -1072,7 +1072,7 @@ fn variant_info_for_coroutine<'tcx>( return (vec![], None); }; - let coroutine = cx.tcx.coroutine_layout(def_id).unwrap(); + let coroutine = cx.tcx.coroutine_layout(def_id, args.as_coroutine().kind_ty()).unwrap(); let upvar_names = cx.tcx.closure_saved_names_of_captured_variables(def_id); let mut upvars_size = Size::ZERO; -- cgit 1.4.1-3-g733a5 From 5ddc4f24cc66d02431fcfc6a4cb32c040c0b511c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 22 Mar 2024 14:33:58 +1100 Subject: coverage: Inline creating a dummy instance for unused functions --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index ee7ea342301..28ff3ecceda 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -385,8 +385,7 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { }; debug!("generating unused fn: {def_id:?}"); - let instance = declare_unused_fn(tcx, def_id); - add_unused_function_coverage(cx, instance, function_coverage_info); + add_unused_function_coverage(cx, def_id, function_coverage_info); } } @@ -421,8 +420,15 @@ fn codegenned_and_inlined_items(tcx: TyCtxt<'_>) -> DefIdSet { result } -fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::Instance<'tcx> { - ty::Instance::new( +fn add_unused_function_coverage<'tcx>( + cx: &CodegenCx<'_, 'tcx>, + def_id: DefId, + function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo, +) { + let tcx = cx.tcx; + + // Make a dummy instance that fills in all generics with placeholders. + let instance = ty::Instance::new( def_id, ty::GenericArgs::for_item(tcx, def_id, |param, _| { if let ty::GenericParamDefKind::Lifetime = param.kind { @@ -431,14 +437,8 @@ fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::Instance<'tc 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 = FunctionCoverageCollector::unused(instance, function_coverage_info); -- cgit 1.4.1-3-g733a5 From e3f66b24933356e5bda6a7161ab92706cd015dea Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 22 Mar 2024 12:59:49 +1100 Subject: coverage: Overhaul the search for unused functions --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 130 +++++++++++---------- 1 file changed, 68 insertions(+), 62 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 28ff3ecceda..95cd4cca4d2 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -6,9 +6,8 @@ use crate::llvm; use itertools::Itertools as _; use rustc_codegen_ssa::traits::{BaseTypeMethods, ConstMethods}; -use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; -use rustc_hir::def::DefKind; -use rustc_hir::def_id::DefId; +use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_index::IndexVec; use rustc_middle::bug; use rustc_middle::mir; @@ -335,16 +334,9 @@ fn save_function_record( ); } -/// When finalizing the coverage map, `FunctionCoverage` only has the `CodeRegion`s and counters for -/// the functions that went through codegen; such as public functions and "used" functions -/// (functions referenced by other "used" or public items). Any other functions considered unused, -/// or "Unreachable", were still parsed and processed through the MIR stage, but were not -/// codegenned. (Note that `-Clink-dead-code` can force some unused code to be codegenned, but -/// that flag is known to cause other errors, when combined with `-C instrument-coverage`; and -/// `-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 (`codegenned_and_inlined_items`). +/// Each CGU will normally only emit coverage metadata for the functions that it actually generates. +/// But since we don't want unused functions to disappear from coverage reports, we also scan for +/// functions that were instrumented but are not participating in codegen. /// /// 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. @@ -354,78 +346,92 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu()); let tcx = cx.tcx; + let usage = prepare_usage_sets(tcx); + + let is_unused_fn = |def_id: LocalDefId| -> bool { + let def_id = def_id.to_def_id(); + + // To be eligible for "unused function" mappings, a definition must: + // - Be function-like + // - Not participate directly in codegen + // - Not have any coverage statements inlined into codegenned functions + tcx.def_kind(def_id).is_fn_like() + && !usage.all_mono_items.contains(&def_id) + && !usage.used_via_inlining.contains(&def_id) + }; - let eligible_def_ids = tcx.mir_keys(()).iter().filter_map(|local_def_id| { - let def_id = local_def_id.to_def_id(); - let kind = tcx.def_kind(def_id); - // `mir_keys` will give us `DefId`s for all kinds of things, not - // just "functions", like consts, statics, etc. Filter those out. - if !matches!(kind, DefKind::Fn | DefKind::AssocFn | DefKind::Closure) { - return None; - } + // Scan for unused functions that were instrumented for coverage. + for def_id in tcx.mir_keys(()).iter().copied().filter(|&def_id| is_unused_fn(def_id)) { + // Get the coverage info from MIR, skipping functions that were never instrumented. + let body = tcx.optimized_mir(def_id); + let Some(function_coverage_info) = body.function_coverage_info.as_deref() else { continue }; // FIXME(79651): Consider trying to filter out dummy instantiations of // unused generic functions from library crates, because they can produce // "unused instantiation" in coverage reports even when they are actually // used by some downstream crate in the same binary. - Some(local_def_id.to_def_id()) - }); - - 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. - for def_id in eligible_def_ids.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(def_id)); - let Some(function_coverage_info) = body.function_coverage_info.as_deref() else { - continue; - }; - debug!("generating unused fn: {def_id:?}"); add_unused_function_coverage(cx, def_id, function_coverage_info); } } -/// 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()); - } - } - } +struct UsageSets<'tcx> { + all_mono_items: &'tcx DefIdSet, + used_via_inlining: FxHashSet, +} + +/// Prepare sets of definitions that are relevant to deciding whether something +/// is an "unused function" for coverage purposes. +fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> { + let (all_mono_items, cgus) = tcx.collect_and_partition_mono_items(()); + + // Obtain a MIR body for each function participating in codegen, via an + // arbitrary instance. + let mut def_ids_seen = FxHashSet::default(); + let def_and_mir_for_all_mono_fns = cgus + .iter() + .flat_map(|cgu| cgu.items().keys()) + .filter_map(|item| match item { + mir::mono::MonoItem::Fn(instance) => Some(instance), + mir::mono::MonoItem::Static(_) | mir::mono::MonoItem::GlobalAsm(_) => None, + }) + // We only need one arbitrary instance per definition. + .filter(move |instance| def_ids_seen.insert(instance.def_id())) + .map(|instance| { + // We don't care about the instance, just its underlying MIR. + let body = tcx.instance_mir(instance.def); + (instance.def_id(), body) + }); + + // Functions whose coverage statments were found inlined into other functions. + let mut used_via_inlining = FxHashSet::default(); + + for (_def_id, body) in def_and_mir_for_all_mono_fns { + // Inspect every coverage statement in the function's MIR. + for stmt in body + .basic_blocks + .iter() + .flat_map(|block| &block.statements) + .filter(|stmt| matches!(stmt.kind, mir::StatementKind::Coverage(_))) + { + if let Some(inlined) = stmt.source_info.scope.inlined_instance(&body.source_scopes) { + // This coverage statement was inlined from another function. + used_via_inlining.insert(inlined.def_id()); } } } - result + UsageSets { all_mono_items, used_via_inlining } } fn add_unused_function_coverage<'tcx>( cx: &CodegenCx<'_, 'tcx>, - def_id: DefId, + def_id: LocalDefId, function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo, ) { let tcx = cx.tcx; + let def_id = def_id.to_def_id(); // Make a dummy instance that fills in all generics with placeholders. let instance = ty::Instance::new( -- cgit 1.4.1-3-g733a5 From 54116c8cae58f1d46f231fbeac5630a81b994a4c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 22 Mar 2024 13:23:04 +1100 Subject: coverage: Detect functions that have lost all their coverage statements If a function was instrumented for coverage, but all of its coverage statements have been removed by later MIR transforms, it should be treated as "unused" even if the compiler generates an unreachable stub for it. --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 95cd4cca4d2..0fbc624389b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -353,10 +353,11 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { // To be eligible for "unused function" mappings, a definition must: // - Be function-like - // - Not participate directly in codegen + // - Not participate directly in codegen (or have lost all its coverage statements) // - Not have any coverage statements inlined into codegenned functions tcx.def_kind(def_id).is_fn_like() - && !usage.all_mono_items.contains(&def_id) + && (!usage.all_mono_items.contains(&def_id) + || usage.missing_own_coverage.contains(&def_id)) && !usage.used_via_inlining.contains(&def_id) }; @@ -379,6 +380,7 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { struct UsageSets<'tcx> { all_mono_items: &'tcx DefIdSet, used_via_inlining: FxHashSet, + missing_own_coverage: FxHashSet, } /// Prepare sets of definitions that are relevant to deciding whether something @@ -406,8 +408,13 @@ fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> { // Functions whose coverage statments were found inlined into other functions. let mut used_via_inlining = FxHashSet::default(); + // Functions that were instrumented, but had all of their coverage statements + // removed by later MIR transforms (e.g. UnreachablePropagation). + let mut missing_own_coverage = FxHashSet::default(); + + for (def_id, body) in def_and_mir_for_all_mono_fns { + let mut saw_own_coverage = false; - for (_def_id, body) in def_and_mir_for_all_mono_fns { // Inspect every coverage statement in the function's MIR. for stmt in body .basic_blocks @@ -418,11 +425,18 @@ fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> { if let Some(inlined) = stmt.source_info.scope.inlined_instance(&body.source_scopes) { // This coverage statement was inlined from another function. used_via_inlining.insert(inlined.def_id()); + } else { + // Non-inlined coverage statements belong to the enclosing function. + saw_own_coverage = true; } } + + if !saw_own_coverage && body.function_coverage_info.is_some() { + missing_own_coverage.insert(def_id); + } } - UsageSets { all_mono_items, used_via_inlining } + UsageSets { all_mono_items, used_via_inlining, missing_own_coverage } } fn add_unused_function_coverage<'tcx>( -- cgit 1.4.1-3-g733a5