coverage: Split out counter increment sites from BCB node/edge counters

This makes it possible for two nodes/edges in the coverage graph to share the
same counter, without causing the instrumentor to inject unwanted duplicate
counter-increment statements.
This commit is contained in:
Zalathar 2024-01-25 13:35:40 +11:00
parent 11f32b73e0
commit 2e212b79e0
2 changed files with 79 additions and 80 deletions

View File

@ -1,4 +1,5 @@
use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::graph::WithNumNodes; use rustc_data_structures::graph::WithNumNodes;
use rustc_index::bit_set::BitSet; use rustc_index::bit_set::BitSet;
use rustc_index::IndexVec; use rustc_index::IndexVec;
@ -38,19 +39,27 @@ impl Debug for BcbCounter {
} }
} }
#[derive(Debug)]
pub(super) enum CounterIncrementSite {
Node { bcb: BasicCoverageBlock },
Edge { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock },
}
/// Generates and stores coverage counter and coverage expression information /// Generates and stores coverage counter and coverage expression information
/// associated with nodes/edges in the BCB graph. /// associated with nodes/edges in the BCB graph.
pub(super) struct CoverageCounters { pub(super) struct CoverageCounters {
next_counter_id: CounterId, /// List of places where a counter-increment statement should be injected
/// into MIR, each with its corresponding counter ID.
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>>, bcb_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.
/// ///
/// The iteration order of this map can affect the precise contents of MIR, /// We currently don't iterate over this map, but if we do in the future,
/// so we use `FxIndexMap` to avoid query stability hazards. /// switch it back to `FxIndexMap` to avoid query stability hazards.
bcb_edge_counters: FxIndexMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>,
/// Tracks which BCBs have a counter associated with some incoming edge. /// Tracks which BCBs have a counter associated with some incoming edge.
/// Only used by assertions, to verify that BCBs with incoming edge /// Only used by assertions, to verify that BCBs with incoming edge
/// counters do not have their own physical counters (expressions are allowed). /// counters do not have their own physical counters (expressions are allowed).
@ -71,9 +80,9 @@ impl CoverageCounters {
let num_bcbs = basic_coverage_blocks.num_nodes(); let num_bcbs = basic_coverage_blocks.num_nodes();
let mut this = Self { let mut this = Self {
next_counter_id: CounterId::START, counter_increment_sites: IndexVec::new(),
bcb_counters: IndexVec::from_elem_n(None, num_bcbs), bcb_counters: IndexVec::from_elem_n(None, num_bcbs),
bcb_edge_counters: FxIndexMap::default(), bcb_edge_counters: FxHashMap::default(),
bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs), bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs),
expressions: IndexVec::new(), expressions: IndexVec::new(),
}; };
@ -84,8 +93,8 @@ impl CoverageCounters {
this this
} }
fn make_counter(&mut self) -> BcbCounter { fn make_counter(&mut self, site: CounterIncrementSite) -> BcbCounter {
let id = self.next_counter(); let id = self.counter_increment_sites.push(site);
BcbCounter::Counter { id } BcbCounter::Counter { id }
} }
@ -103,15 +112,8 @@ impl CoverageCounters {
self.make_expression(lhs, Op::Add, rhs) self.make_expression(lhs, Op::Add, rhs)
} }
/// Counter IDs start from one and go up.
fn next_counter(&mut self) -> CounterId {
let next = self.next_counter_id;
self.next_counter_id = self.next_counter_id + 1;
next
}
pub(super) fn num_counters(&self) -> usize { pub(super) fn num_counters(&self) -> usize {
self.next_counter_id.as_usize() self.counter_increment_sites.len()
} }
#[cfg(test)] #[cfg(test)]
@ -171,22 +173,26 @@ impl CoverageCounters {
self.bcb_counters[bcb] self.bcb_counters[bcb]
} }
pub(super) fn bcb_node_counters( /// Returns an iterator over all the nodes/edges in the coverage graph that
/// should have a counter-increment statement injected into MIR, along with
/// each site's corresponding counter ID.
pub(super) fn counter_increment_sites(
&self, &self,
) -> impl Iterator<Item = (BasicCoverageBlock, &BcbCounter)> { ) -> impl Iterator<Item = (CounterId, &CounterIncrementSite)> {
self.bcb_counters self.counter_increment_sites.iter_enumerated()
.iter_enumerated()
.filter_map(|(bcb, counter_kind)| Some((bcb, counter_kind.as_ref()?)))
} }
/// For each edge in the BCB graph that has an associated counter, yields /// Returns an iterator over the subset of BCB nodes that have been associated
/// that edge's *from* and *to* nodes, and its counter. /// with a counter *expression*, along with the ID of that expression.
pub(super) fn bcb_edge_counters( pub(super) fn bcb_nodes_with_coverage_expressions(
&self, &self,
) -> impl Iterator<Item = (BasicCoverageBlock, BasicCoverageBlock, &BcbCounter)> { ) -> impl Iterator<Item = (BasicCoverageBlock, ExpressionId)> + Captures<'_> {
self.bcb_edge_counters self.bcb_counters.iter_enumerated().filter_map(|(bcb, &counter_kind)| match counter_kind {
.iter() // Yield the BCB along with its associated expression ID.
.map(|(&(from_bcb, to_bcb), counter_kind)| (from_bcb, to_bcb, counter_kind)) Some(BcbCounter::Expression { id }) => Some((bcb, id)),
// This BCB is associated with a counter or nothing, so skip it.
Some(BcbCounter::Counter { .. }) | None => None,
})
} }
pub(super) fn into_expressions(self) -> IndexVec<ExpressionId, Expression> { pub(super) fn into_expressions(self) -> IndexVec<ExpressionId, Expression> {
@ -339,7 +345,8 @@ impl<'a> MakeBcbCounters<'a> {
// program results in a tight infinite loop, but it should still compile. // program results in a tight infinite loop, but it should still compile.
let one_path_to_target = !self.basic_coverage_blocks.bcb_has_multiple_in_edges(bcb); let one_path_to_target = !self.basic_coverage_blocks.bcb_has_multiple_in_edges(bcb);
if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) { if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) {
let counter_kind = self.coverage_counters.make_counter(); let counter_kind =
self.coverage_counters.make_counter(CounterIncrementSite::Node { bcb });
if one_path_to_target { if one_path_to_target {
debug!("{bcb:?} gets a new counter: {counter_kind:?}"); debug!("{bcb:?} gets a new counter: {counter_kind:?}");
} else { } else {
@ -401,7 +408,8 @@ impl<'a> MakeBcbCounters<'a> {
} }
// Make a new counter to count this edge. // Make a new counter to count this edge.
let counter_kind = self.coverage_counters.make_counter(); let counter_kind =
self.coverage_counters.make_counter(CounterIncrementSite::Edge { from_bcb, to_bcb });
debug!("Edge {from_bcb:?}->{to_bcb:?} gets a new counter: {counter_kind:?}"); debug!("Edge {from_bcb:?}->{to_bcb:?} gets a new counter: {counter_kind:?}");
self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter_kind) self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter_kind)
} }

