Skip to content

Commit 39ba191

Browse files
committed
coverage: Use CoverageKind::Mappings in the MIR coverage instrumentor
1 parent c510440 commit 39ba191

File tree

8 files changed

+56
-112
lines changed

8 files changed

+56
-112
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ pub struct FunctionCoverage<'tcx> {
2222
source_hash: u64,
2323
is_used: bool,
2424

25-
/// Tracks which counters have been seen, to avoid duplicate mappings
26-
/// that might be introduced by MIR inlining.
25+
/// Tracks which counters have been seen, so that we can identify mappings
26+
/// to counters that were optimized out, and set them to zero.
2727
counters_seen: BitSet<CounterId>,
2828
expressions: IndexVec<ExpressionId, Option<Expression>>,
2929
mappings: Vec<Mapping>,
@@ -135,19 +135,44 @@ impl<'tcx> FunctionCoverage<'tcx> {
135135
pub(crate) fn finalize(&mut self) {
136136
self.assert_source_hash_is_set();
137137

138-
self.simplify_expressions();
138+
let zero_expressions = self.simplify_expressions_and_get_zero_expressions();
139+
140+
for mapping in self.mappings.iter_mut().filter(|mapping| match mapping.term {
141+
CovTerm::Expression(id) if zero_expressions.contains(&id) => true,
142+
CovTerm::Counter(id) if !self.counters_seen.contains(id) => true,
143+
_ => false,
144+
}) {
145+
mapping.term = CovTerm::Zero;
146+
}
139147

140148
// Sort all of the collected mappings into a predictable order.
141149
self.mappings.sort_unstable();
150+
151+
// Remove redundant mappings. This relies on mappings being sorted
152+
// primarily by code region, and on zero mappings sorting after
153+
// counter/expression mappings.
154+
self.mappings.dedup_by(|b, a| {
155+
if &a.code_region != &b.code_region {
156+
return false;
157+
}
158+
match b.term {
159+
// A zero code region is redundant with any other code region
160+
// (including zero), and can be removed.
161+
CovTerm::Zero => true,
162+
// Otherwise, only remove identical mappings.
163+
_ => a.term == b.term,
164+
}
165+
});
142166
}
143167

144168
/// Perform some simplifications to make the final coverage mappings
145-
/// slightly smaller.
169+
/// slightly smaller. Returns the set of expression IDs that are guaranteed
170+
/// to have a value of zero.
146171
///
147172
/// This method mainly exists to preserve the simplifications that were
148173
/// already being performed by the Rust-side expression renumbering, so that
149174
/// the resulting coverage mappings don't get worse.
150-
fn simplify_expressions(&mut self) {
175+
fn simplify_expressions_and_get_zero_expressions(&mut self) -> FxIndexSet<ExpressionId> {
151176
// The set of expressions that either were optimized out entirely, or
152177
// have zero as both of their operands, and will therefore always have
153178
// a value of zero. Other expressions that refer to these as operands
@@ -191,6 +216,8 @@ impl<'tcx> FunctionCoverage<'tcx> {
191216
zero_expressions.insert(id);
192217
}
193218
}
219+
220+
zero_expressions
194221
}
195222

196223
/// Return the source hash, generated from the HIR node structure, and used to indicate whether

compiler/rustc_mir_transform/src/coverage/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,17 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
314314
})
315315
.collect::<Vec<_>>();
316316

317+
inject_statement(
318+
self.mir_body,
319+
CoverageKind::Mappings { term: counter_kind.as_term() },
320+
mir::START_BLOCK,
321+
code_regions,
322+
);
317323
inject_statement(
318324
self.mir_body,
319325
self.make_mir_coverage_kind(&counter_kind),
320326
self.bcb_leader_bb(bcb),
321-
code_regions,
327+
Vec::new(),
322328
);
323329
}
324330
}

compiler/rustc_mir_transform/src/simplify.rs

