Skip to content

Commit c412cd4

Browse files
committed
coverage: Indicate whether a block's successors allow BCB chaining
1 parent 6d1c396 commit c412cd4

File tree

2 files changed

+63
-28
lines changed

2 files changed

+63
-28
lines changed

compiler/rustc_mir_transform/src/coverage/graph.rs

+62-28
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ impl CoverageGraph {
3939
let bcb_data = &bcbs[bcb];
4040
let mut bcb_successors = Vec::new();
4141
for successor in bcb_filtered_successors(mir_body[bcb_data.last_bb()].terminator())
42+
.into_iter()
4243
.filter_map(|successor_bb| bb_to_bcb[successor_bb])
4344
{
4445
if !seen[successor] {
@@ -122,11 +123,8 @@ impl CoverageGraph {
122123

123124
let term = mir_body[bb].terminator();
124125

125-
match term.kind {
126-
TerminatorKind::Return { .. }
127-
| TerminatorKind::UnwindTerminate(_)
128-
| TerminatorKind::Yield { .. }
129-
| TerminatorKind::SwitchInt { .. } => {
126+
match bcb_filtered_successors(term) {
127+
CoverageSuccessors::NotChainable(_) => {
130128
// The `bb` has more than one _outgoing_ edge, or exits the function. Save the
131129
// current sequence of `basic_blocks` gathered to this point, as a new
132130
// `BasicCoverageBlockData`.
@@ -153,16 +151,7 @@ impl CoverageGraph {
153151
// for a coverage region containing the `Terminator` that began the panic. This
154152
// is as intended. (See Issue #78544 for a possible future option to support
155153
// coverage in test programs that panic.)
156-
TerminatorKind::Goto { .. }
157-
| TerminatorKind::UnwindResume
158-
| TerminatorKind::Unreachable
159-
| TerminatorKind::Drop { .. }
160-
| TerminatorKind::Call { .. }
161-
| TerminatorKind::CoroutineDrop
162-
| TerminatorKind::Assert { .. }
163-
| TerminatorKind::FalseEdge { .. }
164-
| TerminatorKind::FalseUnwind { .. }
165-
| TerminatorKind::InlineAsm { .. } => {}
154+
CoverageSuccessors::Chainable(_) => {}
166155
}
167156
}
168157

@@ -349,22 +338,67 @@ impl BasicCoverageBlockData {
349338
}
350339
}
351340

341+
/// Holds the coverage-relevant successors of a basic block's terminator, and
342+
/// indicates whether that block can potentially be combined into the same BCB
343+
/// as its sole successor.
344+
#[derive(Clone, Copy, Debug)]
345+
enum CoverageSuccessors<'a> {
346+
/// The terminator has exactly one straight-line successor, so its block can
347+
/// potentially be combined into the same BCB as that successor.
348+
Chainable(BasicBlock),
349+
/// The block cannot be combined into the same BCB as its successor(s).
350+
NotChainable(&'a [BasicBlock]),
351+
}
352+
353+
impl IntoIterator for CoverageSuccessors<'_> {
354+
type Item = BasicBlock;
355+
type IntoIter = impl DoubleEndedIterator<Item = Self::Item>;
356+
357+
fn into_iter(self) -> Self::IntoIter {
358+
match self {
359+
Self::Chainable(bb) => Some(bb).into_iter().chain((&[]).iter().copied()),
360+
Self::NotChainable(bbs) => None.into_iter().chain(bbs.iter().copied()),
361+
}
362+
}
363+
}
364+
352365
// Returns the subset of a block's successors that are relevant to the coverage
353366
// graph, i.e. those that do not represent unwinds or false edges.
354367
// FIXME(#78544): MIR InstrumentCoverage: Improve coverage of `#[should_panic]` tests and
355368
// `catch_unwind()` handlers.
356-
fn bcb_filtered_successors<'a, 'tcx>(
357-
terminator: &'a Terminator<'tcx>,
358-
) -> impl Iterator<Item = BasicBlock> + Captures<'a> + Captures<'tcx> {
359-
let take_n_successors = match terminator.kind {
360-
// SwitchInt successors are never unwinds, so all of them should be traversed.
361-
TerminatorKind::SwitchInt { .. } => usize::MAX,
362-
// For all other kinds, return only the first successor (if any), ignoring any
363-
// unwind successors.
364-
_ => 1,
365-
};
366-
367-
terminator.successors().take(take_n_successors)
369+
fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> CoverageSuccessors<'a> {
370+
use TerminatorKind::*;
371+
match terminator.kind {
372+
// A switch terminator can have many coverage-relevant successors.
373+
// (If there is exactly one successor, we still treat it as not chainable.)
374+
SwitchInt { ref targets, .. } => CoverageSuccessors::NotChainable(targets.all_targets()),
375+
376+
// A yield terminator has exactly 1 successor, but should not be chained,
377+
// because its resume edge has a different execution count.
378+
Yield { ref resume, .. } => CoverageSuccessors::NotChainable(std::slice::from_ref(resume)),
379+
380+
// These terminators have exactly one coverage-relevant successor,
381+
// and can be chained into it.
382+
Assert { target, .. }
383+
| Drop { target, .. }
384+
| FalseEdge { real_target: target, .. }
385+
| FalseUnwind { real_target: target, .. }
386+
| Goto { target } => CoverageSuccessors::Chainable(target),
387+
388+
// These terminators can normally be chained, except when they have no
389+
// successor because they are known to diverge.
390+
Call { target: maybe_target, .. } | InlineAsm { destination: maybe_target, .. } => {
391+
match maybe_target {
392+
Some(target) => CoverageSuccessors::Chainable(target),
393+
None => CoverageSuccessors::NotChainable(&[]),
394+
}
395+
}
396+
397+
// These terminators have no coverage-relevant successors.
398+
CoroutineDrop | Return | Unreachable | UnwindResume | UnwindTerminate(_) => {
399+
CoverageSuccessors::NotChainable(&[])
400+
}
401+
}
368402
}
369403

370404
/// Maintains separate worklists for each loop in the BasicCoverageBlock CFG, plus one for the
@@ -542,7 +576,7 @@ fn short_circuit_preorder<'a, 'tcx, F, Iter>(
542576
) -> impl Iterator<Item = BasicBlock> + Captures<'a> + Captures<'tcx>
543577
where
544578
F: Fn(BasicBlock) -> Iter,
545-
Iter: Iterator<Item = BasicBlock>,
579+
Iter: IntoIterator<Item = BasicBlock>,
546580
{
547581
let mut visited = BitSet::new_empty(body.basic_blocks.len());
548582
let mut worklist = vec![mir::START_BLOCK];

compiler/rustc_mir_transform/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![feature(box_patterns)]
55
#![feature(cow_is_borrowed)]
66
#![feature(decl_macro)]
7+
#![feature(impl_trait_in_assoc_type)]
78
#![feature(is_sorted)]
89
#![feature(let_chains)]
910
#![feature(map_try_insert)]

0 commit comments

Comments
 (0)