View File

@ -7,7 +7,7 @@ mod spans;
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
use self::counters::{BcbCounter, CoverageCounters}; use self::counters::{CounterIncrementSite, CoverageCounters};
use self::graph::{BasicCoverageBlock, CoverageGraph}; use self::graph::{BasicCoverageBlock, CoverageGraph};
use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans}; use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans};
@ -155,61 +155,52 @@ fn inject_coverage_statements<'tcx>(
bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool, bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
coverage_counters: &CoverageCounters, coverage_counters: &CoverageCounters,
) { ) {
// Process the counters associated with BCB nodes. // Inject counter-increment statements into MIR.
for (bcb, counter_kind) in coverage_counters.bcb_node_counters() { for (id, counter_increment_site) in coverage_counters.counter_increment_sites() {
let do_inject = match counter_kind { // Determine the block to inject a counter-increment statement into.
// Counter-increment statements always need to be injected. // For BCB nodes this is just their first block, but for edges we need
BcbCounter::Counter { .. } => true, // to create a new block between the two BCBs, and inject into that.
// The only purpose of expression-used statements is to detect let target_bb = match *counter_increment_site {
// when a mapping is unreachable, so we only inject them for CounterIncrementSite::Node { bcb } => basic_coverage_blocks[bcb].leader_bb(),
// expressions with one or more mappings. CounterIncrementSite::Edge { from_bcb, to_bcb } => {
BcbCounter::Expression { .. } => bcb_has_coverage_spans(bcb), // Create a new block between the last block of `from_bcb` and
// the first block of `to_bcb`.
let from_bb = basic_coverage_blocks[from_bcb].last_bb();
let to_bb = basic_coverage_blocks[to_bcb].leader_bb();
let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb);
debug!(
"Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \
requires a new MIR BasicBlock {new_bb:?} for counter increment {id:?}",
);
new_bb
}
}; };
if do_inject {
inject_statement( inject_statement(mir_body, CoverageKind::CounterIncrement { id }, target_bb);
mir_body,
make_mir_coverage_kind(counter_kind),
basic_coverage_blocks[bcb].leader_bb(),
);
}
} }
// Process the counters associated with BCB edges. // For each counter expression that is directly associated with at least one
for (from_bcb, to_bcb, counter_kind) in coverage_counters.bcb_edge_counters() { // span, we inject an "expression-used" statement, so that coverage codegen
let do_inject = match counter_kind { // can check whether the injected statement survived MIR optimization.
// Counter-increment statements always need to be injected. // (BCB edges can't have spans, so we only need to process BCB nodes here.)
BcbCounter::Counter { .. } => true, //
// BCB-edge expressions never have mappings, so they never need // See the code in `rustc_codegen_llvm::coverageinfo::map_data` that deals
// a corresponding statement. // with "expressions seen" and "zero terms".
BcbCounter::Expression { .. } => false, for (bcb, expression_id) in coverage_counters
}; .bcb_nodes_with_coverage_expressions()
if !do_inject { .filter(|&(bcb, _)| bcb_has_coverage_spans(bcb))
continue; {
} inject_statement(
mir_body,
// We need to inject a coverage statement into a new BB between the CoverageKind::ExpressionUsed { id: expression_id },
// last BB of `from_bcb` and the first BB of `to_bcb`. basic_coverage_blocks[bcb].leader_bb(),
let from_bb = basic_coverage_blocks[from_bcb].last_bb();
let to_bb = basic_coverage_blocks[to_bcb].leader_bb();
let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb);
debug!(
"Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \
requires a new MIR BasicBlock {new_bb:?} for edge counter {counter_kind:?}",
); );
// Inject a counter into the newly-created BB.
inject_statement(mir_body, make_mir_coverage_kind(counter_kind), new_bb);
}
}
fn make_mir_coverage_kind(counter_kind: &BcbCounter) -> CoverageKind {
match *counter_kind {
BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id },
BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id },
} }
} }
/// Given two basic blocks that have a control-flow edge between them, creates
/// and returns a new block that sits between those blocks.
fn inject_edge_counter_basic_block( fn inject_edge_counter_basic_block(
mir_body: &mut mir::Body<'_>, mir_body: &mut mir::Body<'_>,
from_bb: BasicBlock, from_bb: BasicBlock,