Lines changed: 2 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,8 @@
2828
//! return.
2929
3030
use crate::MirPass;
31-
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
31+
use rustc_data_structures::fx::FxIndexSet;
3232
use rustc_index::{Idx, IndexSlice, IndexVec};
33-
use rustc_middle::mir::coverage::*;
3433
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
3534
use rustc_middle::mir::*;
3635
use rustc_middle::ty::TyCtxt;
@@ -336,15 +335,14 @@ pub fn remove_duplicate_unreachable_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut B
336335
}
337336
}
338337

339-
pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
338+
pub fn remove_dead_blocks<'tcx>(_tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
340339
let reachable = traversal::reachable_as_bitset(body);
341340
let num_blocks = body.basic_blocks.len();
342341
if num_blocks == reachable.count() {
343342
return;
344343
}
345344

346345
let basic_blocks = body.basic_blocks.as_mut();
347-
let source_scopes = &body.source_scopes;
348346
let mut replacements: Vec<_> = (0..num_blocks).map(BasicBlock::new).collect();
349347
let mut used_blocks = 0;
350348
for alive_index in reachable.iter() {
@@ -358,10 +356,6 @@ pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
358356
used_blocks += 1;
359357
}
360358

361-
if tcx.sess.instrument_coverage() {
362-
save_unreachable_coverage(basic_blocks, source_scopes, used_blocks);
363-
}
364-
365359
basic_blocks.raw.truncate(used_blocks);
366360

367361
for block in basic_blocks {
@@ -371,93 +365,6 @@ pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
371365
}
372366
}
373367

374-
/// Some MIR transforms can determine at compile time that a sequences of
375-
/// statements will never be executed, so they can be dropped from the MIR.
376-
/// For example, an `if` or `else` block that is guaranteed to never be executed
377-
/// because its condition can be evaluated at compile time, such as by const
378-
/// evaluation: `if false { ... }`.
379-
///
380-
/// Those statements are bypassed by redirecting paths in the CFG around the
381-
/// `dead blocks`; but with `-C instrument-coverage`, the dead blocks usually
382-
/// include `Coverage` statements representing the Rust source code regions to
383-
/// be counted at runtime. Without these `Coverage` statements, the regions are
384-
/// lost, and the Rust source code will show no coverage information.
385-
///
386-
/// What we want to show in a coverage report is the dead code with coverage
387-
/// counts of `0`. To do this, we need to save the code regions, by injecting
388-
/// `Unreachable` coverage statements. These are non-executable statements whose
389-
/// code regions are still recorded in the coverage map, representing regions
390-
/// with `0` executions.
391-
///
392-
/// If there are no live `Counter` `Coverage` statements remaining, we remove
393-
/// `Coverage` statements along with the dead blocks. Since at least one
394-
/// counter per function is required by LLVM (and necessary, to add the
395-
/// `function_hash` to the counter's call to the LLVM intrinsic
396-
/// `instrprof.increment()`).
397-
///
398-
/// The `generator::StateTransform` MIR pass and MIR inlining can create
399-
/// atypical conditions, where all live `Counter`s are dropped from the MIR.
400-
///
401-
/// With MIR inlining we can have coverage counters belonging to different
402-
/// instances in a single body, so the strategy described above is applied to
403-
/// coverage counters from each instance individually.
404-
fn save_unreachable_coverage(
405-
basic_blocks: &mut IndexSlice<BasicBlock, BasicBlockData<'_>>,
406-
source_scopes: &IndexSlice<SourceScope, SourceScopeData<'_>>,
407-
first_dead_block: usize,
408-
) {
409-
// Identify instances that still have some live coverage counters left.
410-
let mut live = FxHashSet::default();
411-
for basic_block in &basic_blocks.raw[0..first_dead_block] {
412-
for statement in &basic_block.statements {
413-
let StatementKind::Coverage(coverage) = &statement.kind else { continue };
414-
let CoverageKind::Counter { .. } = coverage.kind else { continue };
415-
let instance = statement.source_info.scope.inlined_instance(source_scopes);
416-
live.insert(instance);
417-
}
418-
}
419-
420-
for block in &mut basic_blocks.raw[..first_dead_block] {
421-
for statement in &mut block.statements {
422-
let StatementKind::Coverage(_) = &statement.kind else { continue };
423-
let instance = statement.source_info.scope.inlined_instance(source_scopes);
424-
if !live.contains(&instance) {
425-
statement.make_nop();
426-
}
427-
}
428-
}
429-
430-
if live.is_empty() {
431-
return;
432-
}
433-
434-
// Retain coverage for instances that still have some live counters left.
435-
let mut retained_coverage = Vec::new();
436-
for dead_block in &basic_blocks.raw[first_dead_block..] {
437-
for statement in &dead_block.statements {
438-
let StatementKind::Coverage(coverage) = &statement.kind else { continue };
439-
if coverage.code_regions.is_empty() {
440-
continue;
441-
};
442-
let instance = statement.source_info.scope.inlined_instance(source_scopes);
443-
if live.contains(&instance) {
444-
retained_coverage.push((statement.source_info, coverage.code_regions.clone()));
445-
}
446-
}
447-
}
448-
449-
let start_block = &mut basic_blocks[START_BLOCK];
450-
start_block.statements.extend(retained_coverage.into_iter().map(
451-
|(source_info, code_regions)| Statement {
452-
source_info,
453-
kind: StatementKind::Coverage(Box::new(Coverage {
454-
kind: CoverageKind::Unreachable,
455-
code_regions,
456-
})),
457-
},
458-
));
459-
}
460-
461368
pub enum SimplifyLocals {
462369
BeforeConstProp,
463370
Final,

tests/coverage-map/status-quo/issue-84561.cov-map

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
Function name: <issue_84561::Foo as core::cmp::PartialEq>::eq
2-
Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 0a, 00, 13, 00, 00, 0a, 00, 13]
2+
Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 0a, 00, 13]
33
Number of files: 1
44
- file 0 => global file 1
55
Number of expressions: 0
6-
Number of file 0 mappings: 2
6+
Number of file 0 mappings: 1
77
- Code(Counter(0)) at (prev + 4, 10) to (start + 0, 19)
8-
- Code(Zero) at (prev + 0, 10) to (start + 0, 19)
98

