diff options
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs | 11 | ||||
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs | 105 | ||||
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/coverage.rs | 41 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/pretty.rs | 5 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/counters.rs | 47 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/mod.rs | 42 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/tests.rs | 2 | ||||
| -rw-r--r-- | tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff | 6 |
9 files changed, 90 insertions, 173 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 194ed7d661d..7ad2d03a5ed 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -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 e6b39c71453..84319b4ba2d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -2,19 +2,11 @@ use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; use rustc_data_structures::fx::FxIndexSet; use rustc_index::bit_set::BitSet; -use rustc_index::IndexVec; use rustc_middle::mir::coverage::{ - CodeRegion, CounterId, CovTerm, ExpressionId, FunctionCoverageInfo, Mapping, Op, + CodeRegion, CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, Op, }; use rustc_middle::ty::Instance; -#[derive(Clone, Debug, PartialEq)] -pub struct Expression { - lhs: CovTerm, - op: Op, - rhs: CovTerm, -} - /// 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)] @@ -26,7 +18,12 @@ pub struct FunctionCoverage<'tcx> { /// 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>, - expressions: IndexVec<ExpressionId, Option<Expression>>, + /// 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> { @@ -52,16 +49,30 @@ impl<'tcx> FunctionCoverage<'tcx> { is_used: bool, ) -> Self { let num_counters = function_coverage_info.num_counters; - let num_expressions = function_coverage_info.num_expressions; + let num_expressions = function_coverage_info.expressions.len(); debug!( "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 { function_coverage_info, is_used, counters_seen: BitSet::new_empty(num_counters), - expressions: IndexVec::from_elem_n(None, num_expressions), + expressions_seen, } } @@ -76,35 +87,10 @@ impl<'tcx> FunctionCoverage<'tcx> { self.counters_seen.insert(id); } - /// Adds information about a coverage expression. + /// Marks an expression ID as having been seen in an expression-used statement. #[instrument(level = "debug", skip(self))] - pub(crate) fn add_counter_expression( - &mut self, - expression_id: ExpressionId, - lhs: CovTerm, - op: Op, - rhs: CovTerm, - ) { - 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 }; - 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_expression_id_seen(&mut self, id: ExpressionId) { + self.expressions_seen.insert(id); } /// Identify expressions that will always have a value of zero, and note @@ -125,13 +111,13 @@ impl<'tcx> FunctionCoverage<'tcx> { // 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, maybe_expression) in self.expressions.iter_enumerated() { - let Some(expression) = maybe_expression else { - // If an expression is missing, it must have been optimized away, + 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 @@ -197,7 +183,7 @@ impl<'tcx> FunctionCoverage<'tcx> { // 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(zero_expressions); @@ -217,27 +203,16 @@ impl<'tcx> FunctionCoverage<'tcx> { _ => Counter::from_term(operand), }; - self.expressions + 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<_>>() } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 14332927c8d..f6bc4783564 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -145,8 +145,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); + CoverageKind::ExpressionUsed { id } => { + func_coverage.mark_expression_id_seen(id); } } } diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 2587a6eae1a..db578854fdc 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; @@ -68,14 +69,16 @@ pub enum CoverageKind { /// 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 }, - 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: CovTerm, - op: Op, - rhs: CovTerm, - }, + + /// 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 { @@ -83,17 +86,7 @@ impl Debug for CoverageKind { use CoverageKind::*; match self { CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), - Expression { id, lhs, op, rhs } => write!( - fmt, - "Expression({:?}) = {:?} {} {:?}", - id.index(), - lhs, - match op { - Op::Add => "+", - Op::Subtract => "-", - }, - rhs, - ), + ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), } } } @@ -137,6 +130,14 @@ impl Op { #[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, @@ -157,7 +158,7 @@ pub struct Mapping { pub struct FunctionCoverageInfo { pub function_source_hash: u64, pub num_counters: usize, - pub num_expressions: usize, + pub expressions: IndexVec<ExpressionId, Expression>, pub mappings: Vec<Mapping>, } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 11269e749f8..3b3b61e4e21 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -504,8 +504,11 @@ fn write_function_coverage_info( function_coverage_info: &coverage::FunctionCoverageInfo, w: &mut dyn io::Write, ) -> io::Result<()> { - let coverage::FunctionCoverageInfo { mappings, .. } = function_coverage_info; + 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:?};")?; } diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 4140d402c75..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: CovTerm, op: Op, rhs: CovTerm }, + Expression { id: ExpressionId }, } impl BcbCounter { @@ -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(), } } @@ -107,8 +93,8 @@ impl CoverageCounters { } fn make_expression(&mut self, lhs: CovTerm, op: Op, rhs: CovTerm) -> BcbCounter { - let id = self.next_expression(); - BcbCounter::Expression { id, lhs, op, rhs } + let id = self.expressions.push(Expression { lhs, op, rhs }); + BcbCounter::Expression { id } } /// Counter IDs start from one and go up. @@ -118,20 +104,13 @@ 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.next_expression_id.as_usize() + self.expressions.len() } fn set_bcb_counter( @@ -207,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 @@ -333,7 +316,6 @@ impl<'a> MakeBcbCounters<'a> { ); debug!(" [new intermediate expression: {:?}]", intermediate_expression); let intermediate_expression_operand = intermediate_expression.as_term(); - self.coverage_counters.intermediate_expressions.push(intermediate_expression); some_sumup_counter_operand.replace(intermediate_expression_operand); } } @@ -446,7 +428,6 @@ impl<'a> MakeBcbCounters<'a> { intermediate_expression ); let intermediate_expression_operand = intermediate_expression.as_term(); - self.coverage_counters.intermediate_expressions.push(intermediate_expression); some_sumup_edge_counter_operand.replace(intermediate_expression_operand); } } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 078123a7b7f..df4dccf0f0b 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -167,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 @@ -195,29 +192,16 @@ 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(), - num_expressions: self.coverage_counters.num_expressions(), + expressions: self.coverage_counters.take_expressions(), mappings: std::mem::take(&mut self.mappings), })); } @@ -304,10 +288,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { inject_to_bb, ); } - 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 { .. } => {} } } } @@ -330,9 +312,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind { match *counter_kind { BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id }, - BcbCounter::Expression { id, lhs, op, rhs } => { - CoverageKind::Expression { id, lhs, op, rhs } - } + BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id }, } } } @@ -371,20 +351,6 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb 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 })), - }; - 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/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/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff index 5459ecd98c5..f5c59c84693 100644 --- a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff @@ -7,6 +7,8 @@ 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; @@ -19,7 +21,7 @@ } bb1: { -+ Coverage::Expression(0) = Counter(0) + Counter(1); ++ Coverage::ExpressionUsed(0); falseUnwind -> [real: bb2, unwind: bb6]; } @@ -33,7 +35,7 @@ } bb4: { -+ Coverage::Expression(1) = Expression(0) - Counter(1); ++ Coverage::ExpressionUsed(1); _0 = const (); StorageDead(_2); return; |