coverage: Streamline several names of things in counter creation

This commit is contained in:
Zalathar 2024-10-19 18:02:03 +11:00
parent a400d7fb76
commit 4b8f7f547a

View File

@ -5,7 +5,6 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::graph::DirectedGraph; use rustc_data_structures::graph::DirectedGraph;
use rustc_index::IndexVec; use rustc_index::IndexVec;
use rustc_index::bit_set::BitSet; use rustc_index::bit_set::BitSet;
use rustc_middle::bug;
use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, Op}; use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, Op};
use tracing::{debug, debug_span, instrument}; use tracing::{debug, debug_span, instrument};
@ -58,13 +57,13 @@ pub(super) struct CoverageCounters {
counter_increment_sites: IndexVec<CounterId, CounterIncrementSite>, counter_increment_sites: IndexVec<CounterId, CounterIncrementSite>,
/// Coverage counters/expressions that are associated with individual BCBs. /// Coverage counters/expressions that are associated with individual BCBs.
bcb_counters: IndexVec<BasicCoverageBlock, Option<BcbCounter>>, node_counters: IndexVec<BasicCoverageBlock, Option<BcbCounter>>,
/// Coverage counters/expressions that are associated with the control-flow /// Coverage counters/expressions that are associated with the control-flow
/// edge between two BCBs. /// edge between two BCBs.
/// ///
/// We currently don't iterate over this map, but if we do in the future, /// 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. /// 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 /// Table of expression data, associating each expression ID with its
/// corresponding operator (+ or -) and its LHS/RHS operands. /// 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 /// Ensures that each BCB node needing a counter has one, by creating physical
/// counters or counter expressions for nodes and edges as required. /// counters or counter expressions for nodes and edges as required.
pub(super) fn make_bcb_counters( pub(super) fn make_bcb_counters(
basic_coverage_blocks: &CoverageGraph, graph: &CoverageGraph,
bcb_needs_counter: &BitSet<BasicCoverageBlock>, bcb_needs_counter: &BitSet<BasicCoverageBlock>,
) -> Self { ) -> Self {
let mut counters = MakeBcbCounters::new(basic_coverage_blocks, bcb_needs_counter); let mut builder = CountersBuilder::new(graph, bcb_needs_counter);
counters.make_bcb_counters(); builder.make_bcb_counters();
counters.coverage_counters builder.counters
} }
fn with_num_bcbs(num_bcbs: usize) -> Self { fn with_num_bcbs(num_bcbs: usize) -> Self {
Self { Self {
counter_increment_sites: IndexVec::new(), counter_increment_sites: IndexVec::new(),
bcb_counters: IndexVec::from_elem_n(None, num_bcbs), node_counters: IndexVec::from_elem_n(None, num_bcbs),
bcb_edge_counters: FxHashMap::default(), edge_counters: FxHashMap::default(),
expressions: IndexVec::new(), expressions: IndexVec::new(),
expressions_memo: FxHashMap::default(), expressions_memo: FxHashMap::default(),
} }
@ -187,35 +186,31 @@ impl CoverageCounters {
self.counter_increment_sites.len() self.counter_increment_sites.len()
} }
fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter { fn set_node_counter(&mut self, bcb: BasicCoverageBlock, counter: BcbCounter) -> BcbCounter {
if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { let existing = self.node_counters[bcb].replace(counter);
bug!( assert!(
"attempt to set a BasicCoverageBlock coverage counter more than once; \ existing.is_none(),
{bcb:?} already had counter {replaced:?}", "node {bcb:?} already has a counter: {existing:?} => {counter:?}"
); );
} else { counter
counter_kind
}
} }
fn set_bcb_edge_counter( fn set_edge_counter(
&mut self, &mut self,
from_bcb: BasicCoverageBlock, from_bcb: BasicCoverageBlock,
to_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock,
counter_kind: BcbCounter, counter: BcbCounter,
) -> BcbCounter { ) -> BcbCounter {
if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) { let existing = self.edge_counters.insert((from_bcb, to_bcb), counter);
bug!( assert!(
"attempt to set an edge counter more than once; from_bcb: \ existing.is_none(),
{from_bcb:?} already had counter {replaced:?}", "edge ({from_bcb:?} -> {to_bcb:?}) already has a counter: {existing:?} => {counter:?}"
); );
} else { counter
counter_kind
}
} }
pub(super) fn term_for_bcb(&self, bcb: BasicCoverageBlock) -> Option<CovTerm> { pub(super) fn term_for_bcb(&self, bcb: BasicCoverageBlock) -> Option<CovTerm> {
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 /// Returns an iterator over all the nodes/edges in the coverage graph that
@ -232,7 +227,7 @@ impl CoverageCounters {
pub(super) fn bcb_nodes_with_coverage_expressions( pub(super) fn bcb_nodes_with_coverage_expressions(
&self, &self,
) -> impl Iterator<Item = (BasicCoverageBlock, ExpressionId)> + Captures<'_> { ) -> impl Iterator<Item = (BasicCoverageBlock, ExpressionId)> + 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. // Yield the BCB along with its associated expression ID.
Some(BcbCounter::Expression { id }) => Some((bcb, id)), Some(BcbCounter::Expression { id }) => Some((bcb, id)),
// This BCB is associated with a counter or nothing, so skip it. // This BCB is associated with a counter or nothing, so skip it.
@ -259,22 +254,20 @@ impl CoverageCounters {
} }
} }
/// Helper struct that allows counter creation to inspect the BCB graph. /// Helper struct that allows counter creation to inspect the BCB graph, and
struct MakeBcbCounters<'a> { /// the set of nodes that need counters.
coverage_counters: CoverageCounters, struct CountersBuilder<'a> {
basic_coverage_blocks: &'a CoverageGraph, counters: CoverageCounters,
graph: &'a CoverageGraph,
bcb_needs_counter: &'a BitSet<BasicCoverageBlock>, bcb_needs_counter: &'a BitSet<BasicCoverageBlock>,
} }
impl<'a> MakeBcbCounters<'a> { impl<'a> CountersBuilder<'a> {
fn new( fn new(graph: &'a CoverageGraph, bcb_needs_counter: &'a BitSet<BasicCoverageBlock>) -> Self {
basic_coverage_blocks: &'a CoverageGraph, assert_eq!(graph.num_nodes(), bcb_needs_counter.domain_size());
bcb_needs_counter: &'a BitSet<BasicCoverageBlock>,
) -> Self {
assert_eq!(basic_coverage_blocks.num_nodes(), bcb_needs_counter.domain_size());
Self { Self {
coverage_counters: CoverageCounters::with_num_bcbs(basic_coverage_blocks.num_nodes()), counters: CoverageCounters::with_num_bcbs(graph.num_nodes()),
basic_coverage_blocks, graph,
bcb_needs_counter, bcb_needs_counter,
} }
} }
@ -289,7 +282,7 @@ impl<'a> MakeBcbCounters<'a> {
// nodes within the loop are visited before visiting any nodes outside // nodes within the loop are visited before visiting any nodes outside
// the loop. It also keeps track of which loop(s) the traversal is // the loop. It also keeps track of which loop(s) the traversal is
// currently inside. // currently inside.
let mut traversal = TraverseCoverageGraphWithLoops::new(self.basic_coverage_blocks); let mut traversal = TraverseCoverageGraphWithLoops::new(self.graph);
while let Some(bcb) = traversal.next() { while let Some(bcb) = traversal.next() {
let _span = debug_span!("traversal", ?bcb).entered(); let _span = debug_span!("traversal", ?bcb).entered();
if self.bcb_needs_counter.contains(bcb) { if self.bcb_needs_counter.contains(bcb) {
@ -316,26 +309,26 @@ impl<'a> MakeBcbCounters<'a> {
// We might also use that counter to compute one of the out-edge counters. // 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 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, // If this node's out-edges won't sum to the node's counter,
// then there's no reason to create edge counters here. // 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; return;
} }
// When choosing which out-edge should be given a counter expression, ignore edges that // 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. // already have counters, or could use the existing counter of their target node.
let out_edge_has_counter = |to_bcb| { 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; return true;
} }
self.basic_coverage_blocks.sole_predecessor(to_bcb) == Some(from_bcb) self.graph.sole_predecessor(to_bcb) == Some(from_bcb)
&& self.coverage_counters.bcb_counters[to_bcb].is_some() && self.counters.node_counters[to_bcb].is_some()
}; };
// Determine the set of out-edges that could benefit from being given an expression. // 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() .iter()
.copied() .copied()
.filter(|&to_bcb| !out_edge_has_counter(to_bcb)) .filter(|&to_bcb| !out_edge_has_counter(to_bcb))
@ -344,7 +337,7 @@ impl<'a> MakeBcbCounters<'a> {
// If there are out-edges without counters, choose one to be given an expression // 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. // (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) self.choose_out_edge_for_expression(traversal, &candidate_successors)
else { else {
return; return;
@ -357,49 +350,44 @@ impl<'a> MakeBcbCounters<'a> {
.iter() .iter()
.copied() .copied()
// Skip the chosen edge, since we'll calculate its count from this sum. // 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)) .map(|to_bcb| self.get_or_make_edge_counter(from_bcb, to_bcb))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let Some(sum_of_all_other_out_edges) = let Some(sum_of_all_other_out_edges) = self.counters.make_sum(&other_out_edge_counters)
self.coverage_counters.make_sum(&other_out_edge_counters)
else { else {
return; return;
}; };
// Now create an expression for the chosen edge, by taking the counter // 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. // for its source node and subtracting the sum of its sibling out-edges.
let expression = self.coverage_counters.make_expression( let expression =
node_counter, self.counters.make_expression(node_counter, Op::Subtract, sum_of_all_other_out_edges);
Op::Subtract,
sum_of_all_other_out_edges,
);
debug!("{expression_to_bcb:?} gets an expression: {expression:?}"); debug!("{target_bcb:?} gets an expression: {expression:?}");
self.coverage_counters.set_bcb_edge_counter(from_bcb, expression_to_bcb, expression); self.counters.set_edge_counter(from_bcb, target_bcb, expression);
} }
#[instrument(level = "debug", skip(self))] #[instrument(level = "debug", skip(self))]
fn get_or_make_node_counter(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { fn get_or_make_node_counter(&mut self, bcb: BasicCoverageBlock) -> BcbCounter {
// If the BCB already has a counter, return it. // If the BCB already has a counter, return it.
if let Some(counter_kind) = self.coverage_counters.bcb_counters[bcb] { if let Some(counter) = self.counters.node_counters[bcb] {
debug!("{bcb:?} already has a counter: {counter_kind:?}"); debug!("{bcb:?} already has a counter: {counter:?}");
return counter_kind; return counter;
} }
let counter = self.make_node_counter_inner(bcb); 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 { fn make_node_counter_inner(&mut self, bcb: BasicCoverageBlock) -> BcbCounter {
// If the node's sole in-edge already has a counter, use that. // 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) if let Some(sole_pred) = self.graph.sole_predecessor(bcb)
&& let Some(&edge_counter) = && let Some(&edge_counter) = self.counters.edge_counters.get(&(sole_pred, bcb))
self.coverage_counters.bcb_edge_counters.get(&(sole_pred, bcb))
{ {
return edge_counter; 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: // 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). // - START_BCB has no in-edges, so taking the sum would panic (or be wrong).
@ -408,7 +396,7 @@ impl<'a> MakeBcbCounters<'a> {
// leading to infinite recursion. // leading to infinite recursion.
if predecessors.len() <= 1 || predecessors.contains(&bcb) { if predecessors.len() <= 1 || predecessors.contains(&bcb) {
debug!(?bcb, ?predecessors, "node has <=1 predecessors or is its own predecessor"); 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"); debug!(?bcb, ?counter, "node gets a physical counter");
return counter; return counter;
} }
@ -420,10 +408,8 @@ impl<'a> MakeBcbCounters<'a> {
.copied() .copied()
.map(|from_bcb| self.get_or_make_edge_counter(from_bcb, bcb)) .map(|from_bcb| self.get_or_make_edge_counter(from_bcb, bcb))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let sum_of_in_edges: BcbCounter = self let sum_of_in_edges: BcbCounter =
.coverage_counters self.counters.make_sum(&in_edge_counters).expect("there must be at least one in-edge");
.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:?}"); debug!("{bcb:?} gets a new counter (sum of predecessor counters): {sum_of_in_edges:?}");
sum_of_in_edges sum_of_in_edges
@ -436,15 +422,13 @@ impl<'a> MakeBcbCounters<'a> {
to_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock,
) -> BcbCounter { ) -> BcbCounter {
// If the edge already has a counter, return it. // If the edge already has a counter, return it.
if let Some(&counter_kind) = if let Some(&counter) = self.counters.edge_counters.get(&(from_bcb, to_bcb)) {
self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter:?}");
{ return counter;
debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter_kind:?}");
return counter_kind;
} }
let counter = self.make_edge_counter_inner(from_bcb, to_bcb); 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( fn make_edge_counter_inner(
@ -454,7 +438,7 @@ impl<'a> MakeBcbCounters<'a> {
) -> BcbCounter { ) -> BcbCounter {
// If the target node has exactly one in-edge (i.e. this one), then just // 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. // 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); assert_eq!(sole_pred, from_bcb);
// This call must take care not to invoke `get_or_make_edge` for // This call must take care not to invoke `get_or_make_edge` for
// this edge, since that would result in infinite recursion! // this edge, since that would result in infinite recursion!
@ -463,13 +447,13 @@ impl<'a> MakeBcbCounters<'a> {
// If the source node has exactly one out-edge (i.e. this one) and would have // 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. // 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); assert_eq!(simple_succ, to_bcb);
return self.get_or_make_node_counter(from_bcb); return self.get_or_make_node_counter(from_bcb);
} }
// Make a new counter to count this edge. // 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"); debug!(?from_bcb, ?to_bcb, ?counter, "edge gets a physical counter");
counter counter
} }
@ -516,9 +500,9 @@ impl<'a> MakeBcbCounters<'a> {
for &target_bcb in candidate_successors { for &target_bcb in candidate_successors {
// An edge is a reloop edge if its target dominates any BCB that has // 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.) // an edge back to the loop header. (Otherwise it's an exit edge.)
let is_reloop_edge = reloop_bcbs.iter().any(|&reloop_bcb| { let is_reloop_edge = reloop_bcbs
self.basic_coverage_blocks.dominates(target_bcb, reloop_bcb) .iter()
}); .any(|&reloop_bcb| self.graph.dominates(target_bcb, reloop_bcb));
if is_reloop_edge { if is_reloop_edge {
// We found a good out-edge to be given an expression. // We found a good out-edge to be given an expression.
return Some(target_bcb); return Some(target_bcb);