109
Function name: <issue_84561::Foo as core::fmt::Debug>::fmt
1110
Raw bytes (29): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 89, 01, 09, 00, 25, 05, 00, 25, 00, 26, 02, 01, 09, 00, 0f, 07, 01, 05, 00, 06]

tests/mir-opt/coverage_graphviz.bar.InstrumentCoverage.0.dot

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@ digraph Cov_0_4 {
22
graph [fontname="Courier, monospace"];
33
node [fontname="Courier, monospace"];
44
edge [fontname="Courier, monospace"];
5-
bcb0__Cov_0_4 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 18:1-20:2<br align="left"/> 19:5-19:9: @0[0]: Coverage::Counter(0) for [$DIR/coverage_graphviz.rs:18:1 - 20:2]<br align="left"/> 20:2-20:2: @0.Return: return</td></tr><tr><td align="left" balign="left">bb0: Return</td></tr></table>>];
5+
bcb0__Cov_0_4 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 18:1-20:2<br align="left"/> 19:5-19:9: @0[0]: Coverage::Counter(0)<br align="left"/> 20:2-20:2: @0.Return: return</td></tr><tr><td align="left" balign="left">bb0: Return</td></tr></table>>];
66
}

tests/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ digraph Cov_0_3 {
22
graph [fontname="Courier, monospace"];
33
node [fontname="Courier, monospace"];
44
edge [fontname="Courier, monospace"];
5-
bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br align="left"/> 13:10-13:10: @5[0]: Coverage::Counter(1) for [$DIR/coverage_graphviz.rs:13:10 - 13:11]</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>];
6-
bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb1:(bcb0 + bcb3) - bcb3) at 12:13-12:18<br align="left"/> 12:13-12:18: @4[0]: Coverage::Expression(1) = Expression(0) - Counter(1) for [$DIR/coverage_graphviz.rs:12:13 - 12:18, $DIR/coverage_graphviz.rs:15:1 - 15:2]<br align="left"/>Expression(bcb1:(bcb0 + bcb3) - bcb3) at 15:2-15:2<br align="left"/> 15:2-15:2: @4.Return: return</td></tr><tr><td align="left" balign="left">bb4: Return</td></tr></table>>];
5+
bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br align="left"/> 13:10-13:10: @5[0]: Coverage::Counter(1)</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>];
6+
bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb1:(bcb0 + bcb3) - bcb3) at 12:13-12:18<br align="left"/> 12:13-12:18: @4[0]: Coverage::Expression(1) = Expression(0) - Counter(1)<br align="left"/>Expression(bcb1:(bcb0 + bcb3) - bcb3) at 15:2-15:2<br align="left"/> 15:2-15:2: @4.Return: return</td></tr><tr><td align="left" balign="left">bb4: Return</td></tr></table>>];
77
bcb1__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb1</td></tr><tr><td align="left" balign="left">Expression(bcb0 + bcb3) at 10:5-11:17<br align="left"/> 11:12-11:17: @2.Call: _2 = bar() -&gt; [return: bb3, unwind: bb6]</td></tr><tr><td align="left" balign="left">bb1: FalseUnwind<br align="left"/>bb2: Call</td></tr><tr><td align="left" balign="left">bb3: SwitchInt</td></tr></table>>];
88
bcb0__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 9:1-9:11<br align="left"/> </td></tr><tr><td align="left" balign="left">bb0: Goto</td></tr></table>>];
99
bcb3__Cov_0_3 -> bcb1__Cov_0_3 [label=<>];

tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
let mut _0: bool;
66

77
bb0: {
8-
+ Coverage::Counter(0) for [/the/src/instrument_coverage.rs:20:1 - 22:2];
8+
+ Coverage::Counter(0);
9+
+ Coverage::Mappings { term: Counter(0) } for [/the/src/instrument_coverage.rs:20:1 - 22:2];
910
_0 = const true;
1011
return;
1112
}

tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@
88
let mut _3: !;
99

1010
bb0: {
11-
+ Coverage::Counter(0) for [/the/src/instrument_coverage.rs:11:1 - 11:11];
11+
+ Coverage::Mappings { term: Counter(1) } for [/the/src/instrument_coverage.rs:15:10 - 15:11];
12+
+ Coverage::Mappings { term: Expression(1) } for [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2];
13+
+ Coverage::Mappings { term: Expression(0) } for [/the/src/instrument_coverage.rs:12:5 - 13:17];
14+
+ Coverage::Counter(0);
15+
+ Coverage::Mappings { term: Counter(0) } for [/the/src/instrument_coverage.rs:11:1 - 11:11];
1216
goto -> bb1;
1317
}
1418

1519
bb1: {
16-
+ Coverage::Expression(0) = Counter(0) + Counter(1) for [/the/src/instrument_coverage.rs:12:5 - 13:17];
20+
+ Coverage::Expression(0) = Counter(0) + Counter(1);
1721
falseUnwind -> [real: bb2, unwind: bb6];
1822
}
1923

@@ -27,14 +31,14 @@
2731
}
2832

2933
bb4: {
30-
+ Coverage::Expression(1) = Expression(0) - Counter(1) for [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2];
34+
+ Coverage::Expression(1) = Expression(0) - Counter(1);
3135
_0 = const ();
3236
StorageDead(_2);
3337
return;
3438
}
3539

3640
bb5: {
37-
+ Coverage::Counter(1) for [/the/src/instrument_coverage.rs:15:10 - 15:11];
41+
+ Coverage::Counter(1);
3842
_1 = const ();
3943
StorageDead(_2);
4044
goto -> bb1;

0 commit comments

Comments
 (0)