diff options
| author | Zalathar <Zalathar@users.noreply.github.com> | 2024-12-06 22:33:24 +1100 |
|---|---|---|
| committer | Zalathar <Zalathar@users.noreply.github.com> | 2024-12-08 20:53:39 +1100 |
| commit | 2022ef7f12fe6ebc810ee5731557d23ba763cc96 (patch) | |
| tree | 943bfc4d16b647d2720f3ffaa227abbd03e930d4 | |
| parent | f3f7c20f7b85fa289657b36c30faddff869f8a1f (diff) | |
| download | rust-2022ef7f12fe6ebc810ee5731557d23ba763cc96.tar.gz rust-2022ef7f12fe6ebc810ee5731557d23ba763cc96.zip | |
coverage: Use a query to find counters/expressions that must be zero
This query (`coverage_ids_info`) already determines which counter/expression IDs are unused, so it only takes a little extra effort to also determine which counters/expressions must have a value of zero.
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs | 116 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/coverage.rs | 13 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/query.rs | 105 |
3 files changed, 117 insertions, 117 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 74e0a647b65..95db80f63ee 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,9 +1,7 @@ use rustc_data_structures::captures::Captures; -use rustc_data_structures::fx::FxIndexSet; -use rustc_index::bit_set::BitSet; use rustc_middle::mir::coverage::{ - CounterId, CovTerm, CoverageIdsInfo, Expression, ExpressionId, FunctionCoverageInfo, Mapping, - MappingKind, Op, SourceRegion, + CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, + SourceRegion, }; use rustc_middle::ty::Instance; use tracing::debug; @@ -55,83 +53,10 @@ impl<'tcx> FunctionCoverageCollector<'tcx> { Self { function_coverage_info, ids_info, is_used } } - /// 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. - 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 `CovTerm::Zero`. - let mut zero_expressions = ZeroExpressions::default(); - - // 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 pass is sufficient.) - for (id, expression) in self.function_coverage_info.expressions.iter_enumerated() { - if !self.is_used || !self.ids_info.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 a counter or expression that is always - // zero, then that operand can be replaced with `CovTerm::Zero`. - let maybe_set_operand_to_zero = |operand: &mut CovTerm| { - if let CovTerm::Expression(id) = *operand { - assert_operand_expression_is_lower(id); - } - - if is_zero_term(&self.ids_info.counters_seen, &zero_expressions, *operand) { - *operand = CovTerm::Zero; - } - }; - 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 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 lhs == CovTerm::Zero && rhs == CovTerm::Zero { - zero_expressions.insert(id); - } - } - - zero_expressions - } - pub(crate) fn into_finished(self) -> FunctionCoverage<'tcx> { - let zero_expressions = self.identify_zero_expressions(); let FunctionCoverageCollector { function_coverage_info, ids_info, is_used, .. } = self; - FunctionCoverage { function_coverage_info, ids_info, is_used, zero_expressions } + FunctionCoverage { function_coverage_info, ids_info, is_used } } } @@ -139,8 +64,6 @@ pub(crate) struct FunctionCoverage<'tcx> { pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, ids_info: &'tcx CoverageIdsInfo, is_used: bool, - - zero_expressions: ZeroExpressions, } impl<'tcx> FunctionCoverage<'tcx> { @@ -195,37 +118,6 @@ impl<'tcx> FunctionCoverage<'tcx> { } fn is_zero_term(&self, term: CovTerm) -> bool { - !self.is_used || is_zero_term(&self.ids_info.counters_seen, &self.zero_expressions, term) - } -} - -/// 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. -#[derive(Default)] -struct ZeroExpressions(FxIndexSet<ExpressionId>); - -impl ZeroExpressions { - fn insert(&mut self, id: ExpressionId) { - self.0.insert(id); - } - - fn contains(&self, id: ExpressionId) -> bool { - self.0.contains(&id) - } -} - -/// Returns `true` if the given term is known to have a value of zero, taking -/// into account knowledge of which counters are unused and which expressions -/// are always zero. -fn is_zero_term( - counters_seen: &BitSet<CounterId>, - zero_expressions: &ZeroExpressions, - term: CovTerm, -) -> bool { - match term { - CovTerm::Zero => true, - CovTerm::Counter(id) => !counters_seen.contains(id), - CovTerm::Expression(id) => zero_expressions.contains(id), + !self.is_used || self.ids_info.is_zero_term(term) } } diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index ce87255380b..962176290df 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -320,7 +320,7 @@ pub struct MCDCDecisionSpan { #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)] pub struct CoverageIdsInfo { pub counters_seen: BitSet<CounterId>, - pub expressions_seen: BitSet<ExpressionId>, + pub zero_expressions: BitSet<ExpressionId>, } impl CoverageIdsInfo { @@ -337,4 +337,15 @@ impl CoverageIdsInfo { // used. Fixing this would require adding a renumbering step somewhere. self.counters_seen.last_set_in(..).map_or(0, |max| max.as_u32() + 1) } + + /// Returns `true` if the given term is known to have a value of zero, taking + /// into account knowledge of which counters are unused and which expressions + /// are always zero. + pub fn is_zero_term(&self, term: CovTerm) -> bool { + match term { + CovTerm::Zero => true, + CovTerm::Counter(id) => !self.counters_seen.contains(id), + CovTerm::Expression(id) => self.zero_expressions.contains(id), + } + } } diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index bd1632a666c..edaec3c7965 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -1,7 +1,10 @@ use rustc_data_structures::captures::Captures; use rustc_index::bit_set::BitSet; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; -use rustc_middle::mir::coverage::{CovTerm, CoverageIdsInfo, CoverageKind, MappingKind}; +use rustc_middle::mir::coverage::{ + CounterId, CovTerm, CoverageIdsInfo, CoverageKind, Expression, ExpressionId, + FunctionCoverageInfo, MappingKind, Op, +}; use rustc_middle::mir::{Body, Statement, StatementKind}; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::{self, TyCtxt}; @@ -87,10 +90,10 @@ fn coverage_ids_info<'tcx>( ) -> CoverageIdsInfo { let mir_body = tcx.instance_mir(instance_def); - let Some(fn_cov_info) = mir_body.function_coverage_info.as_ref() else { + let Some(fn_cov_info) = mir_body.function_coverage_info.as_deref() else { return CoverageIdsInfo { counters_seen: BitSet::new_empty(0), - expressions_seen: BitSet::new_empty(0), + zero_expressions: BitSet::new_empty(0), }; }; @@ -123,7 +126,10 @@ fn coverage_ids_info<'tcx>( } } - CoverageIdsInfo { counters_seen, expressions_seen } + let zero_expressions = + identify_zero_expressions(fn_cov_info, &counters_seen, &expressions_seen); + + CoverageIdsInfo { counters_seen, zero_expressions } } fn all_coverage_in_mir_body<'a, 'tcx>( @@ -141,3 +147,94 @@ 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() } + +/// Identify expressions that will always have a value of zero, and note +/// their IDs in a `BitSet`. Mappings that refer to a zero expression +/// can instead become mappings to a constant zero value. +/// +/// This function 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. +fn identify_zero_expressions( + fn_cov_info: &FunctionCoverageInfo, + counters_seen: &BitSet<CounterId>, + expressions_seen: &BitSet<ExpressionId>, +) -> BitSet<ExpressionId> { + // 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 `CovTerm::Zero`. + let mut zero_expressions = BitSet::new_empty(fn_cov_info.expressions.len()); + + // 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 pass is sufficient.) + for (id, expression) in fn_cov_info.expressions.iter_enumerated() { + if !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 a counter or expression that is always + // zero, then that operand can be replaced with `CovTerm::Zero`. + let maybe_set_operand_to_zero = |operand: &mut CovTerm| { + if let CovTerm::Expression(id) = *operand { + assert_operand_expression_is_lower(id); + } + + if is_zero_term(&counters_seen, &zero_expressions, *operand) { + *operand = CovTerm::Zero; + } + }; + 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 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 lhs == CovTerm::Zero && rhs == CovTerm::Zero { + zero_expressions.insert(id); + } + } + + zero_expressions +} + +/// Returns `true` if the given term is known to have a value of zero, taking +/// into account knowledge of which counters are unused and which expressions +/// are always zero. +fn is_zero_term( + counters_seen: &BitSet<CounterId>, + zero_expressions: &BitSet<ExpressionId>, + term: CovTerm, +) -> bool { + match term { + CovTerm::Zero => true, + CovTerm::Counter(id) => !counters_seen.contains(id), + CovTerm::Expression(id) => zero_expressions.contains(id), + } +} |
