From cad50f40e54ff41252a3a33aae166fcdc291aba0 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 14 Aug 2023 11:41:22 +1000 Subject: [PATCH 1/4] coverage: Remove a useless `let () =` --- compiler/rustc_mir_transform/src/coverage/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index d797a6057a7..f46542ef58b 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -676,7 +676,7 @@ fn test_make_bcb_counters() { } } let mut coverage_counters = counters::CoverageCounters::new(0, &basic_coverage_blocks); - let () = coverage_counters + coverage_counters .make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans) .expect("should be Ok"); assert_eq!(coverage_counters.intermediate_expressions.len(), 0); From 629437eec78a56b04c4a2dbc6c85278ec1221a26 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 22 Jul 2023 13:17:33 +1000 Subject: [PATCH 2/4] coverage: Move a debug print into `make_code_region` --- .../rustc_mir_transform/src/coverage/mod.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index e08b6d6f6e8..43a54496db0 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -309,19 +309,14 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { }; graphviz_data.add_bcb_coverage_span_with_counter(bcb, &covspan, &counter_kind); - debug!( - "Calling make_code_region(file_name={}, source_file={:?}, span={}, body_span={})", - file_name, - self.source_file, - source_map.span_to_diagnostic_string(span), - source_map.span_to_diagnostic_string(body_span) - ); + let code_region = + make_code_region(source_map, file_name, &self.source_file, span, body_span); inject_statement( self.mir_body, counter_kind, self.bcb_leader_bb(bcb), - Some(make_code_region(source_map, file_name, &self.source_file, span, body_span)), + Some(code_region), ); } } @@ -498,6 +493,14 @@ fn make_code_region( span: Span, body_span: Span, ) -> CodeRegion { + debug!( + "Called make_code_region(file_name={}, source_file={:?}, span={}, body_span={})", + file_name, + source_file, + source_map.span_to_diagnostic_string(span), + source_map.span_to_diagnostic_string(body_span) + ); + let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); let (end_line, end_col) = if span.hi() == span.lo() { let (end_line, mut end_col) = (start_line, start_col); From fbab055e7704a1cd321628a1896333e0c13ebc2f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 8 Jul 2023 13:43:29 +1000 Subject: [PATCH 3/4] coverage: Give the instrumentor its own counter type, separate from MIR This splits off `BcbCounter` from MIR's `CoverageKind`, allowing the two types to evolve in different directions as necessary. --- compiler/rustc_middle/src/mir/coverage.rs | 15 ---- .../src/coverage/counters.rs | 80 ++++++++++++++----- .../rustc_mir_transform/src/coverage/debug.rs | 69 ++++++++-------- .../rustc_mir_transform/src/coverage/mod.rs | 45 ++++++++--- .../rustc_mir_transform/src/coverage/tests.rs | 5 +- 5 files changed, 129 insertions(+), 85 deletions(-) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index d7d6e3a0086..1efb54bdb08 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -96,21 +96,6 @@ pub enum CoverageKind { Unreachable, } -impl CoverageKind { - pub fn as_operand(&self) -> Operand { - use CoverageKind::*; - match *self { - Counter { id, .. } => Operand::Counter(id), - Expression { id, .. } => Operand::Expression(id), - Unreachable => bug!("Unreachable coverage cannot be part of an expression"), - } - } - - pub fn is_expression(&self) -> bool { - matches!(self, Self::Expression { .. }) - } -} - impl Debug for CoverageKind { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { use CoverageKind::*; diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index d1f2f0c76c8..3eca2610de6 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -14,6 +14,48 @@ use rustc_index::bit_set::BitSet; use rustc_index::IndexVec; use rustc_middle::mir::coverage::*; +use std::fmt::{self, Debug}; + +/// The coverage counter or counter expression associated with a particular +/// BCB node or BCB edge. +#[derive(Clone)] +pub(super) enum BcbCounter { + Counter { function_source_hash: u64, id: CounterId }, + Expression { id: ExpressionId, lhs: Operand, op: Op, rhs: Operand }, +} + +impl BcbCounter { + fn is_expression(&self) -> bool { + matches!(self, Self::Expression { .. }) + } + + pub(super) fn as_operand(&self) -> Operand { + match *self { + BcbCounter::Counter { id, .. } => Operand::Counter(id), + BcbCounter::Expression { id, .. } => Operand::Expression(id), + } + } +} + +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, + ), + } + } +} + /// Generates and stores coverage counter and coverage expression information /// associated with nodes/edges in the BCB graph. pub(super) struct CoverageCounters { @@ -22,18 +64,18 @@ pub(super) struct CoverageCounters { next_expression_id: ExpressionId, /// Coverage counters/expressions that are associated with individual BCBs. - bcb_counters: IndexVec>, + bcb_counters: IndexVec>, /// Coverage counters/expressions that are associated with the control-flow /// edge between two BCBs. - bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), CoverageKind>, + bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, /// Tracks which BCBs have a counter associated with some incoming edge. /// 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, /// Expression nodes that are not directly associated with any particular /// BCB/edge, but are needed as operands to more complex expressions. - /// These are always `CoverageKind::Expression`. - pub(super) intermediate_expressions: Vec, + /// These are always [`BcbCounter::Expression`]. + pub(super) intermediate_expressions: Vec, pub debug_counters: DebugCounters, } @@ -57,12 +99,12 @@ impl CoverageCounters { } /// Activate the `DebugCounters` data structures, to provide additional debug formatting - /// features when formatting `CoverageKind` (counter) values. + /// features when formatting [`BcbCounter`] (counter) values. pub fn enable_debug(&mut self) { self.debug_counters.enable(); } - /// Makes `CoverageKind` `Counter`s and `Expressions` for the `BasicCoverageBlock`s directly or + /// Makes [`BcbCounter`] `Counter`s and `Expressions` for the `BasicCoverageBlock`s directly or /// indirectly associated with `CoverageSpans`, and accumulates additional `Expression`s /// representing intermediate values. pub fn make_bcb_counters( @@ -73,11 +115,11 @@ impl CoverageCounters { MakeBcbCounters::new(self, basic_coverage_blocks).make_bcb_counters(coverage_spans) } - fn make_counter(&mut self, debug_block_label_fn: F) -> CoverageKind + fn make_counter(&mut self, debug_block_label_fn: F) -> BcbCounter where F: Fn() -> Option, { - let counter = CoverageKind::Counter { + let counter = BcbCounter::Counter { function_source_hash: self.function_source_hash, id: self.next_counter(), }; @@ -93,19 +135,19 @@ impl CoverageCounters { op: Op, rhs: Operand, debug_block_label_fn: F, - ) -> CoverageKind + ) -> BcbCounter where F: Fn() -> Option, { let id = self.next_expression(); - let expression = CoverageKind::Expression { id, lhs, op, rhs }; + let expression = BcbCounter::Expression { id, lhs, op, rhs }; if self.debug_counters.is_enabled() { self.debug_counters.add_counter(&expression, (debug_block_label_fn)()); } expression } - pub fn make_identity_counter(&mut self, counter_operand: Operand) -> CoverageKind { + pub fn make_identity_counter(&mut self, counter_operand: Operand) -> BcbCounter { let some_debug_block_label = if self.debug_counters.is_enabled() { self.debug_counters.some_block_label(counter_operand).cloned() } else { @@ -134,7 +176,7 @@ impl CoverageCounters { fn set_bcb_counter( &mut self, bcb: BasicCoverageBlock, - counter_kind: CoverageKind, + counter_kind: BcbCounter, ) -> Result { debug_assert!( // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also @@ -158,7 +200,7 @@ impl CoverageCounters { &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - counter_kind: CoverageKind, + counter_kind: BcbCounter, ) -> Result { if level_enabled!(tracing::Level::DEBUG) { // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also @@ -183,17 +225,17 @@ impl CoverageCounters { } } - pub(super) fn bcb_counter(&self, bcb: BasicCoverageBlock) -> Option<&CoverageKind> { + pub(super) fn bcb_counter(&self, bcb: BasicCoverageBlock) -> Option<&BcbCounter> { self.bcb_counters[bcb].as_ref() } - pub(super) fn take_bcb_counter(&mut self, bcb: BasicCoverageBlock) -> Option { + pub(super) fn take_bcb_counter(&mut self, bcb: BasicCoverageBlock) -> Option { self.bcb_counters[bcb].take() } pub(super) fn drain_bcb_counters( &mut self, - ) -> impl Iterator + '_ { + ) -> impl Iterator + '_ { self.bcb_counters .iter_enumerated_mut() .filter_map(|(bcb, counter)| Some((bcb, counter.take()?))) @@ -201,7 +243,7 @@ impl CoverageCounters { pub(super) fn drain_bcb_edge_counters( &mut self, - ) -> impl Iterator + '_ { + ) -> impl Iterator + '_ { self.bcb_edge_counters.drain() } } @@ -653,7 +695,7 @@ impl<'a> MakeBcbCounters<'a> { self.branch_counter(branch).is_none() } - fn branch_counter(&self, branch: &BcbBranch) -> Option<&CoverageKind> { + fn branch_counter(&self, branch: &BcbBranch) -> Option<&BcbCounter> { let to_bcb = branch.target_bcb; if let Some(from_bcb) = branch.edge_from_bcb { self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) @@ -675,7 +717,7 @@ impl<'a> MakeBcbCounters<'a> { } #[inline] - fn format_counter(&self, counter_kind: &CoverageKind) -> String { + fn format_counter(&self, counter_kind: &BcbCounter) -> String { self.coverage_counters.debug_counters.format_counter(counter_kind) } } diff --git a/compiler/rustc_mir_transform/src/coverage/debug.rs b/compiler/rustc_mir_transform/src/coverage/debug.rs index 083f1f8aef6..af616c498fd 100644 --- a/compiler/rustc_mir_transform/src/coverage/debug.rs +++ b/compiler/rustc_mir_transform/src/coverage/debug.rs @@ -108,7 +108,7 @@ //! recursively, generating labels with nested operations, enclosed in parentheses //! (for example: `bcb2 + (bcb0 - bcb1)`). -use super::counters::CoverageCounters; +use super::counters::{BcbCounter, CoverageCounters}; use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; use super::spans::CoverageSpan; @@ -247,11 +247,11 @@ impl Default for ExpressionFormat { } } -/// If enabled, this struct maintains a map from `CoverageKind` IDs (as `Operand`) to -/// the `CoverageKind` data and optional label (normally, the counter's associated +/// If enabled, this struct maintains a map from `BcbCounter` IDs (as `Operand`) to +/// the `BcbCounter` data and optional label (normally, the counter's associated /// `BasicCoverageBlock` format string, if any). /// -/// Use `format_counter` to convert one of these `CoverageKind` counters to a debug output string, +/// Use `format_counter` to convert one of these `BcbCounter` counters to a debug output string, /// as directed by the `DebugOptions`. This allows the format of counter labels in logs and dump /// files (including the `CoverageGraph` graphviz file) to be changed at runtime, via environment /// variable. @@ -276,7 +276,7 @@ impl DebugCounters { self.some_counters.is_some() } - pub fn add_counter(&mut self, counter_kind: &CoverageKind, some_block_label: Option) { + pub fn add_counter(&mut self, counter_kind: &BcbCounter, some_block_label: Option) { if let Some(counters) = &mut self.some_counters { let id = counter_kind.as_operand(); counters @@ -291,21 +291,20 @@ impl DebugCounters { }) } - pub fn format_counter(&self, counter_kind: &CoverageKind) -> String { + pub fn format_counter(&self, counter_kind: &BcbCounter) -> String { match *counter_kind { - CoverageKind::Counter { .. } => { + BcbCounter::Counter { .. } => { format!("Counter({})", self.format_counter_kind(counter_kind)) } - CoverageKind::Expression { .. } => { + BcbCounter::Expression { .. } => { format!("Expression({})", self.format_counter_kind(counter_kind)) } - CoverageKind::Unreachable { .. } => "Unreachable".to_owned(), } } - fn format_counter_kind(&self, counter_kind: &CoverageKind) -> String { + fn format_counter_kind(&self, counter_kind: &BcbCounter) -> String { let counter_format = &debug_options().counter_format; - if let CoverageKind::Expression { id, lhs, op, rhs } = *counter_kind { + if let BcbCounter::Expression { id, lhs, op, rhs } = *counter_kind { if counter_format.operation { return format!( "{}{} {} {}", @@ -346,7 +345,7 @@ impl DebugCounters { } if let Some(counters) = &self.some_counters { if let Some(DebugCounter { counter_kind, some_block_label }) = counters.get(&operand) { - if let CoverageKind::Expression { .. } = counter_kind { + if let BcbCounter::Expression { .. } = counter_kind { if let Some(label) = some_block_label && debug_options().counter_format.block { return format!( "{}:({})", @@ -366,12 +365,12 @@ impl DebugCounters { /// A non-public support class to `DebugCounters`. #[derive(Debug)] struct DebugCounter { - counter_kind: CoverageKind, + counter_kind: BcbCounter, some_block_label: Option, } impl DebugCounter { - fn new(counter_kind: CoverageKind, some_block_label: Option) -> Self { + fn new(counter_kind: BcbCounter, some_block_label: Option) -> Self { Self { counter_kind, some_block_label } } } @@ -380,9 +379,9 @@ impl DebugCounter { /// a Graphviz (.dot file) representation of the `CoverageGraph`, for debugging purposes. pub(super) struct GraphvizData { some_bcb_to_coverage_spans_with_counters: - Option>>, - some_bcb_to_dependency_counters: Option>>, - some_edge_to_counter: Option>, + Option>>, + some_bcb_to_dependency_counters: Option>>, + some_edge_to_counter: Option>, } impl GraphvizData { @@ -409,7 +408,7 @@ impl GraphvizData { &mut self, bcb: BasicCoverageBlock, coverage_span: &CoverageSpan, - counter_kind: &CoverageKind, + counter_kind: &BcbCounter, ) { if let Some(bcb_to_coverage_spans_with_counters) = self.some_bcb_to_coverage_spans_with_counters.as_mut() @@ -424,7 +423,7 @@ impl GraphvizData { pub fn get_bcb_coverage_spans_with_counters( &self, bcb: BasicCoverageBlock, - ) -> Option<&[(CoverageSpan, CoverageKind)]> { + ) -> Option<&[(CoverageSpan, BcbCounter)]> { if let Some(bcb_to_coverage_spans_with_counters) = self.some_bcb_to_coverage_spans_with_counters.as_ref() { @@ -437,7 +436,7 @@ impl GraphvizData { pub fn add_bcb_dependency_counter( &mut self, bcb: BasicCoverageBlock, - counter_kind: &CoverageKind, + counter_kind: &BcbCounter, ) { if let Some(bcb_to_dependency_counters) = self.some_bcb_to_dependency_counters.as_mut() { bcb_to_dependency_counters @@ -447,7 +446,7 @@ impl GraphvizData { } } - pub fn get_bcb_dependency_counters(&self, bcb: BasicCoverageBlock) -> Option<&[CoverageKind]> { + pub fn get_bcb_dependency_counters(&self, bcb: BasicCoverageBlock) -> Option<&[BcbCounter]> { if let Some(bcb_to_dependency_counters) = self.some_bcb_to_dependency_counters.as_ref() { bcb_to_dependency_counters.get(&bcb).map(Deref::deref) } else { @@ -459,7 +458,7 @@ impl GraphvizData { &mut self, from_bcb: BasicCoverageBlock, to_bb: BasicBlock, - counter_kind: &CoverageKind, + counter_kind: &BcbCounter, ) { if let Some(edge_to_counter) = self.some_edge_to_counter.as_mut() { edge_to_counter @@ -472,7 +471,7 @@ impl GraphvizData { &self, from_bcb: BasicCoverageBlock, to_bb: BasicBlock, - ) -> Option<&CoverageKind> { + ) -> Option<&BcbCounter> { if let Some(edge_to_counter) = self.some_edge_to_counter.as_ref() { edge_to_counter.get(&(from_bcb, to_bb)) } else { @@ -488,7 +487,7 @@ impl GraphvizData { pub(super) struct UsedExpressions { some_used_expression_operands: Option>>, some_unused_expressions: - Option, BasicCoverageBlock)>>, + Option, BasicCoverageBlock)>>, } impl UsedExpressions { @@ -506,16 +505,16 @@ impl UsedExpressions { self.some_used_expression_operands.is_some() } - pub fn add_expression_operands(&mut self, expression: &CoverageKind) { + pub fn add_expression_operands(&mut self, expression: &BcbCounter) { if let Some(used_expression_operands) = self.some_used_expression_operands.as_mut() { - if let CoverageKind::Expression { id, lhs, rhs, .. } = *expression { + if let BcbCounter::Expression { id, lhs, rhs, .. } = *expression { used_expression_operands.entry(lhs).or_insert_with(Vec::new).push(id); used_expression_operands.entry(rhs).or_insert_with(Vec::new).push(id); } } } - pub fn expression_is_used(&self, expression: &CoverageKind) -> bool { + pub fn expression_is_used(&self, expression: &BcbCounter) -> bool { if let Some(used_expression_operands) = self.some_used_expression_operands.as_ref() { used_expression_operands.contains_key(&expression.as_operand()) } else { @@ -525,7 +524,7 @@ impl UsedExpressions { pub fn add_unused_expression_if_not_found( &mut self, - expression: &CoverageKind, + expression: &BcbCounter, edge_from_bcb: Option, target_bcb: BasicCoverageBlock, ) { @@ -540,11 +539,11 @@ impl UsedExpressions { } } - /// Return the list of unused counters (if any) as a tuple with the counter (`CoverageKind`), + /// Return the list of unused counters (if any) as a tuple with the counter (`BcbCounter`), /// optional `from_bcb` (if it was an edge counter), and `target_bcb`. pub fn get_unused_expressions( &self, - ) -> Vec<(CoverageKind, Option, BasicCoverageBlock)> { + ) -> Vec<(BcbCounter, Option, BasicCoverageBlock)> { if let Some(unused_expressions) = self.some_unused_expressions.as_ref() { unused_expressions.clone() } else { @@ -560,7 +559,7 @@ impl UsedExpressions { bcb_counters_without_direct_coverage_spans: &[( Option, BasicCoverageBlock, - CoverageKind, + BcbCounter, )], ) { if self.is_enabled() { @@ -662,7 +661,7 @@ pub(super) fn dump_coverage_graphviz<'tcx>( basic_coverage_blocks: &CoverageGraph, coverage_counters: &CoverageCounters, graphviz_data: &GraphvizData, - intermediate_expressions: &[CoverageKind], + intermediate_expressions: &[BcbCounter], debug_used_expressions: &UsedExpressions, ) { let debug_counters = &coverage_counters.debug_counters; @@ -743,9 +742,9 @@ fn bcb_to_string_sections<'tcx>( coverage_counters: &CoverageCounters, bcb: BasicCoverageBlock, bcb_data: &BasicCoverageBlockData, - some_coverage_spans_with_counters: Option<&[(CoverageSpan, CoverageKind)]>, - some_dependency_counters: Option<&[CoverageKind]>, - some_intermediate_expressions: Option<&[CoverageKind]>, + some_coverage_spans_with_counters: Option<&[(CoverageSpan, BcbCounter)]>, + some_dependency_counters: Option<&[BcbCounter]>, + some_intermediate_expressions: Option<&[BcbCounter]>, ) -> Vec { let debug_counters = &coverage_counters.debug_counters; diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 43a54496db0..39b56d1f00b 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -8,9 +8,9 @@ mod spans; #[cfg(test)] mod tests; -use counters::CoverageCounters; -use graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; -use spans::{CoverageSpan, CoverageSpans}; +use self::counters::{BcbCounter, CoverageCounters}; +use self::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; +use self::spans::{CoverageSpan, CoverageSpans}; use crate::MirPass; @@ -270,8 +270,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { //////////////////////////////////////////////////// // Finally, inject the intermediate expressions collected along the way. - for intermediate_expression in self.coverage_counters.intermediate_expressions.drain(..) { - inject_intermediate_expression(self.mir_body, intermediate_expression); + for intermediate_expression in &self.coverage_counters.intermediate_expressions { + inject_intermediate_expression( + self.mir_body, + self.make_mir_coverage_kind(intermediate_expression), + ); } } @@ -314,7 +317,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { inject_statement( self.mir_body, - counter_kind, + self.make_mir_coverage_kind(&counter_kind), self.bcb_leader_bb(bcb), Some(code_region), ); @@ -362,7 +365,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { ); match counter_kind { - CoverageKind::Counter { .. } => { + BcbCounter::Counter { .. } => { let inject_to_bb = if let Some(from_bcb) = edge_from_bcb { // The MIR edge starts `from_bb` (the outgoing / last BasicBlock in // `from_bcb`) and ends at `to_bb` (the incoming / first BasicBlock in the @@ -395,12 +398,17 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { target_bb }; - inject_statement(self.mir_body, counter_kind, inject_to_bb, None); + inject_statement( + self.mir_body, + self.make_mir_coverage_kind(&counter_kind), + inject_to_bb, + None, + ); } - CoverageKind::Expression { .. } => { - inject_intermediate_expression(self.mir_body, counter_kind) - } - _ => bug!("CoverageKind should be a counter"), + BcbCounter::Expression { .. } => inject_intermediate_expression( + self.mir_body, + self.make_mir_coverage_kind(&counter_kind), + ), } } } @@ -421,9 +429,20 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { } #[inline] - fn format_counter(&self, counter_kind: &CoverageKind) -> String { + fn format_counter(&self, counter_kind: &BcbCounter) -> String { self.coverage_counters.debug_counters.format_counter(counter_kind) } + + fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind { + match *counter_kind { + BcbCounter::Counter { function_source_hash, id } => { + CoverageKind::Counter { function_source_hash, id } + } + BcbCounter::Expression { id, lhs, op, rhs } => { + CoverageKind::Expression { id, lhs, op, rhs } + } + } + } } fn inject_edge_counter_basic_block( diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index f46542ef58b..99ac769be64 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -34,7 +34,6 @@ use itertools::Itertools; use rustc_data_structures::graph::WithNumNodes; use rustc_data_structures::graph::WithSuccessors; use rustc_index::{Idx, IndexVec}; -use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::*; use rustc_middle::ty; use rustc_span::{self, BytePos, Pos, Span, DUMMY_SP}; @@ -685,7 +684,7 @@ fn test_make_bcb_counters() { assert_eq!( 0, // bcb1 has a `Counter` with id = 0 match coverage_counters.bcb_counter(bcb1).expect("should have a counter") { - CoverageKind::Counter { id, .. } => id, + counters::BcbCounter::Counter { id, .. } => id, _ => panic!("expected a Counter"), } .as_u32() @@ -695,7 +694,7 @@ fn test_make_bcb_counters() { assert_eq!( 1, // bcb2 has a `Counter` with id = 1 match coverage_counters.bcb_counter(bcb2).expect("should have a counter") { - CoverageKind::Counter { id, .. } => id, + counters::BcbCounter::Counter { id, .. } => id, _ => panic!("expected a Counter"), } .as_u32() From 72f4c78dc6d5fc4a09d946f979ccfbf3fa763585 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 14 Aug 2023 12:16:29 +1000 Subject: [PATCH 4/4] coverage: Don't store `function_source_hash` in `BcbCounter::Counter` This shows one small benefit of separating `BcbCounter` from `CoverageKind`. The function source hash will be the same for all counters within a function, so instead of passing it through `CoverageCounters` and storing it in every counter, we can just supply it during the final conversion to `CoverageKind`. --- compiler/rustc_mir_transform/src/coverage/counters.rs | 11 +++-------- compiler/rustc_mir_transform/src/coverage/mod.rs | 8 +++++--- compiler/rustc_mir_transform/src/coverage/tests.rs | 2 +- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 3eca2610de6..3d442e5dca9 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -20,7 +20,7 @@ use std::fmt::{self, Debug}; /// BCB node or BCB edge. #[derive(Clone)] pub(super) enum BcbCounter { - Counter { function_source_hash: u64, id: CounterId }, + Counter { id: CounterId }, Expression { id: ExpressionId, lhs: Operand, op: Op, rhs: Operand }, } @@ -59,7 +59,6 @@ impl Debug for BcbCounter { /// Generates and stores coverage counter and coverage expression information /// associated with nodes/edges in the BCB graph. pub(super) struct CoverageCounters { - function_source_hash: u64, next_counter_id: CounterId, next_expression_id: ExpressionId, @@ -81,11 +80,10 @@ pub(super) struct CoverageCounters { } impl CoverageCounters { - pub(super) fn new(function_source_hash: u64, basic_coverage_blocks: &CoverageGraph) -> Self { + pub(super) fn new(basic_coverage_blocks: &CoverageGraph) -> Self { let num_bcbs = basic_coverage_blocks.num_nodes(); Self { - function_source_hash, next_counter_id: CounterId::START, next_expression_id: ExpressionId::START, @@ -119,10 +117,7 @@ impl CoverageCounters { where F: Fn() -> Option, { - let counter = BcbCounter::Counter { - function_source_hash: self.function_source_hash, - id: self.next_counter(), - }; + let counter = BcbCounter::Counter { id: self.next_counter() }; if self.debug_counters.is_enabled() { self.debug_counters.add_counter(&counter, (debug_block_label_fn)()); } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 39b56d1f00b..8c9eae508b4 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -106,6 +106,7 @@ struct Instrumentor<'a, 'tcx> { source_file: Lrc, fn_sig_span: Span, body_span: Span, + function_source_hash: u64, basic_coverage_blocks: CoverageGraph, coverage_counters: CoverageCounters, } @@ -137,7 +138,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { let function_source_hash = hash_mir_source(tcx, hir_body); let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); - let coverage_counters = CoverageCounters::new(function_source_hash, &basic_coverage_blocks); + let coverage_counters = CoverageCounters::new(&basic_coverage_blocks); Self { pass_name, @@ -146,6 +147,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { source_file, fn_sig_span, body_span, + function_source_hash, basic_coverage_blocks, coverage_counters, } @@ -435,8 +437,8 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind { match *counter_kind { - BcbCounter::Counter { function_source_hash, id } => { - CoverageKind::Counter { function_source_hash, id } + 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 } diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 99ac769be64..4a066ed3abd 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -674,7 +674,7 @@ fn test_make_bcb_counters() { )); } } - let mut coverage_counters = counters::CoverageCounters::new(0, &basic_coverage_blocks); + let mut coverage_counters = counters::CoverageCounters::new(&basic_coverage_blocks); coverage_counters .make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans) .expect("should be Ok");