Skip to content

Commit 7cb8586

Browse files
committed
coverage: Streamline creation of physical node counters
- Look up the node's predecessors only once - Get rid of some overly verbose logging - Explain why some nodes need physical counters - Extract a helper method to create and set a physical node counter
1 parent 13b814f commit 7cb8586

File tree

1 file changed

+19
-23
lines changed

1 file changed

+19
-23
lines changed

compiler/rustc_mir_transform/src/coverage/counters.rs

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,14 @@ impl CoverageCounters {
100100
BcbCounter::Counter { id }
101101
}
102102

103+
/// Creates a new physical counter attached a BCB node.
104+
/// The node must not already have a counter.
105+
fn make_phys_node_counter(&mut self, bcb: BasicCoverageBlock) -> BcbCounter {
106+
let counter = self.make_counter(CounterIncrementSite::Node { bcb });
107+
debug!(?bcb, ?counter, "node gets a physical counter");
108+
self.set_bcb_counter(bcb, counter)
109+
}
110+
103111
fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter {
104112
let new_expr = BcbExpression { lhs, op, rhs };
105113
*self
@@ -353,28 +361,21 @@ impl<'a> MakeBcbCounters<'a> {
353361
return counter_kind;
354362
}
355363

356-
// A BCB with only one incoming edge gets a simple `Counter` (via `make_counter()`).
357-
// Also, a BCB that loops back to itself gets a simple `Counter`. This may indicate the
358-
// program results in a tight infinite loop, but it should still compile.
359-
let one_path_to_target = !self.basic_coverage_blocks.bcb_has_multiple_in_edges(bcb);
360-
if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) {
361-
let counter_kind =
362-
self.coverage_counters.make_counter(CounterIncrementSite::Node { bcb });
363-
if one_path_to_target {
364-
debug!("{bcb:?} gets a new counter: {counter_kind:?}");
365-
} else {
366-
debug!(
367-
"{bcb:?} has itself as its own predecessor. It can't be part of its own \
368-
Expression sum, so it will get its own new counter: {counter_kind:?}. \
369-
(Note, the compiled code will generate an infinite loop.)",
370-
);
371-
}
372-
return self.coverage_counters.set_bcb_counter(bcb, counter_kind);
364+
let predecessors = self.basic_coverage_blocks.predecessors[bcb].as_slice();
365+
366+
// Handle cases where we can't compute a node's count from its in-edges:
367+
// - START_BCB has no in-edges, so taking the sum would panic (or be wrong).
368+
// - For nodes with one in-edge, or that directly loop to themselves,
369+
// trying to get the in-edge counts would require this node's counter,
370+
// leading to infinite recursion.
371+
if predecessors.len() <= 1 || predecessors.contains(&bcb) {
372+
debug!(?bcb, ?predecessors, "node has <=1 predecessors or is its own predecessor");
373+
return self.coverage_counters.make_phys_node_counter(bcb);
373374
}
374375

375376
// A BCB with multiple incoming edges can compute its count by ensuring that counters
376377
// exist for each of those edges, and then adding them up to get a total count.
377-
let in_edge_counters = self.basic_coverage_blocks.predecessors[bcb]
378+
let in_edge_counters = predecessors
378379
.iter()
379380
.copied()
380381
.map(|from_bcb| self.get_or_make_edge_counter(from_bcb, bcb))
@@ -500,11 +501,6 @@ impl<'a> MakeBcbCounters<'a> {
500501
None
501502
}
502503

503-
#[inline]
504-
fn bcb_predecessors(&self, bcb: BasicCoverageBlock) -> &[BasicCoverageBlock] {
505-
&self.basic_coverage_blocks.predecessors[bcb]
506-
}
507-
508504
#[inline]
509505
fn bcb_successors(&self, bcb: BasicCoverageBlock) -> &[BasicCoverageBlock] {
510506
&self.basic_coverage_blocks.successors[bcb]

0 commit comments

Comments
 (0)