diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 67f4696f6518..0bf8d8c0a8da 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -5,7 +5,6 @@ use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph::DirectedGraph; use rustc_index::IndexVec; use rustc_index::bit_set::BitSet; -use rustc_middle::bug; use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, Op}; use tracing::{debug, debug_span, instrument}; @@ -58,13 +57,13 @@ pub(super) struct CoverageCounters { counter_increment_sites: IndexVec, /// Coverage counters/expressions that are associated with individual BCBs. - bcb_counters: IndexVec>, + node_counters: IndexVec>, /// Coverage counters/expressions that are associated with the control-flow /// edge between two BCBs. /// /// We currently don't iterate over this map, but if we do in the future, /// switch it back to `FxIndexMap` to avoid query stability hazards. - bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, + edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, /// Table of expression data, associating each expression ID with its /// corresponding operator (+ or -) and its LHS/RHS operands. @@ -78,20 +77,20 @@ impl CoverageCounters { /// Ensures that each BCB node needing a counter has one, by creating physical /// counters or counter expressions for nodes and edges as required. pub(super) fn make_bcb_counters( - basic_coverage_blocks: &CoverageGraph, + graph: &CoverageGraph, bcb_needs_counter: &BitSet, ) -> Self { - let mut counters = MakeBcbCounters::new(basic_coverage_blocks, bcb_needs_counter); - counters.make_bcb_counters(); + let mut builder = CountersBuilder::new(graph, bcb_needs_counter); + builder.make_bcb_counters(); - counters.coverage_counters + builder.counters } fn with_num_bcbs(num_bcbs: usize) -> Self { Self { counter_increment_sites: IndexVec::new(), - bcb_counters: IndexVec::from_elem_n(None, num_bcbs), - bcb_edge_counters: FxHashMap::default(), + node_counters: IndexVec::from_elem_n(None, num_bcbs), + edge_counters: FxHashMap::default(), expressions: IndexVec::new(), expressions_memo: FxHashMap::default(), } @@ -189,35 +188,31 @@ impl CoverageCounters { self.counter_increment_sites.len() } - fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter { - if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { - bug!( - "attempt to set a BasicCoverageBlock coverage counter more than once; \ - {bcb:?} already had counter {replaced:?}", - ); - } else { - counter_kind - } + fn set_node_counter(&mut self, bcb: BasicCoverageBlock, counter: BcbCounter) -> BcbCounter { + let existing = self.node_counters[bcb].replace(counter); + assert!( + existing.is_none(), + "node {bcb:?} already has a counter: {existing:?} => {counter:?}" + ); + counter } - fn set_bcb_edge_counter( + fn set_edge_counter( &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - counter_kind: BcbCounter, + counter: BcbCounter, ) -> BcbCounter { - if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) { - bug!( - "attempt to set an edge counter more than once; from_bcb: \ - {from_bcb:?} already had counter {replaced:?}", - ); - } else { - counter_kind - } + let existing = self.edge_counters.insert((from_bcb, to_bcb), counter); + assert!( + existing.is_none(), + "edge ({from_bcb:?} -> {to_bcb:?}) already has a counter: {existing:?} => {counter:?}" + ); + counter } pub(super) fn term_for_bcb(&self, bcb: BasicCoverageBlock) -> Option { - self.bcb_counters[bcb].map(|counter| counter.as_term()) + self.node_counters[bcb].map(|counter| counter.as_term()) } /// Returns an iterator over all the nodes/edges in the coverage graph that @@ -234,7 +229,7 @@ impl CoverageCounters { pub(super) fn bcb_nodes_with_coverage_expressions( &self, ) -> impl Iterator + Captures<'_> { - self.bcb_counters.iter_enumerated().filter_map(|(bcb, &counter_kind)| match counter_kind { + self.node_counters.iter_enumerated().filter_map(|(bcb, &counter)| match counter { // Yield the BCB along with its associated expression ID. Some(BcbCounter::Expression { id }) => Some((bcb, id)), // This BCB is associated with a counter or nothing, so skip it. @@ -261,22 +256,20 @@ impl CoverageCounters { } } -/// Helper struct that allows counter creation to inspect the BCB graph. -struct MakeBcbCounters<'a> { - coverage_counters: CoverageCounters, - basic_coverage_blocks: &'a CoverageGraph, +/// Helper struct that allows counter creation to inspect the BCB graph, and +/// the set of nodes that need counters. +struct CountersBuilder<'a> { + counters: CoverageCounters, + graph: &'a CoverageGraph, bcb_needs_counter: &'a BitSet, } -impl<'a> MakeBcbCounters<'a> { - fn new( - basic_coverage_blocks: &'a CoverageGraph, - bcb_needs_counter: &'a BitSet, - ) -> Self { - assert_eq!(basic_coverage_blocks.num_nodes(), bcb_needs_counter.domain_size()); +impl<'a> CountersBuilder<'a> { + fn new(graph: &'a CoverageGraph, bcb_needs_counter: &'a BitSet) -> Self { + assert_eq!(graph.num_nodes(), bcb_needs_counter.domain_size()); Self { - coverage_counters: CoverageCounters::with_num_bcbs(basic_coverage_blocks.num_nodes()), - basic_coverage_blocks, + counters: CoverageCounters::with_num_bcbs(graph.num_nodes()), + graph, bcb_needs_counter, } } @@ -291,7 +284,7 @@ impl<'a> MakeBcbCounters<'a> { // nodes within the loop are visited before visiting any nodes outside // the loop. It also keeps track of which loop(s) the traversal is // currently inside. - let mut traversal = TraverseCoverageGraphWithLoops::new(self.basic_coverage_blocks); + let mut traversal = TraverseCoverageGraphWithLoops::new(self.graph); while let Some(bcb) = traversal.next() { let _span = debug_span!("traversal", ?bcb).entered(); if self.bcb_needs_counter.contains(bcb) { @@ -318,26 +311,26 @@ impl<'a> MakeBcbCounters<'a> { // We might also use that counter to compute one of the out-edge counters. let node_counter = self.get_or_make_node_counter(from_bcb); - let successors = self.basic_coverage_blocks.successors[from_bcb].as_slice(); + let successors = self.graph.successors[from_bcb].as_slice(); // If this node's out-edges won't sum to the node's counter, // then there's no reason to create edge counters here. - if !self.basic_coverage_blocks[from_bcb].is_out_summable { + if !self.graph[from_bcb].is_out_summable { return; } // When choosing which out-edge should be given a counter expression, ignore edges that // already have counters, or could use the existing counter of their target node. let out_edge_has_counter = |to_bcb| { - if self.coverage_counters.bcb_edge_counters.contains_key(&(from_bcb, to_bcb)) { + if self.counters.edge_counters.contains_key(&(from_bcb, to_bcb)) { return true; } - self.basic_coverage_blocks.sole_predecessor(to_bcb) == Some(from_bcb) - && self.coverage_counters.bcb_counters[to_bcb].is_some() + self.graph.sole_predecessor(to_bcb) == Some(from_bcb) + && self.counters.node_counters[to_bcb].is_some() }; // Determine the set of out-edges that could benefit from being given an expression. - let candidate_successors = self.basic_coverage_blocks.successors[from_bcb] + let candidate_successors = self.graph.successors[from_bcb] .iter() .copied() .filter(|&to_bcb| !out_edge_has_counter(to_bcb)) @@ -346,7 +339,7 @@ impl<'a> MakeBcbCounters<'a> { // If there are out-edges without counters, choose one to be given an expression // (computed from this node and the other out-edges) instead of a physical counter. - let Some(expression_to_bcb) = + let Some(target_bcb) = self.choose_out_edge_for_expression(traversal, &candidate_successors) else { return; @@ -359,49 +352,44 @@ impl<'a> MakeBcbCounters<'a> { .iter() .copied() // Skip the chosen edge, since we'll calculate its count from this sum. - .filter(|&to_bcb| to_bcb != expression_to_bcb) + .filter(|&edge_target_bcb| edge_target_bcb != target_bcb) .map(|to_bcb| self.get_or_make_edge_counter(from_bcb, to_bcb)) .collect::>(); - let Some(sum_of_all_other_out_edges) = - self.coverage_counters.make_sum(&other_out_edge_counters) + let Some(sum_of_all_other_out_edges) = self.counters.make_sum(&other_out_edge_counters) else { return; }; // Now create an expression for the chosen edge, by taking the counter // for its source node and subtracting the sum of its sibling out-edges. - let expression = self.coverage_counters.make_expression( - node_counter, - Op::Subtract, - sum_of_all_other_out_edges, - ); + let expression = + self.counters.make_expression(node_counter, Op::Subtract, sum_of_all_other_out_edges); - debug!("{expression_to_bcb:?} gets an expression: {expression:?}"); - self.coverage_counters.set_bcb_edge_counter(from_bcb, expression_to_bcb, expression); + debug!("{target_bcb:?} gets an expression: {expression:?}"); + self.counters.set_edge_counter(from_bcb, target_bcb, expression); } #[instrument(level = "debug", skip(self))] fn get_or_make_node_counter(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { // If the BCB already has a counter, return it. - if let Some(counter_kind) = self.coverage_counters.bcb_counters[bcb] { - debug!("{bcb:?} already has a counter: {counter_kind:?}"); - return counter_kind; + if let Some(counter) = self.counters.node_counters[bcb] { + debug!("{bcb:?} already has a counter: {counter:?}"); + return counter; } let counter = self.make_node_counter_inner(bcb); - self.coverage_counters.set_bcb_counter(bcb, counter) + self.counters.set_node_counter(bcb, counter) } fn make_node_counter_inner(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { // If the node's sole in-edge already has a counter, use that. - if let Some(sole_pred) = self.basic_coverage_blocks.sole_predecessor(bcb) - && let Some(&edge_counter) = - self.coverage_counters.bcb_edge_counters.get(&(sole_pred, bcb)) + if let Some(sole_pred) = self.graph.sole_predecessor(bcb) + && let Some(&edge_counter) = self.counters.edge_counters.get(&(sole_pred, bcb)) { return edge_counter; } - let predecessors = self.basic_coverage_blocks.predecessors[bcb].as_slice(); + let predecessors = self.graph.predecessors[bcb].as_slice(); // Handle cases where we can't compute a node's count from its in-edges: // - START_BCB has no in-edges, so taking the sum would panic (or be wrong). @@ -410,7 +398,7 @@ impl<'a> MakeBcbCounters<'a> { // leading to infinite recursion. if predecessors.len() <= 1 || predecessors.contains(&bcb) { debug!(?bcb, ?predecessors, "node has <=1 predecessors or is its own predecessor"); - let counter = self.coverage_counters.make_phys_node_counter(bcb); + let counter = self.counters.make_phys_node_counter(bcb); debug!(?bcb, ?counter, "node gets a physical counter"); return counter; } @@ -422,10 +410,8 @@ impl<'a> MakeBcbCounters<'a> { .copied() .map(|from_bcb| self.get_or_make_edge_counter(from_bcb, bcb)) .collect::>(); - let sum_of_in_edges: BcbCounter = self - .coverage_counters - .make_sum(&in_edge_counters) - .expect("there must be at least one in-edge"); + let sum_of_in_edges: BcbCounter = + self.counters.make_sum(&in_edge_counters).expect("there must be at least one in-edge"); debug!("{bcb:?} gets a new counter (sum of predecessor counters): {sum_of_in_edges:?}"); sum_of_in_edges @@ -438,15 +424,13 @@ impl<'a> MakeBcbCounters<'a> { to_bcb: BasicCoverageBlock, ) -> BcbCounter { // If the edge already has a counter, return it. - if let Some(&counter_kind) = - self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) - { - debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter_kind:?}"); - return counter_kind; + if let Some(&counter) = self.counters.edge_counters.get(&(from_bcb, to_bcb)) { + debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter:?}"); + return counter; } let counter = self.make_edge_counter_inner(from_bcb, to_bcb); - self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter) + self.counters.set_edge_counter(from_bcb, to_bcb, counter) } fn make_edge_counter_inner( @@ -456,7 +440,7 @@ impl<'a> MakeBcbCounters<'a> { ) -> BcbCounter { // If the target node has exactly one in-edge (i.e. this one), then just // use the node's counter, since it will have the same value. - if let Some(sole_pred) = self.basic_coverage_blocks.sole_predecessor(to_bcb) { + if let Some(sole_pred) = self.graph.sole_predecessor(to_bcb) { assert_eq!(sole_pred, from_bcb); // This call must take care not to invoke `get_or_make_edge` for // this edge, since that would result in infinite recursion! @@ -465,13 +449,13 @@ impl<'a> MakeBcbCounters<'a> { // If the source node has exactly one out-edge (i.e. this one) and would have // the same execution count as that edge, then just use the node's counter. - if let Some(simple_succ) = self.basic_coverage_blocks.simple_successor(from_bcb) { + if let Some(simple_succ) = self.graph.simple_successor(from_bcb) { assert_eq!(simple_succ, to_bcb); return self.get_or_make_node_counter(from_bcb); } // Make a new counter to count this edge. - let counter = self.coverage_counters.make_phys_edge_counter(from_bcb, to_bcb); + let counter = self.counters.make_phys_edge_counter(from_bcb, to_bcb); debug!(?from_bcb, ?to_bcb, ?counter, "edge gets a physical counter"); counter } @@ -518,9 +502,9 @@ impl<'a> MakeBcbCounters<'a> { for &target_bcb in candidate_successors { // An edge is a reloop edge if its target dominates any BCB that has // an edge back to the loop header. (Otherwise it's an exit edge.) - let is_reloop_edge = reloop_bcbs.iter().any(|&reloop_bcb| { - self.basic_coverage_blocks.dominates(target_bcb, reloop_bcb) - }); + let is_reloop_edge = reloop_bcbs + .iter() + .any(|&reloop_bcb| self.graph.dominates(target_bcb, reloop_bcb)); if is_reloop_edge { // We found a good out-edge to be given an expression. return Some(target_bcb);