Skip to content

Commit 8be70c7

Browse files
committed
coverage: Avoid referring to out-edges as "branches"
This makes the graph terminology a bit more consistent, and avoids potential confusion with branch coverage.
1 parent 96d545a commit 8be70c7

File tree

1 file changed

+80
-97
lines changed

1 file changed

+80
-97
lines changed

compiler/rustc_mir_transform/src/coverage/counters.rs

+80-97
Original file line numberDiff line numberDiff line change
@@ -253,12 +253,6 @@ impl<'a> MakeBcbCounters<'a> {
253253
Self { coverage_counters, basic_coverage_blocks }
254254
}
255255

256-
/// If two `BasicCoverageBlock`s branch from another `BasicCoverageBlock`, one of the branches
257-
/// can be counted by `Expression` by subtracting the other branch from the branching
258-
/// block. Otherwise, the `BasicCoverageBlock` executed the least should have the `Counter`.
259-
/// One way to predict which branch executes the least is by considering loops. A loop is exited
260-
/// at a branch, so the branch that jumps to a `BasicCoverageBlock` outside the loop is almost
261-
/// always executed less than the branch that does not exit the loop.
262256
fn make_bcb_counters(&mut self, bcb_needs_counter: impl Fn(BasicCoverageBlock) -> bool) {
263257
debug!("make_bcb_counters(): adding a counter or expression to each BasicCoverageBlock");
264258

@@ -273,7 +267,7 @@ impl<'a> MakeBcbCounters<'a> {
273267
while let Some(bcb) = traversal.next() {
274268
let _span = debug_span!("traversal", ?bcb).entered();
275269
if bcb_needs_counter(bcb) {
276-
self.make_node_and_branch_counters(&traversal, bcb);
270+
self.make_node_counter_and_out_edge_counters(&traversal, bcb);
277271
}
278272
}
279273

@@ -284,74 +278,66 @@ impl<'a> MakeBcbCounters<'a> {
284278
);
285279
}
286280

281+
/// Make sure the given node has a node counter, and then make sure each of
282+
/// its out-edges has an edge counter (if appropriate).
287283
#[instrument(level = "debug", skip(self, traversal))]
288-
fn make_node_and_branch_counters(
284+
fn make_node_counter_and_out_edge_counters(
289285
&mut self,
290286
traversal: &TraverseCoverageGraphWithLoops<'_>,
291287
from_bcb: BasicCoverageBlock,
292288
) {
293289
// First, ensure that this node has a counter of some kind.
294-
// We might also use its term later to compute one of the branch counters.
290+
// We might also use that counter to compute one of the out-edge counters.
295291
let from_bcb_operand = self.get_or_make_counter_operand(from_bcb);
296292

297-
let branch_target_bcbs = self.basic_coverage_blocks.successors[from_bcb].as_slice();
293+
let successors = self.basic_coverage_blocks.successors[from_bcb].as_slice();
298294

299295
// If this node doesn't have multiple out-edges, or all of its out-edges
300296
// already have counters, then we don't need to create edge counters.
301-
let needs_branch_counters = branch_target_bcbs.len() > 1
302-
&& branch_target_bcbs
303-
.iter()
304-
.any(|&to_bcb| self.branch_has_no_counter(from_bcb, to_bcb));
305-
if !needs_branch_counters {
297+
let needs_out_edge_counters = successors.len() > 1
298+
&& successors.iter().any(|&to_bcb| self.edge_has_no_counter(from_bcb, to_bcb));
299+
if !needs_out_edge_counters {
306300
return;
307301
}
308302

309-
debug!(
310-
"{from_bcb:?} has some branch(es) without counters:\n {}",
311-
branch_target_bcbs
312-
.iter()
313-
.map(|&to_bcb| {
314-
format!("{from_bcb:?}->{to_bcb:?}: {:?}", self.branch_counter(from_bcb, to_bcb))
315-
})
316-
.collect::<Vec<_>>()
317-
.join("\n "),
318-
);
303+
if tracing::enabled!(tracing::Level::DEBUG) {
304+
let _span =
305+
debug_span!("node has some out-edges without counters", ?from_bcb).entered();
306+
for &to_bcb in successors {
307+
debug!(?to_bcb, counter=?self.edge_counter(from_bcb, to_bcb));
308+
}
309+
}
319310

320-
// Of the branch edges that don't have counters yet, one can be given an expression
321-
// (computed from the other edges) instead of a dedicated counter.
322-
let expression_to_bcb = self.choose_preferred_expression_branch(traversal, from_bcb);
311+
// Of the out-edges that don't have counters yet, one can be given an expression
312+
// (computed from the other out-edges) instead of a dedicated counter.
313+
let expression_to_bcb = self.choose_out_edge_for_expression(traversal, from_bcb);
323314

324-
// For each branch arm other than the one that was chosen to get an expression,
315+
// For each out-edge other than the one that was chosen to get an expression,
325316
// ensure that it has a counter (existing counter/expression or a new counter),
326-
// and accumulate the corresponding terms into a single sum term.
327-
let sum_of_all_other_branches: BcbCounter = {
328-
let _span = debug_span!("sum_of_all_other_branches", ?expression_to_bcb).entered();
329-
branch_target_bcbs
317+
// and accumulate the corresponding counters into a single sum expression.
318+
let sum_of_all_other_out_edges: BcbCounter = {
319+
let _span = debug_span!("sum_of_all_other_out_edges", ?expression_to_bcb).entered();
320+
successors
330321
.iter()
331322
.copied()
332-
// Skip the chosen branch, since we'll calculate it from the other branches.
323+
// Skip the chosen edge, since we'll calculate its count from this sum.
333324
.filter(|&to_bcb| to_bcb != expression_to_bcb)
334325
.fold(None, |accum, to_bcb| {
335326
let _span = debug_span!("to_bcb", ?accum, ?to_bcb).entered();
336-
let branch_counter = self.get_or_make_edge_counter_operand(from_bcb, to_bcb);
337-
Some(self.coverage_counters.make_sum_expression(accum, branch_counter))
327+
let edge_counter = self.get_or_make_edge_counter_operand(from_bcb, to_bcb);
328+
Some(self.coverage_counters.make_sum_expression(accum, edge_counter))
338329
})
339-
.expect("there must be at least one other branch")
330+
.expect("there must be at least one other out-edge")
340331
};
341332

342-
// For the branch that was chosen to get an expression, create that expression
343-
// by taking the count of the node we're branching from, and subtracting the
344-
// sum of all the other branches.
345-
debug!(
346-
"Making an expression for the selected expression_branch: \
347-
{expression_to_bcb:?} (expression_branch predecessors: {:?})",
348-
self.bcb_predecessors(expression_to_bcb),
349-
);
333+
// Now create an expression for the chosen edge, by taking the counter
334+
// for its source node and subtracting the sum of its sibling out-edges.
350335
let expression = self.coverage_counters.make_expression(
351336
from_bcb_operand,
352337
Op::Subtract,
353-
sum_of_all_other_branches,
338+
sum_of_all_other_out_edges,
354339
);
340+
355341
debug!("{expression_to_bcb:?} gets an expression: {expression:?}");
356342
if self.basic_coverage_blocks.bcb_has_multiple_in_edges(expression_to_bcb) {
357343
self.coverage_counters.set_bcb_edge_counter(from_bcb, expression_to_bcb, expression);
@@ -442,82 +428,79 @@ impl<'a> MakeBcbCounters<'a> {
442428
self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter_kind)
443429
}
444430

445-
/// Select a branch for the expression, either the recommended `reloop_branch`, or if none was
446-
/// found, select any branch.
447-
fn choose_preferred_expression_branch(
431+
/// Choose one of the out-edges of `from_bcb` to receive an expression
432+
/// instead of a physical counter, and returns that edge's target node.
433+
///
434+
/// - Precondition: The node must have at least one out-edge without a counter.
435+
/// - Postcondition: The selected edge does not have an edge counter.
436+
fn choose_out_edge_for_expression(
448437
&self,
449438
traversal: &TraverseCoverageGraphWithLoops<'_>,
450439
from_bcb: BasicCoverageBlock,
451440
) -> BasicCoverageBlock {
452-
let good_reloop_branch = self.find_good_reloop_branch(traversal, from_bcb);
453-
if let Some(reloop_target) = good_reloop_branch {
454-
assert!(self.branch_has_no_counter(from_bcb, reloop_target));
441+
if let Some(reloop_target) = self.find_good_reloop_edge(traversal, from_bcb) {
442+
assert!(self.edge_has_no_counter(from_bcb, reloop_target));
455443
debug!("Selecting reloop target {reloop_target:?} to get an expression");
456-
reloop_target
457-
} else {
458-
let &branch_without_counter = self
459-
.bcb_successors(from_bcb)
460-
.iter()
461-
.find(|&&to_bcb| self.branch_has_no_counter(from_bcb, to_bcb))
462-
.expect(
463-
"needs_branch_counters was `true` so there should be at least one \
464-
branch",
465-
);
466-
debug!(
467-
"Selecting any branch={:?} that still needs a counter, to get the \
468-
`Expression` because there was no `reloop_branch`, or it already had a \
469-
counter",
470-
branch_without_counter
471-
);
472-
branch_without_counter
444+
return reloop_target;
473445
}
446+
447+
// We couldn't identify a "good" edge, so just choose any edge that
448+
// doesn't already have a counter.
449+
let arbitrary_target = self
450+
.bcb_successors(from_bcb)
451+
.iter()
452+
.copied()
453+
.find(|&to_bcb| self.edge_has_no_counter(from_bcb, to_bcb))
454+
.expect("precondition: at least one out-edge without a counter");
455+
debug!(?arbitrary_target, "selecting arbitrary out-edge to get an expression");
456+
arbitrary_target
474457
}
475458

476-
/// Tries to find a branch that leads back to the top of a loop, and that
477-
/// doesn't already have a counter. Such branches are good candidates to
459+
/// Tries to find an edge that leads back to the top of a loop, and that
460+
/// doesn't already have a counter. Such edges are good candidates to
478461
/// be given an expression (instead of a physical counter), because they
479-
/// will tend to be executed more times than a loop-exit branch.
480-
fn find_good_reloop_branch(
462+
/// will tend to be executed more times than a loop-exit edge.
463+
fn find_good_reloop_edge(
481464
&self,
482465
traversal: &TraverseCoverageGraphWithLoops<'_>,
483466
from_bcb: BasicCoverageBlock,
484467
) -> Option<BasicCoverageBlock> {
485-
let branch_target_bcbs = self.bcb_successors(from_bcb);
468+
let successors = self.bcb_successors(from_bcb);
486469

487470
// Consider each loop on the current traversal context stack, top-down.
488471
for reloop_bcbs in traversal.reloop_bcbs_per_loop() {
489-
let mut all_branches_exit_this_loop = true;
472+
let mut all_edges_exit_this_loop = true;
490473

491-
// Try to find a branch that doesn't exit this loop and doesn't
474+
// Try to find an out-edge that doesn't exit this loop and doesn't
492475
// already have a counter.
493-
for &branch_target_bcb in branch_target_bcbs {
494-
// A branch is a reloop branch if it dominates any BCB that has
495-
// an edge back to the loop header. (Other branches are exits.)
496-
let is_reloop_branch = reloop_bcbs.iter().any(|&reloop_bcb| {
497-
self.basic_coverage_blocks.dominates(branch_target_bcb, reloop_bcb)
476+
for &target_bcb in successors {
477+
// An edge is a reloop edge if its target dominates any BCB that has
478+
// an edge back to the loop header. (Otherwise it's an exit edge.)
479+
let is_reloop_edge = reloop_bcbs.iter().any(|&reloop_bcb| {
480+
self.basic_coverage_blocks.dominates(target_bcb, reloop_bcb)
498481
});
499482

500-
if is_reloop_branch {
501-
all_branches_exit_this_loop = false;
502-
if self.branch_has_no_counter(from_bcb, branch_target_bcb) {
503-
// We found a good branch to be given an expression.
504-
return Some(branch_target_bcb);
483+
if is_reloop_edge {
484+
all_edges_exit_this_loop = false;
485+
if self.edge_has_no_counter(from_bcb, target_bcb) {
486+
// We found a good out-edge to be given an expression.
487+
return Some(target_bcb);
505488
}
506-
// Keep looking for another reloop branch without a counter.
489+
// Keep looking for another reloop edge without a counter.
507490
} else {
508-
// This branch exits the loop.
491+
// This edge exits the loop.
509492
}
510493
}
511494

512-
if !all_branches_exit_this_loop {
513-
// We found one or more reloop branches, but all of them already
514-
// have counters. Let the caller choose one of the exit branches.
515-
debug!("All reloop branches had counters; skip checking the other loops");
495+
if !all_edges_exit_this_loop {
496+
// We found one or more reloop edges, but all of them already
497+
// have counters. Let the caller choose one of the other edges.
498+
debug!("All reloop edges had counters; skipping the other loops");
516499
return None;
517500
}
518501

519-
// All of the branches exit this loop, so keep looking for a good
520-
// reloop branch for one of the outer loops.
502+
// All of the out-edges exit this loop, so keep looking for a good
503+
// reloop edge for one of the outer loops.
521504
}
522505

523506
None
@@ -534,15 +517,15 @@ impl<'a> MakeBcbCounters<'a> {
534517
}
535518

536519
#[inline]
537-
fn branch_has_no_counter(
520+
fn edge_has_no_counter(
538521
&self,
539522
from_bcb: BasicCoverageBlock,
540523
to_bcb: BasicCoverageBlock,
541524
) -> bool {
542-
self.branch_counter(from_bcb, to_bcb).is_none()
525+
self.edge_counter(from_bcb, to_bcb).is_none()
543526
}
544527

545-
fn branch_counter(
528+
fn edge_counter(
546529
&self,
547530
from_bcb: BasicCoverageBlock,
548531
to_bcb: BasicCoverageBlock,

0 commit comments

Comments
 (0)