Skip to content

Commit c765e50

Browse files
committed
coverage: Let each coverage statement hold a vector of code regions
This makes it possible for a `StatementKind::Coverage` to hold more than one code region, but that capability is not yet used.
1 parent c91c5bc commit c765e50

File tree

11 files changed

+98
-92
lines changed

11 files changed

+98
-92
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

Lines changed: 58 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ pub struct Expression {
1111
lhs: Operand,
1212
op: Op,
1313
rhs: Operand,
14-
region: Option<CodeRegion>,
14+
code_regions: Vec<CodeRegion>,
1515
}
1616

1717
/// Collects all of the coverage regions associated with (a) injected counters, (b) counter
@@ -30,7 +30,7 @@ pub struct FunctionCoverage<'tcx> {
3030
instance: Instance<'tcx>,
3131
source_hash: u64,
3232
is_used: bool,
33-
counters: IndexVec<CounterId, Option<CodeRegion>>,
33+
counters: IndexVec<CounterId, Option<Vec<CodeRegion>>>,
3434
expressions: IndexVec<ExpressionId, Option<Expression>>,
3535
unreachable_regions: Vec<CodeRegion>,
3636
}
@@ -77,28 +77,40 @@ impl<'tcx> FunctionCoverage<'tcx> {
7777
}
7878
}
7979

