diff options
26 files changed, 458 insertions, 667 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 763186a58bf..7ad2d03a5ed 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -1,4 +1,4 @@ -use rustc_middle::mir::coverage::{CounterId, ExpressionId, Operand}; +use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId}; /// Must match the layout of `LLVMRustCounterKind`. #[derive(Copy, Clone, Debug)] @@ -43,11 +43,11 @@ impl Counter { Self { kind: CounterKind::Expression, id: expression_id.as_u32() } } - pub(crate) fn from_operand(operand: Operand) -> Self { - match operand { - Operand::Zero => Self::ZERO, - Operand::Counter(id) => Self::counter_value_reference(id), - Operand::Expression(id) => Self::expression(id), + pub(crate) fn from_term(term: CovTerm) -> Self { + match term { + CovTerm::Zero => Self::ZERO, + CovTerm::Counter(id) => Self::counter_value_reference(id), + CovTerm::Expression(id) => Self::expression(id), } } } @@ -73,17 +73,6 @@ pub struct CounterExpression { pub rhs: Counter, } -impl CounterExpression { - /// The dummy expression `(0 - 0)` has a representation of all zeroes, - /// making it marginally more efficient to initialize than `(0 + 0)`. - pub(crate) const DUMMY: Self = - Self { lhs: Counter::ZERO, kind: ExprKind::Subtract, rhs: Counter::ZERO }; - - pub fn new(lhs: Counter, kind: ExprKind, rhs: Counter) -> Self { - Self { kind, lhs, rhs } - } -} - /// Corresponds to enum `llvm::coverage::CounterMappingRegion::RegionKind`. /// /// Must match the layout of `LLVMRustCounterMappingRegionKind`. diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 55f43aa5341..84319b4ba2d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,64 +1,78 @@ use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; use rustc_data_structures::fx::FxIndexSet; -use rustc_index::IndexVec; -use rustc_middle::mir::coverage::{CodeRegion, CounterId, ExpressionId, Op, Operand}; +use rustc_index::bit_set::BitSet; +use rustc_middle::mir::coverage::{ + CodeRegion, CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, Op, +}; use rustc_middle::ty::Instance; -use rustc_middle::ty::TyCtxt; -#[derive(Clone, Debug, PartialEq)] -pub struct Expression { - lhs: Operand, - op: Op, - rhs: Operand, - code_regions: Vec<CodeRegion>, -} - -/// Collects all of the coverage regions associated with (a) injected counters, (b) counter -/// expressions (additions or subtraction), and (c) unreachable regions (always counted as zero), -/// for a given Function. This struct also stores the `function_source_hash`, -/// computed during instrumentation, and forwarded with counters. -/// -/// Note, it may be important to understand LLVM's definitions of `unreachable` regions versus "gap -/// regions" (or "gap areas"). A gap region is a code region within a counted region (either counter -/// or expression), but the line or lines in the gap region are not executable (such as lines with -/// only whitespace or comments). According to LLVM Code Coverage Mapping documentation, "A count -/// for a gap area is only used as the line execution count if there are no other regions on a -/// line." +/// Holds all of the coverage mapping data associated with a function instance, +/// collected during traversal of `Coverage` statements in the function's MIR. #[derive(Debug)] pub struct FunctionCoverage<'tcx> { - instance: Instance<'tcx>, - source_hash: u64, + /// Coverage info that was attached to this function by the instrumentor. + function_coverage_info: &'tcx FunctionCoverageInfo, is_used: bool, - counters: IndexVec<CounterId, Option<Vec<CodeRegion>>>, - expressions: IndexVec<ExpressionId, Option<Expression>>, - unreachable_regions: Vec<CodeRegion>, + + /// Tracks which counters have been seen, so that we can identify mappings + /// to counters that were optimized out, and set them to zero. + counters_seen: BitSet<CounterId>, + /// Contains all expression IDs that have been seen in an `ExpressionUsed` + /// coverage statement, plus all expression IDs that aren't directly used + /// by any mappings (and therefore do not have expression-used statements). + /// After MIR traversal is finished, we can conclude that any IDs missing + /// from this set must have had their statements deleted by MIR opts. + expressions_seen: BitSet<ExpressionId>, } impl<'tcx> FunctionCoverage<'tcx> { /// Creates a new set of coverage data for a used (called) function. - pub fn new(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self { - Self::create(tcx, instance, true) + pub fn new( + instance: Instance<'tcx>, + function_coverage_info: &'tcx FunctionCoverageInfo, + ) -> Self { + Self::create(instance, function_coverage_info, true) } /// Creates a new set of coverage data for an unused (never called) function. - pub fn unused(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self { - Self::create(tcx, instance, false) + pub fn unused( + instance: Instance<'tcx>, + function_coverage_info: &'tcx FunctionCoverageInfo, + ) -> Self { + Self::create(instance, function_coverage_info, false) } - fn create(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, is_used: bool) -> Self { - let coverageinfo = tcx.coverageinfo(instance.def); + fn create( + instance: Instance<'tcx>, + function_coverage_info: &'tcx FunctionCoverageInfo, + is_used: bool, + ) -> Self { + let num_counters = function_coverage_info.num_counters; + let num_expressions = function_coverage_info.expressions.len(); debug!( - "FunctionCoverage::create(instance={:?}) has coverageinfo={:?}. is_used={}", - instance, coverageinfo, is_used + "FunctionCoverage::create(instance={instance:?}) has \ + num_counters={num_counters}, num_expressions={num_expressions}, is_used={is_used}" ); + + // Create a filled set of expression IDs, so that expressions not + // directly used by mappings will be treated as "seen". + // (If they end up being unused, LLVM will delete them for us.) + let mut expressions_seen = BitSet::new_filled(num_expressions); + // For each expression ID that is directly used by one or more mappings, + // mark it as not-yet-seen. This indicates that we expect to see a + // corresponding `ExpressionUsed` statement during MIR traversal. + for Mapping { term, .. } in &function_coverage_info.mappings { + if let &CovTerm::Expression(id) = term { + expressions_seen.remove(id); + } + } + Self { - instance, - source_hash: 0, // will be set with the first `add_counter()` + function_coverage_info, is_used, - counters: IndexVec::from_elem_n(None, coverageinfo.num_counters as usize), - expressions: IndexVec::from_elem_n(None, coverageinfo.num_expressions as usize), - unreachable_regions: Vec::new(), + counters_seen: BitSet::new_empty(num_counters), + expressions_seen, } } @@ -67,135 +81,94 @@ impl<'tcx> FunctionCoverage<'tcx> { self.is_used } - /// Sets the function source hash value. If called multiple times for the same function, all - /// calls should have the same hash value. - pub fn set_function_source_hash(&mut self, source_hash: u64) { - if self.source_hash == 0 { - self.source_hash = source_hash; - } else { - debug_assert_eq!(source_hash, self.source_hash); - } - } - - /// Adds code regions to be counted by an injected counter intrinsic. + /// Marks a counter ID as having been seen in a counter-increment statement. #[instrument(level = "debug", skip(self))] - pub(crate) fn add_counter(&mut self, id: CounterId, code_regions: &[CodeRegion]) { - if code_regions.is_empty() { - return; - } - - let slot = &mut self.counters[id]; - match slot { - None => *slot = Some(code_regions.to_owned()), - // If this counter ID slot has already been filled, it should - // contain identical information. - Some(ref previous_regions) => assert_eq!( - previous_regions, code_regions, - "add_counter: code regions for id changed" - ), - } - } - - /// Adds information about a coverage expression, along with zero or more - /// code regions mapped to that expression. - /// - /// Both counters and "counter expressions" (or simply, "expressions") can be operands in other - /// expressions. These are tracked as separate variants of `Operand`, so there is no ambiguity - /// between operands that are counter IDs and operands that are expression IDs. - #[instrument(level = "debug", skip(self))] - pub(crate) fn add_counter_expression( - &mut self, - expression_id: ExpressionId, - lhs: Operand, - op: Op, - rhs: Operand, - code_regions: &[CodeRegion], - ) { - debug_assert!( - expression_id.as_usize() < self.expressions.len(), - "expression_id {} is out of range for expressions.len() = {} - for {:?}", - expression_id.as_usize(), - self.expressions.len(), - self, - ); - - let expression = Expression { lhs, op, rhs, code_regions: code_regions.to_owned() }; - let slot = &mut self.expressions[expression_id]; - match slot { - None => *slot = Some(expression), - // If this expression ID slot has already been filled, it should - // contain identical information. - Some(ref previous_expression) => assert_eq!( - previous_expression, &expression, - "add_counter_expression: expression for id changed" - ), - } + pub(crate) fn mark_counter_id_seen(&mut self, id: CounterId) { + self.counters_seen.insert(id); } - /// Adds regions that will be marked as "unreachable", with a constant "zero counter". + /// Marks an expression ID as having been seen in an expression-used statement. #[instrument(level = "debug", skip(self))] - pub(crate) fn add_unreachable_regions(&mut self, code_regions: &[CodeRegion]) { - assert!(!code_regions.is_empty(), "unreachable regions always have code regions"); - self.unreachable_regions.extend_from_slice(code_regions); + pub(crate) fn mark_expression_id_seen(&mut self, id: ExpressionId) { + self.expressions_seen.insert(id); } - /// Perform some simplifications to make the final coverage mappings - /// slightly smaller. + /// Identify expressions that will always have a value of zero, and note + /// their IDs in [`ZeroExpressions`]. Mappings that refer to a zero expression + /// can instead become mappings to a constant zero value. /// /// This method mainly exists to preserve the simplifications that were /// already being performed by the Rust-side expression renumbering, so that /// the resulting coverage mappings don't get worse. - pub(crate) fn simplify_expressions(&mut self) { + fn identify_zero_expressions(&self) -> ZeroExpressions { // The set of expressions that either were optimized out entirely, or // have zero as both of their operands, and will therefore always have // a value of zero. Other expressions that refer to these as operands - // can have those operands replaced with `Operand::Zero`. + // can have those operands replaced with `CovTerm::Zero`. let mut zero_expressions = FxIndexSet::default(); - // For each expression, perform simplifications based on lower-numbered - // expressions, and then update the set of always-zero expressions if - // necessary. + // Simplify a copy of each expression based on lower-numbered expressions, + // and then update the set of always-zero expressions if necessary. // (By construction, expressions can only refer to other expressions - // that have lower IDs, so one simplification pass is sufficient.) - for (id, maybe_expression) in self.expressions.iter_enumerated_mut() { - let Some(expression) = maybe_expression else { - // If an expression is missing, it must have been optimized away, + // that have lower IDs, so one pass is sufficient.) + for (id, expression) in self.function_coverage_info.expressions.iter_enumerated() { + if !self.expressions_seen.contains(id) { + // If an expression was not seen, it must have been optimized away, // so any operand that refers to it can be replaced with zero. zero_expressions.insert(id); continue; + } + + // We don't need to simplify the actual expression data in the + // expressions list; we can just simplify a temporary copy and then + // use that to update the set of always-zero expressions. + let Expression { mut lhs, op, mut rhs } = *expression; + + // If an expression has an operand that is also an expression, the + // operand's ID must be strictly lower. This is what lets us find + // all zero expressions in one pass. + let assert_operand_expression_is_lower = |operand_id: ExpressionId| { + assert!( + operand_id < id, + "Operand {operand_id:?} should be less than {id:?} in {expression:?}", + ) }; // If an operand refers to an expression that is always zero, then - // that operand can be replaced with `Operand::Zero`. - let maybe_set_operand_to_zero = |operand: &mut Operand| match &*operand { - Operand::Expression(id) if zero_expressions.contains(id) => { - *operand = Operand::Zero; + // that operand can be replaced with `CovTerm::Zero`. + let maybe_set_operand_to_zero = |operand: &mut CovTerm| match *operand { + CovTerm::Expression(id) => { + assert_operand_expression_is_lower(id); + if zero_expressions.contains(&id) { + *operand = CovTerm::Zero; + } } _ => (), }; - maybe_set_operand_to_zero(&mut expression.lhs); - maybe_set_operand_to_zero(&mut expression.rhs); + maybe_set_operand_to_zero(&mut lhs); + maybe_set_operand_to_zero(&mut rhs); // Coverage counter values cannot be negative, so if an expression // involves subtraction from zero, assume that its RHS must also be zero. // (Do this after simplifications that could set the LHS to zero.) - if let Expression { lhs: Operand::Zero, op: Op::Subtract, .. } = expression { - expression.rhs = Operand::Zero; + if lhs == CovTerm::Zero && op == Op::Subtract { + rhs = CovTerm::Zero; } // After the above simplifications, if both operands are zero, then // we know that this expression is always zero too. - if let Expression { lhs: Operand::Zero, rhs: Operand::Zero, .. } = expression { + if lhs == CovTerm::Zero && rhs == CovTerm::Zero { zero_expressions.insert(id); } } + + ZeroExpressions(zero_expressions) } /// Return the source hash, generated from the HIR node structure, and used to indicate whether /// or not the source code structure changed between different compilations. pub fn source_hash(&self) -> u64 { - self.source_hash + if self.is_used { self.function_coverage_info.function_source_hash } else { 0 } } /// Generate an array of CounterExpressions, and an iterator over all `Counter`s and their @@ -204,91 +177,80 @@ impl<'tcx> FunctionCoverage<'tcx> { pub fn get_expressions_and_counter_regions( &self, ) -> (Vec<CounterExpression>, impl Iterator<Item = (Counter, &CodeRegion)>) { - assert!( - self.source_hash != 0 || !self.is_used, - "No counters provided the source_hash for used function: {:?}", - self.instance - ); + let zero_expressions = self.identify_zero_expressions(); - let counter_expressions = self.counter_expressions(); + let counter_expressions = self.counter_expressions(&zero_expressions); // Expression IDs are indices into `self.expressions`, and on the LLVM // side they will be treated as indices into `counter_expressions`, so // the two vectors should correspond 1:1. - assert_eq!(self.expressions.len(), counter_expressions.len()); + assert_eq!(self.function_coverage_info.expressions.len(), counter_expressions.len()); - let counter_regions = self.counter_regions(); - let expression_regions = self.expression_regions(); - let unreachable_regions = self.unreachable_regions(); + let counter_regions = self.counter_regions(zero_expressions); - let counter_regions = - counter_regions.chain(expression_regions.into_iter().chain(unreachable_regions)); (counter_expressions, counter_regions) } - fn counter_regions(&self) -> impl Iterator<Item = (Counter, &CodeRegion)> { - self.counters - .iter_enumerated() - // Filter out counter IDs that we never saw during MIR traversal. - // This can happen if a counter was optimized out by MIR transforms - // (and replaced with `CoverageKind::Unreachable` instead). - .filter_map(|(id, maybe_code_regions)| Some((id, maybe_code_regions.as_ref()?))) - .flat_map(|(id, code_regions)| { - let counter = Counter::counter_value_reference(id); - code_regions.iter().map(move |region| (counter, region)) - }) - } - /// Convert this function's coverage expression data into a form that can be /// passed through FFI to LLVM. - fn counter_expressions(&self) -> Vec<CounterExpression> { + fn counter_expressions(&self, zero_expressions: &ZeroExpressions) -> Vec<CounterExpression> { // We know that LLVM will optimize out any unused expressions before // producing the final coverage map, so there's no need to do the same // thing on the Rust side unless we're confident we can do much better. // (See `CounterExpressionsMinimizer` in `CoverageMappingWriter.cpp`.) - self.expressions + let counter_from_operand = |operand: CovTerm| match operand { + CovTerm::Expression(id) if zero_expressions.contains(id) => Counter::ZERO, + _ => Counter::from_term(operand), + }; + + self.function_coverage_info + .expressions .iter() - .map(|expression| match expression { - None => { - // This expression ID was allocated, but we never saw the - // actual expression, so it must have been optimized out. - // Replace it with a dummy expression, and let LLVM take - // care of omitting it from the expression list. - CounterExpression::DUMMY - } - &Some(Expression { lhs, op, rhs, .. }) => { - // Convert the operands and operator as normal. - CounterExpression::new( - Counter::from_operand(lhs), - match op { - Op::Add => ExprKind::Add, - Op::Subtract => ExprKind::Subtract, - }, - Counter::from_operand(rhs), - ) - } + .map(|&Expression { lhs, op, rhs }| CounterExpression { + lhs: counter_from_operand(lhs), + kind: match op { + Op::Add => ExprKind::Add, + Op::Subtract => ExprKind::Subtract, + }, + rhs: counter_from_operand(rhs), }) .collect::<Vec<_>>() } - fn expression_regions(&self) -> Vec<(Counter, &CodeRegion)> { - // Find all of the expression IDs that weren't optimized out AND have - // one or more attached code regions, and return the corresponding - // mappings as counter/region pairs. - self.expressions - .iter_enumerated() - .filter_map(|(id, maybe_expression)| { - let code_regions = &maybe_expression.as_ref()?.code_regions; - Some((id, code_regions)) - }) - .flat_map(|(id, code_regions)| { - let counter = Counter::expression(id); - code_regions.iter().map(move |code_region| (counter, code_region)) - }) - .collect::<Vec<_>>() + /// Converts this function's coverage mappings into an intermediate form + /// that will be used by `mapgen` when preparing for FFI. + fn counter_regions( + &self, + zero_expressions: ZeroExpressions, + ) -> impl Iterator<Item = (Counter, &CodeRegion)> { + // Historically, mappings were stored directly in counter/expression + // statements in MIR, and MIR optimizations would sometimes remove them. + // That's mostly no longer true, so now we detect cases where that would + // have happened, and zero out the corresponding mappings here instead. + let counter_for_term = move |term: CovTerm| { + let force_to_zero = match term { + CovTerm::Counter(id) => !self.counters_seen.contains(id), + CovTerm::Expression(id) => zero_expressions.contains(id), + CovTerm::Zero => false, + }; + if force_to_zero { Counter::ZERO } else { Counter::from_term(term) } + }; + + self.function_coverage_info.mappings.iter().map(move |mapping| { + let &Mapping { term, ref code_region } = mapping; + let counter = counter_for_term(term); + (counter, code_region) + }) } +} + +/// Set of expression IDs that are known to always evaluate to zero. +/// Any mapping or expression operand that refers to these expressions can have +/// that reference replaced with a constant zero value. +struct ZeroExpressions(FxIndexSet<ExpressionId>); - fn unreachable_regions(&self) -> impl Iterator<Item = (Counter, &CodeRegion)> { - self.unreachable_regions.iter().map(|region| (Counter::ZERO, region)) +impl ZeroExpressions { + fn contains(&self, id: ExpressionId) -> bool { + self.0.contains(&id) } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index d4e77525698..ef3647efd88 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -10,9 +10,8 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_index::IndexVec; use rustc_middle::bug; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::coverage::CodeRegion; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{self, TyCtxt}; use rustc_span::Symbol; /// Generates and exports the Coverage Map. @@ -60,10 +59,8 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) { // Encode coverage mappings and generate function records let mut function_data = Vec::new(); - for (instance, mut function_coverage) in function_coverage_map { + for (instance, function_coverage) in function_coverage_map { debug!("Generate function coverage for {}, {:?}", cx.codegen_unit.name(), instance); - function_coverage.simplify_expressions(); - let function_coverage = function_coverage; let mangled_function_name = tcx.symbol_name(instance).name; let source_hash = function_coverage.source_hash(); @@ -170,10 +167,11 @@ fn encode_mappings_for_function( let mut virtual_file_mapping = IndexVec::<u32, u32>::new(); let mut mapping_regions = Vec::with_capacity(counter_regions.len()); - // Sort the list of (counter, region) mapping pairs by region, so that they - // can be grouped by filename. Prepare file IDs for each filename, and - // prepare the mapping data so that we can pass it through FFI to LLVM. - counter_regions.sort_by_key(|(_counter, region)| *region); + // Sort and group the list of (counter, region) mapping pairs by filename. + // (Preserve any further ordering imposed by `FunctionCoverage`.) + // Prepare file IDs for each filename, and prepare the mapping data so that + // we can pass it through FFI to LLVM. + counter_regions.sort_by_key(|(_counter, region)| region.file_name); for counter_regions_for_file in counter_regions.group_by(|(_, a), (_, b)| a.file_name == b.file_name) { @@ -331,16 +329,14 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { for non_codegenned_def_id in eligible_def_ids.into_iter().filter(|id| !codegenned_def_ids.contains(id)) { - let codegen_fn_attrs = tcx.codegen_fn_attrs(non_codegenned_def_id); - - // If a function is marked `#[coverage(off)]`, then skip generating a - // dead code stub for it. - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { - debug!("skipping unused fn marked #[coverage(off)]: {:?}", non_codegenned_def_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 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); + cx.define_unused_fn(non_codegenned_def_id, function_coverage_info); } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index dd2ce9b525b..204a73b788a 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -16,7 +16,7 @@ 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}; +use rustc_middle::mir::coverage::{CounterId, CoverageKind, FunctionCoverageInfo}; use rustc_middle::mir::Coverage; use rustc_middle::ty; use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt}; @@ -88,44 +88,63 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { /// 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 coverage `CodeRegion`s from the MIR and add - /// them. Since the function is never called, all of its `CodeRegion`s can be - /// added as `unreachable_region`s. - fn define_unused_fn(&self, def_id: DefId) { + /// 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); - add_unused_function_coverage(self, instance, def_id); + add_unused_function_coverage(self, instance, function_coverage_info); } } impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { + #[instrument(level = "debug", skip(self))] fn add_coverage(&mut self, instance: Instance<'tcx>, coverage: &Coverage) { + // Our caller should have already taken care of inlining subtleties, + // so we can assume that counter/expression IDs in this coverage + // statement are meaningful for the given instance. + // + // (Either the statement was not inlined and directly belongs to this + // instance, or it was inlined *from* this instance.) + let bx = self; + let Some(function_coverage_info) = + bx.tcx.instance_mir(instance.def).function_coverage_info.as_deref() + else { + debug!("function has a coverage statement but no coverage info"); + return; + }; + let Some(coverage_context) = bx.coverage_context() else { return }; let mut coverage_map = coverage_context.function_coverage_map.borrow_mut(); let func_coverage = coverage_map .entry(instance) - .or_insert_with(|| FunctionCoverage::new(bx.tcx(), instance)); + .or_insert_with(|| FunctionCoverage::new(instance, function_coverage_info)); - let Coverage { kind, code_regions } = coverage; + let Coverage { kind } = coverage; match *kind { - CoverageKind::Counter { function_source_hash, id } => { - debug!( - "ensuring function source hash is set for instance={:?}; function_source_hash={}", - instance, function_source_hash, - ); - func_coverage.set_function_source_hash(function_source_hash); - func_coverage.add_counter(id, code_regions); + CoverageKind::CounterIncrement { id } => { + func_coverage.mark_counter_id_seen(id); // We need to explicitly drop the `RefMut` before calling into `instrprof_increment`, // as that needs an exclusive borrow. drop(coverage_map); - let coverageinfo = bx.tcx().coverageinfo(instance.def); + // The number of counters passed to `llvm.instrprof.increment` might + // be smaller than the number originally inserted by the instrumentor, + // if some high-numbered counters were removed by MIR optimizations. + // If so, LLVM's profiler runtime will use fewer physical counters. + let num_counters = + bx.tcx().coverage_ids_info(instance.def).max_counter_id.as_u32() + 1; + assert!( + num_counters as usize <= function_coverage_info.num_counters, + "num_counters disagreement: query says {num_counters} but function info only has {}", + function_coverage_info.num_counters + ); let fn_name = bx.get_pgo_func_name_var(instance); - let hash = bx.const_u64(function_source_hash); - let num_counters = bx.const_u32(coverageinfo.num_counters); + let hash = bx.const_u64(function_coverage_info.function_source_hash); + let num_counters = bx.const_u32(num_counters); let index = bx.const_u32(id.as_u32()); debug!( "codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})", @@ -133,11 +152,8 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { ); bx.instrprof_increment(fn_name, hash, num_counters, index); } - CoverageKind::Expression { id, lhs, op, rhs } => { - func_coverage.add_counter_expression(id, lhs, op, rhs, code_regions); - } - CoverageKind::Unreachable => { - func_coverage.add_unreachable_regions(code_regions); + CoverageKind::ExpressionUsed { id } => { + func_coverage.mark_expression_id_seen(id); } } } @@ -200,15 +216,11 @@ fn codegen_unused_fn_and_counter<'tcx>(cx: &CodegenCx<'_, 'tcx>, instance: Insta fn add_unused_function_coverage<'tcx>( cx: &CodegenCx<'_, 'tcx>, instance: Instance<'tcx>, - def_id: DefId, + function_coverage_info: &'tcx FunctionCoverageInfo, ) { - let tcx = cx.tcx; - - let mut function_coverage = FunctionCoverage::unused(tcx, instance); - for &code_region in tcx.covered_code_regions(def_id) { - let code_region = std::slice::from_ref(code_region); - function_coverage.add_unreachable_regions(code_region); - } + // 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); diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index a6d6f6f5df4..08d377a8695 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -1,5 +1,6 @@ //! Metadata from source code coverage analysis and instrumentation. +use rustc_index::IndexVec; use rustc_macros::HashStable; use rustc_span::Symbol; @@ -8,6 +9,11 @@ use std::fmt::{self, Debug, Formatter}; rustc_index::newtype_index! { /// ID of a coverage counter. Values ascend from 0. /// + /// Before MIR inlining, counter IDs are local to their enclosing function. + /// After MIR inlining, coverage statements may have been inlined into + /// another function, so use the statement's source-scope to find which + /// function/instance its IDs are meaningful for. + /// /// Note that LLVM handles counter IDs as `uint32_t`, so there is no need /// to use a larger representation on the Rust side. #[derive(HashStable)] @@ -23,6 +29,11 @@ impl CounterId { rustc_index::newtype_index! { /// ID of a coverage-counter expression. Values ascend from 0. /// + /// Before MIR inlining, expression IDs are local to their enclosing function. + /// After MIR inlining, coverage statements may have been inlined into + /// another function, so use the statement's source-scope to find which + /// function/instance its IDs are meaningful for. + /// /// Note that LLVM handles expression IDs as `uint32_t`, so there is no need /// to use a larger representation on the Rust side. #[derive(HashStable)] @@ -35,19 +46,21 @@ impl ExpressionId { pub const START: Self = Self::from_u32(0); } -/// Operand of a coverage-counter expression. +/// Enum that can hold a constant zero value, the ID of an physical coverage +/// counter, or the ID of a coverage-counter expression. /// -/// Operands can be a constant zero value, an actual coverage counter, or another -/// expression. Counter/expression operands are referred to by ID. +/// This was originally only used for expression operands (and named `Operand`), +/// but the zero/counter/expression distinction is also useful for representing +/// the value of code/gap mappings, and the true/false arms of branch mappings. #[derive(Copy, Clone, PartialEq, Eq)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] -pub enum Operand { +pub enum CovTerm { Zero, Counter(CounterId), Expression(ExpressionId), } -impl Debug for Operand { +impl Debug for CovTerm { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { Self::Zero => write!(f, "Zero"), @@ -59,40 +72,31 @@ impl Debug for Operand { #[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub enum CoverageKind { - Counter { - function_source_hash: u64, - /// ID of this counter within its enclosing function. - /// Expressions in the same function can refer to it as an operand. - id: CounterId, - }, - Expression { - /// ID of this coverage-counter expression within its enclosing function. - /// Other expressions in the same function can refer to it as an operand. - id: ExpressionId, - lhs: Operand, - op: Op, - rhs: Operand, - }, - Unreachable, + /// Marks the point in MIR control flow represented by a coverage counter. + /// + /// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR. + /// + /// If this statement does not survive MIR optimizations, any mappings that + /// refer to this counter can have those references simplified to zero. + CounterIncrement { id: CounterId }, + + /// Marks the point in MIR control-flow represented by a coverage expression. + /// + /// If this statement does not survive MIR optimizations, any mappings that + /// refer to this expression can have those references simplified to zero. + /// + /// (This is only inserted for expression IDs that are directly used by + /// mappings. Intermediate expressions with no direct mappings are + /// retained/zeroed based on whether they are transitively used.) + ExpressionUsed { id: ExpressionId }, } impl Debug for CoverageKind { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { use CoverageKind::*; match self { - Counter { id, .. } => write!(fmt, "Counter({:?})", id.index()), - Expression { id, lhs, op, rhs } => write!( - fmt, - "Expression({:?}) = {:?} {} {:?}", - id.index(), - lhs, - match op { - Op::Add => "+", - Op::Subtract => "-", - }, - rhs, - ), - Unreachable => write!(fmt, "Unreachable"), + CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), + ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), } } } @@ -133,3 +137,38 @@ impl Op { matches!(self, Self::Subtract) } } + +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct Expression { + pub lhs: CovTerm, + pub op: Op, + pub rhs: CovTerm, +} + +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct Mapping { + pub code_region: CodeRegion, + + /// Indicates whether this mapping uses a counter value, expression value, + /// or zero value. + /// + /// FIXME: When we add support for mapping kinds other than `Code` + /// (e.g. branch regions, expansion regions), replace this with a dedicated + /// mapping-kind enum. + pub term: CovTerm, +} + +/// Stores per-function coverage information attached to a `mir::Body`, +/// to be used in conjunction with the individual coverage statements injected +/// into the function's basic blocks. +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct FunctionCoverageInfo { + pub function_source_hash: u64, + pub num_counters: usize, + + pub expressions: IndexVec<ExpressionId, Expression>, + pub mappings: Vec<Mapping>, +} diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index f979f736b15..3a5ff4dc91f 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -345,6 +345,14 @@ pub struct Body<'tcx> { pub injection_phase: Option<MirPhase>, pub tainted_by_errors: Option<ErrorGuaranteed>, + + /// Per-function coverage information added by the `InstrumentCoverage` + /// pass, to be used in conjunction with the coverage statements injected + /// into this body's blocks. + /// + /// If `-Cinstrument-coverage` is not active, or if an individual function + /// is not eligible for coverage, then this should always be `None`. + pub function_coverage_info: Option<Box<coverage::FunctionCoverageInfo>>, } impl<'tcx> Body<'tcx> { @@ -392,6 +400,7 @@ impl<'tcx> Body<'tcx> { is_polymorphic: false, injection_phase: None, tainted_by_errors, + function_coverage_info: None, }; body.is_polymorphic = body.has_non_region_param(); body @@ -420,6 +429,7 @@ impl<'tcx> Body<'tcx> { is_polymorphic: false, injection_phase: None, tainted_by_errors: None, + function_coverage_info: None, }; body.is_polymorphic = body.has_non_region_param(); body diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index c5e3ee575e1..3b3b61e4e21 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -493,6 +493,27 @@ pub fn write_mir_intro<'tcx>( // Add an empty line before the first block is printed. writeln!(w)?; + if let Some(function_coverage_info) = &body.function_coverage_info { + write_function_coverage_info(function_coverage_info, w)?; + } + + Ok(()) +} + +fn write_function_coverage_info( + function_coverage_info: &coverage::FunctionCoverageInfo, + w: &mut dyn io::Write, +) -> io::Result<()> { + let coverage::FunctionCoverageInfo { expressions, mappings, .. } = function_coverage_info; + + for (id, expression) in expressions.iter_enumerated() { + writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?; + } + for coverage::Mapping { term, code_region } in mappings { + writeln!(w, "{INDENT}coverage {term:?} => {code_region:?};")?; + } + writeln!(w)?; + Ok(()) } @@ -685,13 +706,7 @@ impl Debug for Statement<'_> { AscribeUserType(box (ref place, ref c_ty), ref variance) => { write!(fmt, "AscribeUserType({place:?}, {variance:?}, {c_ty:?})") } - Coverage(box mir::Coverage { ref kind, ref code_regions }) => { - if code_regions.is_empty() { - write!(fmt, "Coverage::{kind:?}") - } else { - write!(fmt, "Coverage::{kind:?} for {code_regions:?}") - } - } + Coverage(box mir::Coverage { ref kind }) => write!(fmt, "Coverage::{kind:?}"), Intrinsic(box ref intrinsic) => write!(fmt, "{intrinsic}"), ConstEvalCounter => write!(fmt, "ConstEvalCounter"), Nop => write!(fmt, "nop"), diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index c74a9536b63..f407dc4d7ae 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -1,5 +1,6 @@ //! Values computed by queries that use MIR. +use crate::mir; use crate::ty::{self, OpaqueHiddenType, Ty, TyCtxt}; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::unord::UnordSet; @@ -445,14 +446,19 @@ pub struct DestructuredConstant<'tcx> { pub fields: &'tcx [(ConstValue<'tcx>, Ty<'tcx>)], } -/// Coverage information summarized from a MIR if instrumented for source code coverage (see -/// compiler option `-Cinstrument-coverage`). This information is generated by the -/// `InstrumentCoverage` MIR pass and can be retrieved via the `coverageinfo` query. +/// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass +/// (for compiler option `-Cinstrument-coverage`), after MIR optimizations +/// have had a chance to potentially remove some of them. +/// +/// Used by the `coverage_ids_info` query. #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)] -pub struct CoverageInfo { - /// The total number of coverage region counters added to the MIR `Body`. - pub num_counters: u32, - - /// The total number of coverage region counter expressions added to the MIR `Body`. - pub num_expressions: u32, +pub struct CoverageIdsInfo { + /// Coverage codegen needs to know the highest counter ID that is ever + /// incremented within a function, so that it can set the `num-counters` + /// argument of the `llvm.instrprof.increment` intrinsic. + /// + /// This may be less than the highest counter ID emitted by the + /// InstrumentCoverage MIR pass, if the highest-numbered counter increments + /// were removed by MIR optimizations. + pub max_counter_id: mir::coverage::CounterId, } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index c7d99648f1e..7a645fb5d62 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -5,7 +5,7 @@ use super::{BasicBlock, Const, Local, UserTypeProjection}; -use crate::mir::coverage::{CodeRegion, CoverageKind}; +use crate::mir::coverage::CoverageKind; use crate::traits::Reveal; use crate::ty::adjustment::PointerCoercion; use crate::ty::GenericArgsRef; @@ -361,11 +361,16 @@ pub enum StatementKind<'tcx> { /// Disallowed after drop elaboration. AscribeUserType(Box<(Place<'tcx>, UserTypeProjection)>, ty::Variance), - /// Marks the start of a "coverage region", injected with '-Cinstrument-coverage'. A - /// `Coverage` statement carries metadata about the coverage region, used to inject a coverage - /// map into the binary. If `Coverage::kind` is a `Counter`, the statement also generates - /// executable code, to increment a counter variable at runtime, each time the code region is - /// executed. + /// Carries control-flow-sensitive information injected by `-Cinstrument-coverage`, + /// such as where to generate physical coverage-counter-increments during codegen. + /// + /// Coverage statements are used in conjunction with the coverage mappings and other + /// information stored in the function's + /// [`mir::Body::function_coverage_info`](crate::mir::Body::function_coverage_info). + /// (For inlined MIR, take care to look up the *original function's* coverage info.) + /// + /// Interpreters and codegen backends that don't support coverage instrumentation + /// can usually treat this as a no-op. Coverage(Box<Coverage>), /// Denotes a call to an intrinsic that does not require an unwind path and always returns. @@ -514,7 +519,6 @@ pub enum FakeReadCause { #[derive(TypeFoldable, TypeVisitable)] pub struct Coverage { pub kind: CoverageKind, - pub code_regions: Vec<CodeRegion>, } #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)] diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 940ab190ffc..afe94d10752 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -573,24 +573,14 @@ rustc_queries! { separate_provide_extern } - /// Returns coverage summary info for a function, after executing the `InstrumentCoverage` - /// MIR pass (assuming the -Cinstrument-coverage option is enabled). - query coverageinfo(key: ty::InstanceDef<'tcx>) -> &'tcx mir::CoverageInfo { - desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key.def_id()) } + /// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass + /// (for compiler option `-Cinstrument-coverage`), after MIR optimizations + /// have had a chance to potentially remove some of them. + query coverage_ids_info(key: ty::InstanceDef<'tcx>) -> &'tcx mir::CoverageIdsInfo { + desc { |tcx| "retrieving coverage IDs info from MIR for `{}`", tcx.def_path_str(key.def_id()) } arena_cache } - /// Returns the `CodeRegions` for a function that has instrumented coverage, in case the - /// function was optimized out before codegen, and before being added to the Coverage Map. - query covered_code_regions(key: DefId) -> &'tcx Vec<&'tcx mir::coverage::CodeRegion> { - desc { - |tcx| "retrieving the covered `CodeRegion`s, if instrumented, for `{}`", - tcx.def_path_str(key) - } - arena_cache - cache_on_disk_if { key.is_local() } - } - /// The `DefId` is the `DefId` of the containing MIR body. Promoteds do not have their own /// `DefId`. This function returns all promoteds in the specified body. The body references /// promoteds by the `DefId` and the `mir::Promoted` index. This is necessary, because diff --git a/compiler/rustc_mir_build/src/build/custom/mod.rs b/compiler/rustc_mir_build/src/build/custom/mod.rs index e5c2cc6c7bb..a81f70e3346 100644 --- a/compiler/rustc_mir_build/src/build/custom/mod.rs +++ b/compiler/rustc_mir_build/src/build/custom/mod.rs @@ -60,6 +60,7 @@ pub(super) fn build_custom_mir<'tcx>( tainted_by_errors: None, injection_phase: None, pass_count: 0, + function_coverage_info: None, }; body.local_decls.push(LocalDecl::new(return_ty, return_ty_span)); diff --git a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs index 4500bb7ff0f..74243f1f8f2 100644 --- a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs +++ b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs @@ -113,6 +113,6 @@ impl<'tcx> MirPass<'tcx> for AbortUnwindingCalls { } // We may have invalidated some `cleanup` blocks so clean those up now. - super::simplify::remove_dead_blocks(tcx, body); + super::simplify::remove_dead_blocks(body); } } diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 77e3dee1fef..a83ccf8fc3c 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -19,7 +19,7 @@ const NESTED_INDENT: &str = " "; #[derive(Clone)] pub(super) enum BcbCounter { Counter { id: CounterId }, - Expression { id: ExpressionId, lhs: Operand, op: Op, rhs: Operand }, + Expression { id: ExpressionId }, } impl BcbCounter { @@ -27,10 +27,10 @@ impl BcbCounter { matches!(self, Self::Expression { .. }) } - pub(super) fn as_operand(&self) -> Operand { + pub(super) fn as_term(&self) -> CovTerm { match *self { - BcbCounter::Counter { id, .. } => Operand::Counter(id), - BcbCounter::Expression { id, .. } => Operand::Expression(id), + BcbCounter::Counter { id, .. } => CovTerm::Counter(id), + BcbCounter::Expression { id, .. } => CovTerm::Expression(id), } } } @@ -39,17 +39,7 @@ impl Debug for BcbCounter { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Counter { id, .. } => write!(fmt, "Counter({:?})", id.index()), - Self::Expression { id, lhs, op, rhs } => write!( - fmt, - "Expression({:?}) = {:?} {} {:?}", - id.index(), - lhs, - match op { - Op::Add => "+", - Op::Subtract => "-", - }, - rhs, - ), + Self::Expression { id } => write!(fmt, "Expression({:?})", id.index()), } } } @@ -58,7 +48,6 @@ impl Debug for BcbCounter { /// associated with nodes/edges in the BCB graph. pub(super) struct CoverageCounters { next_counter_id: CounterId, - next_expression_id: ExpressionId, /// Coverage counters/expressions that are associated with individual BCBs. bcb_counters: IndexVec<BasicCoverageBlock, Option<BcbCounter>>, @@ -69,10 +58,9 @@ pub(super) struct CoverageCounters { /// Only used by debug assertions, to verify that BCBs with incoming edge /// counters do not have their own physical counters (expressions are allowed). bcb_has_incoming_edge_counters: BitSet<BasicCoverageBlock>, - /// Expression nodes that are not directly associated with any particular - /// BCB/edge, but are needed as operands to more complex expressions. - /// These are always [`BcbCounter::Expression`]. - pub(super) intermediate_expressions: Vec<BcbCounter>, + /// Table of expression data, associating each expression ID with its + /// corresponding operator (+ or -) and its LHS/RHS operands. + expressions: IndexVec<ExpressionId, Expression>, } impl CoverageCounters { @@ -81,12 +69,10 @@ impl CoverageCounters { Self { next_counter_id: CounterId::START, - next_expression_id: ExpressionId::START, - bcb_counters: IndexVec::from_elem_n(None, num_bcbs), bcb_edge_counters: FxHashMap::default(), bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs), - intermediate_expressions: Vec::new(), + expressions: IndexVec::new(), } } @@ -106,9 +92,9 @@ impl CoverageCounters { BcbCounter::Counter { id } } - fn make_expression(&mut self, lhs: Operand, op: Op, rhs: Operand) -> BcbCounter { - let id = self.next_expression(); - BcbCounter::Expression { id, lhs, op, rhs } + fn make_expression(&mut self, lhs: CovTerm, op: Op, rhs: CovTerm) -> BcbCounter { + let id = self.expressions.push(Expression { lhs, op, rhs }); + BcbCounter::Expression { id } } /// Counter IDs start from one and go up. @@ -118,19 +104,20 @@ impl CoverageCounters { next } - /// Expression IDs start from 0 and go up. - /// (Counter IDs and Expression IDs are distinguished by the `Operand` enum.) - fn next_expression(&mut self) -> ExpressionId { - let next = self.next_expression_id; - self.next_expression_id = self.next_expression_id + 1; - next + pub(super) fn num_counters(&self) -> usize { + self.next_counter_id.as_usize() + } + + #[cfg(test)] + pub(super) fn num_expressions(&self) -> usize { + self.expressions.len() } fn set_bcb_counter( &mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter, - ) -> Result<Operand, Error> { + ) -> Result<CovTerm, Error> { debug_assert!( // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also // have an expression (to be injected into an existing `BasicBlock` represented by this @@ -138,14 +125,14 @@ impl CoverageCounters { counter_kind.is_expression() || !self.bcb_has_incoming_edge_counters.contains(bcb), "attempt to add a `Counter` to a BCB target with existing incoming edge counters" ); - let operand = counter_kind.as_operand(); + let term = counter_kind.as_term(); if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { Error::from_string(format!( "attempt to set a BasicCoverageBlock coverage counter more than once; \ {bcb:?} already had counter {replaced:?}", )) } else { - Ok(operand) + Ok(term) } } @@ -154,7 +141,7 @@ impl CoverageCounters { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, counter_kind: BcbCounter, - ) -> Result<Operand, Error> { + ) -> Result<CovTerm, Error> { if level_enabled!(tracing::Level::DEBUG) { // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also // have an expression (to be injected into an existing `BasicBlock` represented by this @@ -167,14 +154,14 @@ impl CoverageCounters { } } self.bcb_has_incoming_edge_counters.insert(to_bcb); - let operand = counter_kind.as_operand(); + let term = counter_kind.as_term(); if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) { Error::from_string(format!( "attempt to set an edge counter more than once; from_bcb: \ {from_bcb:?} already had counter {replaced:?}", )) } else { - Ok(operand) + Ok(term) } } @@ -199,6 +186,10 @@ impl CoverageCounters { ) -> impl Iterator<Item = ((BasicCoverageBlock, BasicCoverageBlock), BcbCounter)> + '_ { self.bcb_edge_counters.drain() } + + pub(super) fn take_expressions(&mut self) -> IndexVec<ExpressionId, Expression> { + std::mem::take(&mut self.expressions) + } } /// Traverse the `CoverageGraph` and add either a `Counter` or `Expression` to every BCB, to be @@ -276,7 +267,7 @@ impl<'a> MakeBcbCounters<'a> { &mut self, traversal: &TraverseCoverageGraphWithLoops<'_>, branching_bcb: BasicCoverageBlock, - branching_counter_operand: Operand, + branching_counter_operand: CovTerm, ) -> Result<(), Error> { let branches = self.bcb_branches(branching_bcb); debug!( @@ -324,8 +315,7 @@ impl<'a> MakeBcbCounters<'a> { sumup_counter_operand, ); debug!(" [new intermediate expression: {:?}]", intermediate_expression); - let intermediate_expression_operand = intermediate_expression.as_operand(); - self.coverage_counters.intermediate_expressions.push(intermediate_expression); + let intermediate_expression_operand = intermediate_expression.as_term(); some_sumup_counter_operand.replace(intermediate_expression_operand); } } @@ -356,7 +346,7 @@ impl<'a> MakeBcbCounters<'a> { Ok(()) } - fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result<Operand, Error> { + fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result<CovTerm, Error> { self.recursive_get_or_make_counter_operand(bcb, 1) } @@ -364,7 +354,7 @@ impl<'a> MakeBcbCounters<'a> { &mut self, bcb: BasicCoverageBlock, debug_indent_level: usize, - ) -> Result<Operand, Error> { + ) -> Result<CovTerm, Error> { // If the BCB already has a counter, return it. if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] { debug!( @@ -373,7 +363,7 @@ impl<'a> MakeBcbCounters<'a> { bcb, counter_kind, ); - return Ok(counter_kind.as_operand()); + return Ok(counter_kind.as_term()); } // A BCB with only one incoming edge gets a simple `Counter` (via `make_counter()`). @@ -437,8 +427,7 @@ impl<'a> MakeBcbCounters<'a> { NESTED_INDENT.repeat(debug_indent_level), intermediate_expression ); - let intermediate_expression_operand = intermediate_expression.as_operand(); - self.coverage_counters.intermediate_expressions.push(intermediate_expression); + let intermediate_expression_operand = intermediate_expression.as_term(); some_sumup_edge_counter_operand.replace(intermediate_expression_operand); } } @@ -460,7 +449,7 @@ impl<'a> MakeBcbCounters<'a> { &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - ) -> Result<Operand, Error> { + ) -> Result<CovTerm, Error> { self.recursive_get_or_make_edge_counter_operand(from_bcb, to_bcb, 1) } @@ -469,7 +458,7 @@ impl<'a> MakeBcbCounters<'a> { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, debug_indent_level: usize, - ) -> Result<Operand, Error> { + ) -> Result<CovTerm, Error> { // If the source BCB has only one successor (assumed to be the given target), an edge // counter is unnecessary. Just get or make a counter for the source BCB. let successors = self.bcb_successors(from_bcb).iter(); @@ -488,7 +477,7 @@ impl<'a> MakeBcbCounters<'a> { to_bcb, counter_kind ); - return Ok(counter_kind.as_operand()); + return Ok(counter_kind.as_term()); } // Make a new counter to count this edge. diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index abf13519e9e..df4dccf0f0b 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -104,6 +104,7 @@ struct Instrumentor<'a, 'tcx> { function_source_hash: u64, basic_coverage_blocks: CoverageGraph, coverage_counters: CoverageCounters, + mappings: Vec<Mapping>, } impl<'a, 'tcx> Instrumentor<'a, 'tcx> { @@ -144,6 +145,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { function_source_hash, basic_coverage_blocks, coverage_counters, + mappings: Vec::new(), } } @@ -165,9 +167,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock` // and all `Expression` dependencies (operands) are also generated, for any other // `BasicCoverageBlock`s not already associated with a coverage span. - // - // Intermediate expressions (used to compute other `Expression` values), which have no - // direct association with any `BasicCoverageBlock`, are accumulated inside `coverage_counters`. let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb); let result = self .coverage_counters @@ -193,24 +192,18 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { // are in fact counted, even though they don't directly contribute to counting // their own independent code region's coverage. self.inject_indirect_counters(); - - // Intermediate expressions will be injected as the final step, after generating - // debug output, if any. - //////////////////////////////////////////////////// }; if let Err(e) = result { bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e.message) }; - //////////////////////////////////////////////////// - // Finally, inject the intermediate expressions collected along the way. - for intermediate_expression in &self.coverage_counters.intermediate_expressions { - inject_intermediate_expression( - self.mir_body, - self.make_mir_coverage_kind(intermediate_expression), - ); - } + self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { + function_source_hash: self.function_source_hash, + num_counters: self.coverage_counters.num_counters(), + expressions: self.coverage_counters.take_expressions(), + mappings: std::mem::take(&mut self.mappings), + })); } /// Injects a single [`StatementKind::Coverage`] for each BCB that has one @@ -226,18 +219,16 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { bug!("Every BasicCoverageBlock should have a Counter or Expression"); }); - // Convert the coverage spans into a vector of code regions to be - // associated with this BCB's coverage statement. - let code_regions = spans - .iter() - .map(|&span| make_code_region(source_map, file_name, span, body_span)) - .collect::<Vec<_>>(); + let term = counter_kind.as_term(); + self.mappings.extend(spans.iter().map(|&span| { + let code_region = make_code_region(source_map, file_name, span, body_span); + Mapping { code_region, term } + })); inject_statement( self.mir_body, self.make_mir_coverage_kind(&counter_kind), self.bcb_leader_bb(bcb), - code_regions, ); } } @@ -295,13 +286,10 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { self.mir_body, self.make_mir_coverage_kind(&counter_kind), inject_to_bb, - Vec::new(), ); } - BcbCounter::Expression { .. } => inject_intermediate_expression( - self.mir_body, - self.make_mir_coverage_kind(&counter_kind), - ), + // Experessions with no associated spans don't need to inject a statement. + BcbCounter::Expression { .. } => {} } } } @@ -323,12 +311,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind { match *counter_kind { - BcbCounter::Counter { id } => { - CoverageKind::Counter { function_source_hash: self.function_source_hash, id } - } - BcbCounter::Expression { id, lhs, op, rhs } => { - CoverageKind::Expression { id, lhs, op, rhs } - } + BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id }, + BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id }, } } } @@ -356,39 +340,17 @@ fn inject_edge_counter_basic_block( new_bb } -fn inject_statement( - mir_body: &mut mir::Body<'_>, - counter_kind: CoverageKind, - bb: BasicBlock, - code_regions: Vec<CodeRegion>, -) { - debug!(" injecting statement {counter_kind:?} for {bb:?} at code regions: {code_regions:?}"); +fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb: BasicBlock) { + debug!(" injecting statement {counter_kind:?} for {bb:?}"); let data = &mut mir_body[bb]; let source_info = data.terminator().source_info; let statement = Statement { source_info, - kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind, code_regions })), + kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind })), }; data.statements.insert(0, statement); } -// Non-code expressions are injected into the coverage map, without generating executable code. -fn inject_intermediate_expression(mir_body: &mut mir::Body<'_>, expression: CoverageKind) { - debug_assert!(matches!(expression, CoverageKind::Expression { .. })); - debug!(" injecting non-code expression {:?}", expression); - let inject_in_bb = mir::START_BLOCK; - let data = &mut mir_body[inject_in_bb]; - let source_info = data.terminator().source_info; - let statement = Statement { - source_info, - kind: StatementKind::Coverage(Box::new(Coverage { - kind: expression, - code_regions: Vec::new(), - })), - }; - data.statements.push(statement); -} - /// Convert the Span into its file name, start line and column, and end line and column fn make_code_region( source_map: &SourceMap, diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 2c0164e765c..809407f897d 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -2,100 +2,31 @@ use super::*; use rustc_data_structures::captures::Captures; use rustc_middle::mir::coverage::*; -use rustc_middle::mir::{self, Body, Coverage, CoverageInfo}; +use rustc_middle::mir::{Body, Coverage, CoverageIdsInfo}; use rustc_middle::query::Providers; use rustc_middle::ty::{self, TyCtxt}; -use rustc_span::def_id::DefId; /// A `query` provider for retrieving coverage information injected into MIR. pub(crate) fn provide(providers: &mut Providers) { - providers.coverageinfo = |tcx, def_id| coverageinfo(tcx, def_id); - providers.covered_code_regions = |tcx, def_id| covered_code_regions(tcx, def_id); + providers.coverage_ids_info = |tcx, def_id| coverage_ids_info(tcx, def_id); } -/// Coverage codegen needs to know the total number of counter IDs and expression IDs that have -/// been used by a function's coverage mappings. These totals are used to create vectors to hold -/// the relevant counter and expression data, and the maximum counter ID (+ 1) is also needed by -/// the `llvm.instrprof.increment` intrinsic. -/// -/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code -/// including injected counters. (It is OK if some counters are optimized out, but those counters -/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the -/// calls may not work; but computing the number of counters or expressions by adding `1` to the -/// highest ID (for a given instrumented function) is valid. -/// -/// It's possible for a coverage expression to remain in MIR while one or both of its operands -/// have been optimized away. To avoid problems in codegen, we include those operands' IDs when -/// determining the maximum counter/expression ID, even if the underlying counter/expression is -/// no longer present. -struct CoverageVisitor { - max_counter_id: CounterId, - max_expression_id: ExpressionId, -} - -impl CoverageVisitor { - /// Updates `max_counter_id` to the maximum encountered counter ID. - #[inline(always)] - fn update_max_counter_id(&mut self, counter_id: CounterId) { - self.max_counter_id = self.max_counter_id.max(counter_id); - } - - /// Updates `max_expression_id` to the maximum encountered expression ID. - #[inline(always)] - fn update_max_expression_id(&mut self, expression_id: ExpressionId) { - self.max_expression_id = self.max_expression_id.max(expression_id); - } - - fn update_from_expression_operand(&mut self, operand: Operand) { - match operand { - Operand::Counter(id) => self.update_max_counter_id(id), - Operand::Expression(id) => self.update_max_expression_id(id), - Operand::Zero => {} - } - } - - fn visit_body(&mut self, body: &Body<'_>) { - for coverage in all_coverage_in_mir_body(body) { - self.visit_coverage(coverage); - } - } - - fn visit_coverage(&mut self, coverage: &Coverage) { - match coverage.kind { - CoverageKind::Counter { id, .. } => self.update_max_counter_id(id), - CoverageKind::Expression { id, lhs, rhs, .. } => { - self.update_max_expression_id(id); - self.update_from_expression_operand(lhs); - self.update_from_expression_operand(rhs); - } - CoverageKind::Unreachable => {} - } - } -} - -fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) -> CoverageInfo { +/// Query implementation for `coverage_ids_info`. +fn coverage_ids_info<'tcx>( + tcx: TyCtxt<'tcx>, + instance_def: ty::InstanceDef<'tcx>, +) -> CoverageIdsInfo { let mir_body = tcx.instance_mir(instance_def); - let mut coverage_visitor = CoverageVisitor { - max_counter_id: CounterId::START, - max_expression_id: ExpressionId::START, - }; - - coverage_visitor.visit_body(mir_body); - - // Add 1 to the highest IDs to get the total number of IDs. - CoverageInfo { - num_counters: (coverage_visitor.max_counter_id + 1).as_u32(), - num_expressions: (coverage_visitor.max_expression_id + 1).as_u32(), - } -} + let max_counter_id = all_coverage_in_mir_body(mir_body) + .filter_map(|coverage| match coverage.kind { + CoverageKind::CounterIncrement { id } => Some(id), + _ => None, + }) + .max() + .unwrap_or(CounterId::START); -fn covered_code_regions(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<&CodeRegion> { - let body = mir_body(tcx, def_id); - all_coverage_in_mir_body(body) - // Coverage statements have a list of code regions (possibly empty). - .flat_map(|coverage| coverage.code_regions.as_slice()) - .collect() + CoverageIdsInfo { max_counter_id } } fn all_coverage_in_mir_body<'a, 'tcx>( @@ -115,11 +46,3 @@ fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { let scope_data = &body.source_scopes[statement.source_info.scope]; scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() } - -/// This function ensures we obtain the correct MIR for the given item irrespective of -/// whether that means const mir or runtime mir. For `const fn` this opts for runtime -/// mir. -fn mir_body(tcx: TyCtxt<'_>, def_id: DefId) -> &mir::Body<'_> { - let def = ty::InstanceDef::Item(def_id); - tcx.instance_mir(def) -} diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 487d2282364..795cbce963d 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -656,7 +656,7 @@ fn test_make_bcb_counters() { coverage_counters .make_bcb_counters(&mut basic_coverage_blocks, bcb_has_coverage_spans) .expect("should be Ok"); - assert_eq!(coverage_counters.intermediate_expressions.len(), 0); + assert_eq!(coverage_counters.num_expressions(), 0); let_bcb!(1); assert_eq!( diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index d9a132e5cf1..99b070c018e 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -244,7 +244,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { if round_count != 0 { // Merging can introduce overlap between moved arguments and/or call destination in an // unreachable code, which validator considers to be ill-formed. - remove_dead_blocks(tcx, body); + remove_dead_blocks(body); } trace!(round_count); diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index c16f07a453c..a6693519e54 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -1088,7 +1088,7 @@ fn create_generator_drop_shim<'tcx>( // Make sure we remove dead blocks to remove // unrelated code from the resume part of the function - simplify::remove_dead_blocks(tcx, &mut body); + simplify::remove_dead_blocks(&mut body); // Update the body's def to become the drop glue. // This needs to be updated before the AbortUnwindingCalls pass. @@ -1276,7 +1276,7 @@ fn create_generator_resume_function<'tcx>( // Make sure we remove dead blocks to remove // unrelated code from the drop part of the function - simplify::remove_dead_blocks(tcx, body); + simplify::remove_dead_blocks(body); pm::run_passes_no_validate(tcx, body, &[&abort_unwinding_calls::AbortUnwindingCalls], None); diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index a70371172f7..8f578b69694 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -63,7 +63,7 @@ impl<'tcx> MirPass<'tcx> for Inline { if inline(tcx, body) { debug!("running simplify cfg on {:?}", body.source); CfgSimplifier::new(body).simplify(); - remove_dead_blocks(tcx, body); + remove_dead_blocks(body); deref_finder(tcx, body); } } diff --git a/compiler/rustc_mir_transform/src/multiple_return_terminators.rs b/compiler/rustc_mir_transform/src/multiple_return_terminators.rs index c97d034544a..c9b42e75cb2 100644 --- a/compiler/rustc_mir_transform/src/multiple_return_terminators.rs +++ b/compiler/rustc_mir_transform/src/multiple_return_terminators.rs @@ -38,6 +38,6 @@ impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators { } } - simplify::remove_dead_blocks(tcx, body) + simplify::remove_dead_blocks(body) } } diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 73dae044355..88c89e106fd 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -28,10 +28,8 @@ //! return. use crate::MirPass; -use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; -use rustc_index::bit_set::BitSet; +use rustc_data_structures::fx::FxIndexSet; use rustc_index::{Idx, IndexSlice, IndexVec}; -use rustc_middle::mir::coverage::*; use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; @@ -68,7 +66,7 @@ impl SimplifyCfg { pub fn simplify_cfg<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { CfgSimplifier::new(body).simplify(); remove_duplicate_unreachable_blocks(tcx, body); - remove_dead_blocks(tcx, body); + remove_dead_blocks(body); // FIXME: Should probably be moved into some kind of pass manager body.basic_blocks_mut().raw.shrink_to_fit(); @@ -337,7 +335,7 @@ pub fn remove_duplicate_unreachable_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut B } } -pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { +pub fn remove_dead_blocks(body: &mut Body<'_>) { let reachable = traversal::reachable_as_bitset(body); let num_blocks = body.basic_blocks.len(); if num_blocks == reachable.count() { @@ -345,10 +343,6 @@ pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { } let basic_blocks = body.basic_blocks.as_mut(); - let source_scopes = &body.source_scopes; - if tcx.sess.instrument_coverage() { - save_unreachable_coverage(basic_blocks, source_scopes, &reachable); - } let mut replacements: Vec<_> = (0..num_blocks).map(BasicBlock::new).collect(); let mut orig_index = 0; @@ -370,99 +364,6 @@ pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { } } -/// Some MIR transforms can determine at compile time that a sequences of -/// statements will never be executed, so they can be dropped from the MIR. -/// For example, an `if` or `else` block that is guaranteed to never be executed -/// because its condition can be evaluated at compile time, such as by const -/// evaluation: `if false { ... }`. -/// -/// Those statements are bypassed by redirecting paths in the CFG around the -/// `dead blocks`; but with `-C instrument-coverage`, the dead blocks usually -/// include `Coverage` statements representing the Rust source code regions to -/// be counted at runtime. Without these `Coverage` statements, the regions are -/// lost, and the Rust source code will show no coverage information. -/// -/// What we want to show in a coverage report is the dead code with coverage -/// counts of `0`. To do this, we need to save the code regions, by injecting -/// `Unreachable` coverage statements. These are non-executable statements whose -/// code regions are still recorded in the coverage map, representing regions -/// with `0` executions. -/// -/// If there are no live `Counter` `Coverage` statements remaining, we remove -/// `Coverage` statements along with the dead blocks. Since at least one -/// counter per function is required by LLVM (and necessary, to add the -/// `function_hash` to the counter's call to the LLVM intrinsic -/// `instrprof.increment()`). -/// -/// The `generator::StateTransform` MIR pass and MIR inlining can create -/// atypical conditions, where all live `Counter`s are dropped from the MIR. -/// -/// With MIR inlining we can have coverage counters belonging to different -/// instances in a single body, so the strategy described above is applied to -/// coverage counters from each instance individually. -fn save_unreachable_coverage( - basic_blocks: &mut IndexSlice<BasicBlock, BasicBlockData<'_>>, - source_scopes: &IndexSlice<SourceScope, SourceScopeData<'_>>, - reachable: &BitSet<BasicBlock>, -) { - // Identify instances that still have some live coverage counters left. - let mut live = FxHashSet::default(); - for bb in reachable.iter() { - let basic_block = &basic_blocks[bb]; - for statement in &basic_block.statements { - let StatementKind::Coverage(coverage) = &statement.kind else { continue }; - let CoverageKind::Counter { .. } = coverage.kind else { continue }; - let instance = statement.source_info.scope.inlined_instance(source_scopes); - live.insert(instance); - } - } - - for bb in reachable.iter() { - let block = &mut basic_blocks[bb]; - for statement in &mut block.statements { - let StatementKind::Coverage(_) = &statement.kind else { continue }; - let instance = statement.source_info.scope.inlined_instance(source_scopes); - if !live.contains(&instance) { - statement.make_nop(); - } - } - } - - if live.is_empty() { - return; - } - - // Retain coverage for instances that still have some live counters left. - let mut retained_coverage = Vec::new(); - for dead_block in basic_blocks.indices() { - if reachable.contains(dead_block) { - continue; - } - let dead_block = &basic_blocks[dead_block]; - for statement in &dead_block.statements { - let StatementKind::Coverage(coverage) = &statement.kind else { continue }; - if coverage.code_regions.is_empty() { - continue; - }; - let instance = statement.source_info.scope.inlined_instance(source_scopes); - if live.contains(&instance) { - retained_coverage.push((statement.source_info, coverage.code_regions.clone())); - } - } - } - - let start_block = &mut basic_blocks[START_BLOCK]; - start_block.statements.extend(retained_coverage.into_iter().map( - |(source_info, code_regions)| Statement { - source_info, - kind: StatementKind::Coverage(Box::new(Coverage { - kind: CoverageKind::Unreachable, - code_regions, - })), - }, - )); -} - pub enum SimplifyLocals { BeforeConstProp, Final, diff --git a/compiler/rustc_mir_transform/src/unreachable_prop.rs b/compiler/rustc_mir_transform/src/unreachable_prop.rs index 0b9311a20ef..ea7aafd866b 100644 --- a/compiler/rustc_mir_transform/src/unreachable_prop.rs +++ b/compiler/rustc_mir_transform/src/unreachable_prop.rs @@ -65,7 +65,7 @@ impl MirPass<'_> for UnreachablePropagation { } if replaced { - simplify::remove_dead_blocks(tcx, body); + simplify::remove_dead_blocks(body); } } } diff --git a/tests/coverage-map/status-quo/issue-84561.cov-map b/tests/coverage-map/status-quo/issue-84561.cov-map index fe098fd396c..01fa7ec573c 100644 --- a/tests/coverage-map/status-quo/issue-84561.cov-map +++ b/tests/coverage-map/status-quo/issue-84561.cov-map @@ -1,11 +1,10 @@ Function name: <issue_84561::Foo as core::cmp::PartialEq>::eq -Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 0a, 00, 13, 00, 00, 0a, 00, 13] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 0a, 00, 13] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 -Number of file 0 mappings: 2 +Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 4, 10) to (start + 0, 19) -- Code(Zero) at (prev + 0, 10) to (start + 0, 19) Function name: <issue_84561::Foo as core::fmt::Debug>::fmt Raw bytes (29): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 89, 01, 09, 00, 25, 05, 00, 25, 00, 26, 02, 01, 09, 00, 0f, 07, 01, 05, 00, 06] diff --git a/tests/coverage-map/status-quo/try_error_result.cov-map b/tests/coverage-map/status-quo/try_error_result.cov-map index d254ba7b3b6..8367103a21a 100644 --- a/tests/coverage-map/status-quo/try_error_result.cov-map +++ b/tests/coverage-map/status-quo/try_error_result.cov-map @@ -1,62 +1,45 @@ Function name: <try_error_result::Thing1>::get_thing_2 -Raw bytes (63): 0x[01, 01, 02, 01, 05, 05, 02, 0b, 01, 28, 05, 01, 18, 05, 02, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 02, 02, 0d, 00, 1a, 00, 00, 0d, 00, 1a, 00, 00, 0d, 00, 1a, 07, 02, 05, 00, 06] +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 28, 05, 01, 18, 05, 02, 0d, 00, 14, 02, 02, 0d, 00, 1a, 07, 02, 05, 00, 06] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 11 +Number of file 0 mappings: 4 - Code(Counter(0)) at (prev + 40, 5) to (start + 1, 24) - Code(Counter(1)) at (prev + 2, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) - Code(Expression(0, Sub)) at (prev + 2, 13) to (start + 0, 26) = (c0 - c1) -- Code(Zero) at (prev + 0, 13) to (start + 0, 26) -- Code(Zero) at (prev + 0, 13) to (start + 0, 26) - Code(Expression(1, Add)) at (prev + 2, 5) to (start + 0, 6) = (c1 + (c0 - c1)) Function name: <try_error_result::Thing2>::call -Raw bytes (63): 0x[01, 01, 02, 01, 05, 05, 02, 0b, 01, 33, 05, 01, 18, 05, 02, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 00, 00, 0d, 00, 14, 02, 02, 0d, 00, 13, 00, 00, 0d, 00, 13, 00, 00, 0d, 00, 13, 00, 00, 0d, 00, 13, 07, 02, 05, 00, 06] +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 33, 05, 01, 18, 05, 02, 0d, 00, 14, 02, 02, 0d, 00, 13, 07, 02, 05, 00, 06] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 11 +Number of file 0 mappings: 4 - Code(Counter(0)) at (prev + 51, 5) to (start + 1, 24) - Code(Counter(1)) at (prev + 2, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) -- Code(Zero) at (prev + 0, 13) to (start + 0, 20) - Code(Expression(0, Sub)) at (prev + 2, 13) to (start + 0, 19) = (c0 - c1) -- Code(Zero) at (prev + 0, 13) to (start + 0, 19) -- Code(Zero) at (prev + 0, 13) to (start + 0, 19) -- Code(Zero) at (prev + 0, 13) to (start + 0, 19) - Code(Expression(1, Add)) at (prev + 2, 5) to (start + 0, 6) = (c1 + (c0 - c1)) Function name: try_error_result::call -Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 04, 01, 01, 14, 05, 02, 09, 00, 10, 00, 00, 09, 00, 10, 00, 00, 09, 00, 10, 02, 02, 09, 00, 0f, 00, 00, 09, 00, 0f, 07, 02, 01, 00, 02] +Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 04, 01, 01, 14, 05, 02, 09, 00, 10, 02, 02, 09, 00, 0f, 07, 02, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 7 +Number of file 0 mappings: 4 - Code(Counter(0)) at (prev + 4, 1) to (start + 1, 20) - Code(Counter(1)) at (prev + 2, 9) to (start + 0, 16) -- Code(Zero) at (prev + 0, 9) to (start + 0, 16) -- Code(Zero) at (prev + 0, 9) to (start + 0, 16) - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15) = (c0 - c1) -- Code(Zero) at (prev + 0, 9) to (start + 0, 15) - Code(Expression(1, Add)) at (prev + 2, 1) to (start + 0, 2) = (c1 + (c0 - c1)) diff --git a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff index 28a7ffda371..13ff1e284d9 100644 --- a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff @@ -4,8 +4,10 @@ fn bar() -> bool { let mut _0: bool; ++ coverage Counter(0) => /the/src/instrument_coverage.rs:20:1 - 22:2; ++ bb0: { -+ Coverage::Counter(0) for [/the/src/instrument_coverage.rs:20:1 - 22:2]; ++ Coverage::CounterIncrement(0); _0 = const true; return; } diff --git a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff index 9a8caa26307..f5c59c84693 100644 --- a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff @@ -7,13 +7,21 @@ let mut _2: bool; let mut _3: !; ++ coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) }; ++ coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Subtract, rhs: Counter(1) }; ++ coverage Counter(0) => /the/src/instrument_coverage.rs:11:1 - 11:11; ++ coverage Expression(0) => /the/src/instrument_coverage.rs:12:5 - 13:17; ++ coverage Expression(1) => /the/src/instrument_coverage.rs:14:13 - 14:18; ++ coverage Expression(1) => /the/src/instrument_coverage.rs:17:1 - 17:2; ++ coverage Counter(1) => /the/src/instrument_coverage.rs:15:10 - 15:11; ++ bb0: { -+ Coverage::Counter(0) for [/the/src/instrument_coverage.rs:11:1 - 11:11]; ++ Coverage::CounterIncrement(0); goto -> bb1; } bb1: { -+ Coverage::Expression(0) = Counter(0) + Counter(1) for [/the/src/instrument_coverage.rs:12:5 - 13:17]; ++ Coverage::ExpressionUsed(0); falseUnwind -> [real: bb2, unwind: bb6]; } @@ -27,14 +35,14 @@ } bb4: { -+ Coverage::Expression(1) = Expression(0) - Counter(1) for [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2]; ++ Coverage::ExpressionUsed(1); _0 = const (); StorageDead(_2); return; } bb5: { -+ Coverage::Counter(1) for [/the/src/instrument_coverage.rs:15:10 - 15:11]; ++ Coverage::CounterIncrement(1); _1 = const (); StorageDead(_2); goto -> bb1; |