Skip to content

Commit e24310b

Browse files
committed
coverage: Simplify choosing an out-edge to be given a counter expression
By building the list of candidate edges up-front, and making the candidate-selection method fallible, we can remove a few pieces of awkward code.
1 parent 771659d commit e24310b

File tree

1 file changed

+48
-69
lines changed

1 file changed

+48
-69
lines changed

compiler/rustc_mir_transform/src/coverage/counters.rs

Lines changed: 48 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -321,25 +321,23 @@ impl<'a> MakeBcbCounters<'a> {
321321
if !self.basic_coverage_blocks[from_bcb].is_out_summable {
322322
return;
323323
}
324-
// If this node doesn't have multiple out-edges, or all of its out-edges
325-
// already have counters, then we don't need to create edge counters.
326-
let needs_out_edge_counters = successors.len() > 1
327-
&& successors.iter().any(|&to_bcb| self.edge_has_no_counter(from_bcb, to_bcb));
328-
if !needs_out_edge_counters {
329-
return;
330-
}
331324

332-
if tracing::enabled!(tracing::Level::DEBUG) {
333-
let _span =
334-
debug_span!("node has some out-edges without counters", ?from_bcb).entered();
335-
for &to_bcb in successors {
336-
debug!(?to_bcb, counter=?self.edge_counter(from_bcb, to_bcb));
337-
}
338-
}
325+
// Determine the set of out-edges that don't yet have edge counters.
326+
let candidate_successors = self
327+
.bcb_successors(from_bcb)
328+
.iter()
329+
.copied()
330+
.filter(|&to_bcb| self.edge_has_no_counter(from_bcb, to_bcb))
331+
.collect::<Vec<_>>();
332+
debug!(?candidate_successors);
339333

340-
// Of the out-edges that don't have counters yet, one can be given an expression
341-
// (computed from the other out-edges) instead of a dedicated counter.
342-
let expression_to_bcb = self.choose_out_edge_for_expression(traversal, from_bcb);
334+
// If there are out-edges without counters, choose one to be given an expression
335+
// (computed from this node and the other out-edges) instead of a physical counter.
336+
let Some(expression_to_bcb) =
337+
self.choose_out_edge_for_expression(traversal, &candidate_successors)
338+
else {
339+
return;
340+
};
343341

344342
// For each out-edge other than the one that was chosen to get an expression,
345343
// ensure that it has a counter (existing counter/expression or a new counter),
@@ -351,10 +349,11 @@ impl<'a> MakeBcbCounters<'a> {
351349
.filter(|&to_bcb| to_bcb != expression_to_bcb)
352350
.map(|to_bcb| self.get_or_make_edge_counter(from_bcb, to_bcb))
353351
.collect::<Vec<_>>();
354-
let sum_of_all_other_out_edges: BcbCounter = self
355-
.coverage_counters
356-
.make_sum(&other_out_edge_counters)
357-
.expect("there must be at least one other out-edge");
352+
let Some(sum_of_all_other_out_edges) =
353+
self.coverage_counters.make_sum(&other_out_edge_counters)
354+
else {
355+
return;
356+
};
358357

359358
// Now create an expression for the chosen edge, by taking the counter
360359
// for its source node and subtracting the sum of its sibling out-edges.
@@ -440,79 +439,59 @@ impl<'a> MakeBcbCounters<'a> {
440439
self.coverage_counters.make_phys_edge_counter(from_bcb, to_bcb)
441440
}
442441

443-
/// Choose one of the out-edges of `from_bcb` to receive an expression
444-
/// instead of a physical counter, and returns that edge's target node.
445-
///
446-
/// - Precondition: The node must have at least one out-edge without a counter.
447-
/// - Postcondition: The selected edge does not have an edge counter.
442+
/// Given a set of candidate out-edges (represented by their successor node),
443+
/// choose one to be given a counter expression instead of a physical counter.
448444
fn choose_out_edge_for_expression(
449445
&self,
450446
traversal: &TraverseCoverageGraphWithLoops<'_>,
451-
from_bcb: BasicCoverageBlock,
452-
) -> BasicCoverageBlock {
453-
if let Some(reloop_target) = self.find_good_reloop_edge(traversal, from_bcb) {
454-
assert!(self.edge_has_no_counter(from_bcb, reloop_target));
447+
candidate_successors: &[BasicCoverageBlock],
448+
) -> Option<BasicCoverageBlock> {
449+
// Try to find a candidate that leads back to the top of a loop,
450+
// because reloop edges tend to be executed more times than loop-exit edges.
451+
if let Some(reloop_target) = self.find_good_reloop_edge(traversal, &candidate_successors) {
455452
debug!("Selecting reloop target {reloop_target:?} to get an expression");
456-
return reloop_target;
453+
return Some(reloop_target);
457454
}
458455

459-
// We couldn't identify a "good" edge, so just choose any edge that
460-
// doesn't already have a counter.
461-
let arbitrary_target = self
462-
.bcb_successors(from_bcb)
463-
.iter()
464-
.copied()
465-
.find(|&to_bcb| self.edge_has_no_counter(from_bcb, to_bcb))
466-
.expect("precondition: at least one out-edge without a counter");
456+
// We couldn't identify a "good" edge, so just choose an arbitrary one.
457+
let arbitrary_target = candidate_successors.first().copied()?;
467458
debug!(?arbitrary_target, "selecting arbitrary out-edge to get an expression");
468-
arbitrary_target
459+
Some(arbitrary_target)
469460
}
470461

471-
/// Tries to find an edge that leads back to the top of a loop, and that
472-
/// doesn't already have a counter. Such edges are good candidates to
473-
/// be given an expression (instead of a physical counter), because they
474-
/// will tend to be executed more times than a loop-exit edge.
462+
/// Given a set of candidate out-edges (represented by their successor node),
463+
/// tries to find one that leads back to the top of a loop.
464+
///
465+
/// Reloop edges are good candidates for counter expressions, because they
466+
/// will tend to be executed more times than a loop-exit edge, so it's nice
467+
/// for them to be able to avoid a physical counter increment.
475468
fn find_good_reloop_edge(
476469
&self,
477470
traversal: &TraverseCoverageGraphWithLoops<'_>,
478-
from_bcb: BasicCoverageBlock,
471+
candidate_successors: &[BasicCoverageBlock],
479472
) -> Option<BasicCoverageBlock> {
480-
let successors = self.bcb_successors(from_bcb);
473+
// If there are no candidates, avoid iterating over the loop stack.
474+
if candidate_successors.is_empty() {
475+
return None;
476+
}
481477

482478
// Consider each loop on the current traversal context stack, top-down.
483479
for reloop_bcbs in traversal.reloop_bcbs_per_loop() {
484-
let mut all_edges_exit_this_loop = true;
485-
486-
// Try to find an out-edge that doesn't exit this loop and doesn't
487-
// already have a counter.
488-
for &target_bcb in successors {
480+
// Try to find a candidate edge that doesn't exit this loop.
481+
for &target_bcb in candidate_successors {
489482
// An edge is a reloop edge if its target dominates any BCB that has
490483
// an edge back to the loop header. (Otherwise it's an exit edge.)
491484
let is_reloop_edge = reloop_bcbs.iter().any(|&reloop_bcb| {
492485
self.basic_coverage_blocks.dominates(target_bcb, reloop_bcb)
493486
});
494-
495487
if is_reloop_edge {
496-
all_edges_exit_this_loop = false;
497-
if self.edge_has_no_counter(from_bcb, target_bcb) {
498-
// We found a good out-edge to be given an expression.
499-
return Some(target_bcb);
500-
}
501-
// Keep looking for another reloop edge without a counter.
502-
} else {
503-
// This edge exits the loop.
488+
// We found a good out-edge to be given an expression.
489+
return Some(target_bcb);
504490
}
505491
}
506492

507-
if !all_edges_exit_this_loop {
508-
// We found one or more reloop edges, but all of them already
509-
// have counters. Let the caller choose one of the other edges.
510-
debug!("All reloop edges had counters; skipping the other loops");
511-
return None;
512-
}
513-
514-
// All of the out-edges exit this loop, so keep looking for a good
515-
// reloop edge for one of the outer loops.
493+
// All of the candidate edges exit this loop, so keep looking
494+
// for a good reloop edge for one of the outer loops.
516495
}
517496

518497
None

0 commit comments

Comments
 (0)