80-
/// Adds a code region to be counted by an injected counter intrinsic.
81-
pub fn add_counter(&mut self, id: CounterId, region: CodeRegion) {
82-
if let Some(previous_region) = self.counters[id].replace(region.clone()) {
83-
assert_eq!(previous_region, region, "add_counter: code region for id changed");
80+
/// Adds code regions to be counted by an injected counter intrinsic.
81+
#[instrument(level = "debug", skip(self))]
82+
pub(crate) fn add_counter(&mut self, id: CounterId, code_regions: &[CodeRegion]) {
83+
if code_regions.is_empty() {
84+
return;
85+
}
86+
87+
let slot = &mut self.counters[id];
88+
match slot {
89+
None => *slot = Some(code_regions.to_owned()),
90+
// If this counter ID slot has already been filled, it should
91+
// contain identical information.
92+
Some(ref previous_regions) => assert_eq!(
93+
previous_regions, code_regions,
94+
"add_counter: code regions for id changed"
95+
),
8496
}
8597
}
8698

99+
/// Adds information about a coverage expression, along with zero or more
100+
/// code regions mapped to that expression.
101+
///
87102
/// Both counters and "counter expressions" (or simply, "expressions") can be operands in other
88103
/// expressions. These are tracked as separate variants of `Operand`, so there is no ambiguity
89104
/// between operands that are counter IDs and operands that are expression IDs.
90-
pub fn add_counter_expression(
105+
#[instrument(level = "debug", skip(self))]
106+
pub(crate) fn add_counter_expression(
91107
&mut self,
92108
expression_id: ExpressionId,
93109
lhs: Operand,
94110
op: Op,
95111
rhs: Operand,
96-
region: Option<CodeRegion>,
112+
code_regions: &[CodeRegion],
97113
) {
98-
debug!(
99-
"add_counter_expression({:?}, lhs={:?}, op={:?}, rhs={:?} at {:?}",
100-
expression_id, lhs, op, rhs, region
101-
);
102114
debug_assert!(
103115
expression_id.as_usize() < self.expressions.len(),
104116
"expression_id {} is out of range for expressions.len() = {}
@@ -107,23 +119,25 @@ impl<'tcx> FunctionCoverage<'tcx> {
107119
self.expressions.len(),
108120
self,
109121
);
110-
if let Some(previous_expression) = self.expressions[expression_id].replace(Expression {
111-
lhs,
112-
op,
113-
rhs,
114-
region: region.clone(),
115-
}) {
116-
assert_eq!(
117-
previous_expression,
118-
Expression { lhs, op, rhs, region },
122+
123+
let expression = Expression { lhs, op, rhs, code_regions: code_regions.to_owned() };
124+
let slot = &mut self.expressions[expression_id];
125+
match slot {
126+
None => *slot = Some(expression),
127+
// If this expression ID slot has already been filled, it should
128+
// contain identical information.
129+
Some(ref previous_expression) => assert_eq!(
130+
previous_expression, &expression,
119131
"add_counter_expression: expression for id changed"
120-
);
132+
),
121133
}
122134
}
123135

124-
/// Add a region that will be marked as "unreachable", with a constant "zero counter".
125-
pub fn add_unreachable_region(&mut self, region: CodeRegion) {
126-
self.unreachable_regions.push(region)
136+
/// Adds regions that will be marked as "unreachable", with a constant "zero counter".
137+
#[instrument(level = "debug", skip(self))]
138+
pub(crate) fn add_unreachable_regions(&mut self, code_regions: &[CodeRegion]) {
139+
assert!(!code_regions.is_empty(), "unreachable regions always have code regions");
140+
self.unreachable_regions.extend_from_slice(code_regions);
127141
}
128142

129143
/// Perform some simplifications to make the final coverage mappings
@@ -212,11 +226,16 @@ impl<'tcx> FunctionCoverage<'tcx> {
212226
}
213227

214228
fn counter_regions(&self) -> impl Iterator<Item = (Counter, &CodeRegion)> {
215-
self.counters.iter_enumerated().filter_map(|(index, entry)| {
216-
// Option::map() will return None to filter out missing counters. This may happen
217-
// if, for example, a MIR-instrumented counter is removed during an optimization.
218-
entry.as_ref().map(|region| (Counter::counter_value_reference(index), region))
219-
})
229+
self.counters
230+
.iter_enumerated()
231+
// Filter out counter IDs that we never saw during MIR traversal.
232+
// This can happen if a counter was optimized out by MIR transforms
233+
// (and replaced with `CoverageKind::Unreachable` instead).
234+
.filter_map(|(id, maybe_code_regions)| Some((id, maybe_code_regions.as_ref()?)))
235+
.flat_map(|(id, code_regions)| {
236+
let counter = Counter::counter_value_reference(id);
237+
code_regions.iter().map(move |region| (counter, region))
238+
})
220239
}
221240

222241
/// Convert this function's coverage expression data into a form that can be
@@ -254,13 +273,17 @@ impl<'tcx> FunctionCoverage<'tcx> {
254273

255274
fn expression_regions(&self) -> Vec<(Counter, &CodeRegion)> {
256275
// Find all of the expression IDs that weren't optimized out AND have
257-
// an attached code region, and return the corresponding mapping as a
258-
// counter/region pair.
276+
// one or more attached code regions, and return the corresponding
277+
// mappings as counter/region pairs.
259278
self.expressions
260279
.iter_enumerated()
261-
.filter_map(|(id, expression)| {
262-
let code_region = expression.as_ref()?.region.as_ref()?;
263-
Some((Counter::expression(id), code_region))
280+
.filter_map(|(id, maybe_expression)| {
281+
let code_regions = &maybe_expression.as_ref()?.code_regions;
282+
Some((id, code_regions))
283+
})
284+
.flat_map(|(id, code_regions)| {
285+
let counter = Counter::expression(id);
286+
code_regions.iter().map(move |code_region| (counter, code_region))
264287
})
265288
.collect::<Vec<_>>()
266289
}

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -110,25 +110,15 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
110110
.entry(instance)
111111
.or_insert_with(|| FunctionCoverage::new(bx.tcx(), instance));
112112

113-
let Coverage { kind, code_region } = coverage.clone();
114-
match kind {
113+
let Coverage { kind, code_regions } = coverage;
114+
match *kind {
115115
CoverageKind::Counter { function_source_hash, id } => {
116116
debug!(
117117
"ensuring function source hash is set for instance={:?}; function_source_hash={}",
118118
instance, function_source_hash,
119119
);
120120
func_coverage.set_function_source_hash(function_source_hash);
121-
122-
if let Some(code_region) = code_region {
123-
// Note: Some counters do not have code regions, but may still be referenced
124-
// from expressions. In that case, don't add the counter to the coverage map,
125-
// but do inject the counter intrinsic.
126-
debug!(
127-
"adding counter to coverage_map: instance={:?}, id={:?}, region={:?}",
128-
instance, id, code_region,
129-
);
130-
func_coverage.add_counter(id, code_region);
131-
}
121+
func_coverage.add_counter(id, code_regions);
132122
// We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,
133123
// as that needs an exclusive borrow.
134124
drop(coverage_map);
@@ -146,20 +136,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
146136
bx.instrprof_increment(fn_name, hash, num_counters, index);
147137
}
148138
CoverageKind::Expression { id, lhs, op, rhs } => {
149-
debug!(
150-
"adding counter expression to coverage_map: instance={:?}, id={:?}, {:?} {:?} {:?}; region: {:?}",
151-
instance, id, lhs, op, rhs, code_region,
152-
);
153-
func_coverage.add_counter_expression(id, lhs, op, rhs, code_region);
139+
func_coverage.add_counter_expression(id, lhs, op, rhs, code_regions);
154140
}
155141
CoverageKind::Unreachable => {
156-
let code_region =
157-
code_region.expect("unreachable regions always have code regions");
158-
debug!(
159-
"adding unreachable code to coverage_map: instance={:?}, at {:?}",
160-
instance, code_region,
161-
);
162-
func_coverage.add_unreachable_region(code_region);
142+
func_coverage.add_unreachable_regions(code_regions);
163143
}
164144
}
165145
}
@@ -228,7 +208,8 @@ fn add_unused_function_coverage<'tcx>(
228208

229209
let mut function_coverage = FunctionCoverage::unused(tcx, instance);
230210
for &code_region in tcx.covered_code_regions(def_id) {
231-
function_coverage.add_unreachable_region(code_region.clone());
211+
let code_region = std::slice::from_ref(code_region);
212+
function_coverage.add_unreachable_regions(code_region);
232213
}
233214

234215
if let Some(coverage_context) = cx.coverage_context() {

compiler/rustc_middle/src/mir/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,10 +1474,13 @@ impl Debug for Statement<'_> {
14741474
AscribeUserType(box (ref place, ref c_ty), ref variance) => {
14751475
write!(fmt, "AscribeUserType({place:?}, {variance:?}, {c_ty:?})")
14761476
}
1477-
Coverage(box self::Coverage { ref kind, code_region: Some(ref rgn) }) => {
1478-
write!(fmt, "Coverage::{kind:?} for {rgn:?}")
1477+
Coverage(box self::Coverage { ref kind, ref code_regions }) => {
1478+
if code_regions.is_empty() {
1479+
write!(fmt, "Coverage::{kind:?}")
1480+
} else {
1481+
write!(fmt, "Coverage::{kind:?} for {code_regions:?}")
1482+
}
14791483
}
1480-
Coverage(box ref coverage) => write!(fmt, "Coverage::{:?}", coverage.kind),
14811484
Intrinsic(box ref intrinsic) => write!(fmt, "{intrinsic}"),
14821485
ConstEvalCounter => write!(fmt, "ConstEvalCounter"),
14831486
Nop => write!(fmt, "nop"),

compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ pub enum FakeReadCause {
523523
#[derive(TypeFoldable, TypeVisitable)]
524524
pub struct Coverage {
525525
pub kind: CoverageKind,
526-
pub code_region: Option<CodeRegion>,
526+
pub code_regions: Vec<CodeRegion>,
527527
}
528528

529529
#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]

compiler/rustc_mir_transform/src/coverage/mod.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
320320
self.mir_body,
321321
self.make_mir_coverage_kind(&counter_kind),
322322
self.bcb_leader_bb(bcb),
323-
Some(code_region),
323+
vec![code_region],
324324
);
325325
}
326326
}
@@ -403,7 +403,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
403403
self.mir_body,
404404
self.make_mir_coverage_kind(&counter_kind),
405405
inject_to_bb,
406-
None,
406+
Vec::new(),
407407
);
408408
}
409409
BcbCounter::Expression { .. } => inject_intermediate_expression(
@@ -473,20 +473,14 @@ fn inject_statement(
473473
mir_body: &mut mir::Body<'_>,
474474
counter_kind: CoverageKind,
475475
bb: BasicBlock,
476-
some_code_region: Option<CodeRegion>,
476+
code_regions: Vec<CodeRegion>,
477477
) {
478-
debug!(
479-
" injecting statement {:?} for {:?} at code region: {:?}",
480-
counter_kind, bb, some_code_region
481-
);
478+
debug!(" injecting statement {counter_kind:?} for {bb:?} at code regions: {code_regions:?}");
482479
let data = &mut mir_body[bb];
483480
let source_info = data.terminator().source_info;
484481
let statement = Statement {
485482
source_info,
486-
kind: StatementKind::Coverage(Box::new(Coverage {
487-
kind: counter_kind,
488-
code_region: some_code_region,
489-
})),
483+
kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind, code_regions })),
490484
};
491485
data.statements.insert(0, statement);
492486
}
@@ -500,7 +494,10 @@ fn inject_intermediate_expression(mir_body: &mut mir::Body<'_>, expression: Cove
500494
let source_info = data.terminator().source_info;
501495
let statement = Statement {
502496
source_info,
503-
kind: StatementKind::Coverage(Box::new(Coverage { kind: expression, code_region: None })),
497+
kind: StatementKind::Coverage(Box::new(Coverage {
498+
kind: expression,
499+
code_regions: Vec::new(),
500+
})),
504501
};
505502
data.statements.push(statement);
506503
}

compiler/rustc_mir_transform/src/coverage/query.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) ->
9393
fn covered_code_regions(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<&CodeRegion> {
9494
let body = mir_body(tcx, def_id);
9595
all_coverage_in_mir_body(body)
96-
// Not all coverage statements have an attached code region.
97-
.filter_map(|coverage| coverage.code_region.as_ref())
96+
// Coverage statements have a list of code regions (possibly empty).
97+
.flat_map(|coverage| coverage.code_regions.as_slice())
9898
.collect()
9999
}
100100

compiler/rustc_mir_transform/src/simplify.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,21 +436,23 @@ fn save_unreachable_coverage(
436436
for dead_block in &basic_blocks.raw[first_dead_block..] {
437437
for statement in &dead_block.statements {
438438
let StatementKind::Coverage(coverage) = &statement.kind else { continue };
439-
let Some(code_region) = &coverage.code_region else { continue };
439+
if coverage.code_regions.is_empty() {
440+
continue;
441+
};
440442
let instance = statement.source_info.scope.inlined_instance(source_scopes);
441443
if live.contains(&instance) {
442-
retained_coverage.push((statement.source_info, code_region.clone()));
444+
retained_coverage.push((statement.source_info, coverage.code_regions.clone()));
443445
}
444446
}
445447
}
446448

447449
let start_block = &mut basic_blocks[START_BLOCK];
448450
start_block.statements.extend(retained_coverage.into_iter().map(
449-
|(source_info, code_region)| Statement {
451+
|(source_info, code_regions)| Statement {
450452
source_info,
451453
kind: StatementKind::Coverage(Box::new(Coverage {
452454
kind: CoverageKind::Unreachable,
453-
code_region: Some(code_region),
455+
code_regions,
454456
})),
455457
},
456458
));

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) 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>>];
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(2) = Expression(1) + Zero for $DIR/coverage_graphviz.rs:15:1 - 15:2<br align="left"/>Expression(bcb2:(bcb1:(bcb0 + bcb3) - bcb3) + 0) 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) 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(2) = Expression(1) + Zero for [$DIR/coverage_graphviz.rs:15:1 - 15:2]<br align="left"/>Expression(bcb2:(bcb1:(bcb0 + bcb3) - bcb3) + 0) 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
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) for [/the/src/instrument_coverage.rs:20:1 - 22:2];
99
_0 = const true;
1010
return;
1111
}

0 commit comments

Comments
 (0)