mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

coverage: Eliminate BcbBranch

Browse Source 
`BcbBranch` represented an out-edge of a coverage graph node, but would
silently refer to a node instead in cases where that node only had one in-edge.

Instead we now refer to a graph edge as a `(from_bcb, to_bcb)` pair, or
sometimes as just one of those nodes when the other node is implied by the
surrounding context. The case of sole in-edges is handled by special code added
directly to `get_or_make_edge_counter_operand`.
This commit is contained in:
Zalathar 2023-11-01 11:40:12 +11:00
parent 276a32994e
commit 3b9d7037c9
2 changed files with 69 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
compiler/rustc_mir_transform/src/coverage/counters.rs
View File

@ -1,13 +1,11 @@
use super::graph;
use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops};
use rustc_data_structures::fx::FxHashMap; 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;
use rustc_middle::mir::coverage::*; use rustc_middle::mir::coverage::*;
use super::graph::{BasicCoverageBlock, CoverageGraph, TraverseCoverageGraphWithLoops};
use std::fmt::{self, Debug}; use std::fmt::{self, Debug};
/// The coverage counter or counter expression associated with a particular /// The coverage counter or counter expression associated with a particular
@ -259,49 +257,46 @@ fn make_node_and_branch_counters(
// We might also use its term later to compute one of the branch counters. // We might also use its term later to compute one of the branch counters.
let from_bcb_operand = self.get_or_make_counter_operand(from_bcb); let from_bcb_operand = self.get_or_make_counter_operand(from_bcb);
let branches = self.bcb_branches(from_bcb); let branch_target_bcbs = self.basic_coverage_blocks.successors[from_bcb].as_slice();
// If this node doesn't have multiple out-edges, or all of its out-edges // If this node doesn't have multiple out-edges, or all of its out-edges
// already have counters, then we don't need to create edge counters. // already have counters, then we don't need to create edge counters.
let needs_branch_counters = let needs_branch_counters = branch_target_bcbs.len() > 1
branches.len() > 1 && branches.iter().any(|branch| self.branch_has_no_counter(branch)); && branch_target_bcbs
.iter()
.any(|&to_bcb| self.branch_has_no_counter(from_bcb, to_bcb));
if !needs_branch_counters { if !needs_branch_counters {
return; return;
} }
debug!( debug!(
"{from_bcb:?} has some branch(es) without counters:\n {}", "{from_bcb:?} has some branch(es) without counters:\n {}",
branches branch_target_bcbs
.iter() .iter()
.map(|branch| { format!("{:?}: {:?}", branch, self.branch_counter(branch)) }) .map(|&to_bcb| {
format!("{from_bcb:?}->{to_bcb:?}: {:?}", self.branch_counter(from_bcb, to_bcb))
})
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join("\n "), .join("\n "),
); );
// Use the `traversal` state to decide if a subset of the branches exit a loop, making it // Of the branch edges that don't have counters yet, one can be given an expression
// likely that branch is executed less than branches that do not exit the same loop. In this // (computed from the other edges) instead of a dedicated counter.
// case, any branch that does not exit the loop (and has not already been assigned a let expression_to_bcb = self.choose_preferred_expression_branch(traversal, from_bcb);
// counter) should be counted by expression, if possible. (If a preferred expression branch
// is not selected based on the loop context, select any branch without an existing
// counter.)
let expression_branch = self.choose_preferred_expression_branch(traversal, &branches);
// For each branch arm other than the one that was chosen to get an expression, // For each branch arm other than the one that was chosen to get an expression,
// ensure that it has a counter (existing counter/expression or a new counter), // ensure that it has a counter (existing counter/expression or a new counter),
// and accumulate the corresponding terms into a single sum term. // and accumulate the corresponding terms into a single sum term.
let sum_of_all_other_branches: BcbCounter = { let sum_of_all_other_branches: BcbCounter = {
let _span = debug_span!("sum_of_all_other_branches", ?expression_branch).entered(); let _span = debug_span!("sum_of_all_other_branches", ?expression_to_bcb).entered();
branches branch_target_bcbs
.into_iter() .iter()
.copied()
// Skip the chosen branch, since we'll calculate it from the other branches. // Skip the chosen branch, since we'll calculate it from the other branches.
.filter(|branch| branch != &expression_branch) .filter(|&to_bcb| to_bcb != expression_to_bcb)
.fold(None, |accum, branch| { .fold(None, |accum, to_bcb| {
let _span = debug_span!("branch", ?accum, ?branch).entered(); let _span = debug_span!("to_bcb", ?accum, ?to_bcb).entered();
let branch_counter = if branch.is_only_path_to_target() { let branch_counter = self.get_or_make_edge_counter_operand(from_bcb, to_bcb);
self.get_or_make_counter_operand(branch.target_bcb)
} else {
self.get_or_make_edge_counter_operand(from_bcb, branch.target_bcb)
};
Some(self.coverage_counters.make_sum_expression(accum, branch_counter)) Some(self.coverage_counters.make_sum_expression(accum, branch_counter))
}) })
.expect("there must be at least one other branch") .expect("there must be at least one other branch")
@ -311,22 +306,20 @@ fn make_node_and_branch_counters(
// by taking the count of the node we're branching from, and subtracting the // by taking the count of the node we're branching from, and subtracting the
// sum of all the other branches. // sum of all the other branches.
debug!( debug!(
"Making an expression for the selected expression_branch: {:?} \ "Making an expression for the selected expression_branch: \
(expression_branch predecessors: {:?})", {expression_to_bcb:?} (expression_branch predecessors: {:?})",
expression_branch, self.bcb_predecessors(expression_to_bcb),
self.bcb_predecessors(expression_branch.target_bcb),
); );
let expression = self.coverage_counters.make_expression( let expression = self.coverage_counters.make_expression(
from_bcb_operand, from_bcb_operand,
Op::Subtract, Op::Subtract,
sum_of_all_other_branches, sum_of_all_other_branches,
); );
debug!("{:?} gets an expression: {:?}", expression_branch, expression); debug!("{expression_to_bcb:?} gets an expression: {expression:?}");
let bcb = expression_branch.target_bcb; if self.basic_coverage_blocks.bcb_has_multiple_in_edges(expression_to_bcb) {
if expression_branch.is_only_path_to_target() { self.coverage_counters.set_bcb_edge_counter(from_bcb, expression_to_bcb, expression);
self.coverage_counters.set_bcb_counter(bcb, expression);
} else { } else {
self.coverage_counters.set_bcb_edge_counter(from_bcb, bcb, expression); self.coverage_counters.set_bcb_counter(expression_to_bcb, expression);
} }
} }
@ -383,10 +376,16 @@ fn get_or_make_edge_counter_operand(
from_bcb: BasicCoverageBlock, from_bcb: BasicCoverageBlock,
to_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock,
) -> BcbCounter { ) -> BcbCounter {
// If the target BCB has only one in-edge (i.e. this one), then create
// a node counter instead, since it will have the same value.
if !self.basic_coverage_blocks.bcb_has_multiple_in_edges(to_bcb) {
assert_eq!([from_bcb].as_slice(), self.basic_coverage_blocks.predecessors[to_bcb]);
return self.get_or_make_counter_operand(to_bcb);
}
// If the source BCB has only one successor (assumed to be the given target), an edge // If the source BCB has only one successor (assumed to be the given target), an edge
// counter is unnecessary. Just get or make a counter for the source BCB. // counter is unnecessary. Just get or make a counter for the source BCB.
let successors = self.bcb_successors(from_bcb).iter(); if self.bcb_successors(from_bcb).len() == 1 {
if successors.len() == 1 {
return self.get_or_make_counter_operand(from_bcb); return self.get_or_make_counter_operand(from_bcb);
} }
@ -409,16 +408,19 @@ fn get_or_make_edge_counter_operand(
fn choose_preferred_expression_branch( fn choose_preferred_expression_branch(
&self, &self,
traversal: &TraverseCoverageGraphWithLoops<'_>, traversal: &TraverseCoverageGraphWithLoops<'_>,
branches: &[BcbBranch], from_bcb: BasicCoverageBlock,
) -> BcbBranch { ) -> BasicCoverageBlock {
let good_reloop_branch = self.find_good_reloop_branch(traversal, branches); let good_reloop_branch = self.find_good_reloop_branch(traversal, from_bcb);
if let Some(reloop_branch) = good_reloop_branch { if let Some(reloop_target) = good_reloop_branch {
assert!(self.branch_has_no_counter(&reloop_branch)); assert!(self.branch_has_no_counter(from_bcb, reloop_target));
debug!("Selecting reloop branch {reloop_branch:?} to get an expression"); debug!("Selecting reloop target {reloop_target:?} to get an expression");
reloop_branch reloop_target
} else { } else {
let &branch_without_counter = let &branch_without_counter = self
branches.iter().find(|&branch| self.branch_has_no_counter(branch)).expect( .bcb_successors(from_bcb)
.iter()
.find(|&&to_bcb| self.branch_has_no_counter(from_bcb, to_bcb))
.expect(
"needs_branch_counters was `true` so there should be at least one \ "needs_branch_counters was `true` so there should be at least one \
branch", branch",
); );
@ -439,26 +441,28 @@ fn choose_preferred_expression_branch(
fn find_good_reloop_branch( fn find_good_reloop_branch(
&self, &self,
traversal: &TraverseCoverageGraphWithLoops<'_>, traversal: &TraverseCoverageGraphWithLoops<'_>,
branches: &[BcbBranch], from_bcb: BasicCoverageBlock,
) -> Option<BcbBranch> { ) -> Option<BasicCoverageBlock> {
let branch_target_bcbs = self.bcb_successors(from_bcb);
// Consider each loop on the current traversal context stack, top-down. // Consider each loop on the current traversal context stack, top-down.
for reloop_bcbs in traversal.reloop_bcbs_per_loop() { for reloop_bcbs in traversal.reloop_bcbs_per_loop() {
let mut all_branches_exit_this_loop = true; let mut all_branches_exit_this_loop = true;
// Try to find a branch that doesn't exit this loop and doesn't // Try to find a branch that doesn't exit this loop and doesn't
// already have a counter. // already have a counter.
for &branch in branches { for &branch_target_bcb in branch_target_bcbs {
// A branch is a reloop branch if it dominates any BCB that has // A branch is a reloop branch if it dominates any BCB that has
// an edge back to the loop header. (Other branches are exits.) // an edge back to the loop header. (Other branches are exits.)
let is_reloop_branch = reloop_bcbs.iter().any(|&reloop_bcb| { let is_reloop_branch = reloop_bcbs.iter().any(|&reloop_bcb| {
self.basic_coverage_blocks.dominates(branch.target_bcb, reloop_bcb) self.basic_coverage_blocks.dominates(branch_target_bcb, reloop_bcb)
}); });
if is_reloop_branch { if is_reloop_branch {
all_branches_exit_this_loop = false; all_branches_exit_this_loop = false;
if self.branch_has_no_counter(&branch) { if self.branch_has_no_counter(from_bcb, branch_target_bcb) {
// We found a good branch to be given an expression. // We found a good branch to be given an expression.
return Some(branch); return Some(branch_target_bcb);
} }
// Keep looking for another reloop branch without a counter. // Keep looking for another reloop branch without a counter.
} else { } else {
@ -491,20 +495,20 @@ fn bcb_successors(&self, bcb: BasicCoverageBlock) -> &[BasicCoverageBlock] {
} }
#[inline] #[inline]
fn bcb_branches(&self, from_bcb: BasicCoverageBlock) -> Vec<BcbBranch> { fn branch_has_no_counter(
self.bcb_successors(from_bcb) &self,
.iter() from_bcb: BasicCoverageBlock,
.map(|&to_bcb| BcbBranch::from_to(from_bcb, to_bcb, self.basic_coverage_blocks)) to_bcb: BasicCoverageBlock,
.collect::<Vec<_>>() ) -> bool {
self.branch_counter(from_bcb, to_bcb).is_none()
} }
fn branch_has_no_counter(&self, branch: &BcbBranch) -> bool { fn branch_counter(
self.branch_counter(branch).is_none() &self,
} from_bcb: BasicCoverageBlock,
to_bcb: BasicCoverageBlock,
fn branch_counter(&self, branch: &BcbBranch) -> Option<&BcbCounter> { ) -> Option<&BcbCounter> {
let to_bcb = branch.target_bcb; if self.basic_coverage_blocks.bcb_has_multiple_in_edges(to_bcb) {
if let Some(from_bcb) = branch.edge_from_bcb {
self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb))
} else { } else {
self.coverage_counters.bcb_counters[to_bcb].as_ref() self.coverage_counters.bcb_counters[to_bcb].as_ref()

39
compiler/rustc_mir_transform/src/coverage/graph.rs
View File

@ -333,45 +333,6 @@ pub fn last_bb(&self) -> BasicBlock {
} }
} }
/// Represents a successor from a branching BasicCoverageBlock (such as the arms of a `SwitchInt`)
/// as either the successor BCB itself, if it has only one incoming edge, or the successor _plus_
/// the specific branching BCB, representing the edge between the two. The latter case
/// distinguishes this incoming edge from other incoming edges to the same `target_bcb`.
#[derive(Clone, Copy, PartialEq, Eq)]
pub(super) struct BcbBranch {
pub edge_from_bcb: Option<BasicCoverageBlock>,
pub target_bcb: BasicCoverageBlock,
}
impl BcbBranch {
pub fn from_to(
from_bcb: BasicCoverageBlock,
to_bcb: BasicCoverageBlock,
basic_coverage_blocks: &CoverageGraph,
) -> Self {
let edge_from_bcb = if basic_coverage_blocks.bcb_has_multiple_in_edges(from_bcb) {
Some(from_bcb)
} else {
None
};
Self { edge_from_bcb, target_bcb: to_bcb }
}
pub fn is_only_path_to_target(&self) -> bool {
self.edge_from_bcb.is_none()
}
}
impl std::fmt::Debug for BcbBranch {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(from_bcb) = self.edge_from_bcb {
write!(fmt, "{:?}->{:?}", from_bcb, self.target_bcb)
} else {
write!(fmt, "{:?}", self.target_bcb)
}
}
}
// Returns the subset of a block's successors that are relevant to the coverage // Returns the subset of a block's successors that are relevant to the coverage
// graph, i.e. those that do not represent unwinds or unreachable branches. // graph, i.e. those that do not represent unwinds or unreachable branches.
// FIXME(#78544): MIR InstrumentCoverage: Improve coverage of `#[should_panic]` tests and // FIXME(#78544): MIR InstrumentCoverage: Improve coverage of `#[should_panic]` tests and