Skip to content

Commit 7d32b77

Browse files
ZalatharZalathar
committed
coverage: Store BCB node IDs in mappings, and resolve them in codegen
Even though the coverage graph itself is no longer available during codegen, its nodes can still be used as opaque IDs.
1 parent f940188 commit 7d32b77

13 files changed

+92
-103
lines changed

‎compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ use rustc_codegen_ssa::traits::{
1111
BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods,
1212
};
1313
use rustc_middle::mir::coverage::{
14-
CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op,
14+
BasicCoverageBlock, CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping,
15+
MappingKind, Op,
1516
};
1617
use rustc_middle::ty::{Instance, TyCtxt};
1718
use rustc_span::Span;
@@ -140,7 +141,12 @@ fn fill_region_tables<'tcx>(
140141
for &Mapping { ref kind, span } in &fn_cov_info.mappings {
141142
// If the mapping refers to counters/expressions that were removed by
142143
// MIR opts, replace those occurrences with zero.
143-
let kind = kind.map_terms(|term| if is_zero_term(term) { CovTerm::Zero } else { term });
144+
let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter {
145+
let term =
146+
fn_cov_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term");
147+
let term = if is_zero_term(term) { CovTerm::Zero } else { term };
148+
ffi::Counter::from_term(term)
149+
};
144150

145151
// Convert the `Span` into coordinates that we can pass to LLVM, or
146152
// discard the span if conversion fails. In rare, cases _all_ of a
@@ -154,23 +160,22 @@ fn fill_region_tables<'tcx>(
154160
continue;
155161
}
156162

157-
match kind {
158-
MappingKind::Code(term) => {
159-
code_regions
160-
.push(ffi::CodeRegion { cov_span, counter: ffi::Counter::from_term(term) });
163+
match *kind {
164+
MappingKind::Code { bcb } => {
165+
code_regions.push(ffi::CodeRegion { cov_span, counter: counter_for_bcb(bcb) });
161166
}
162-
MappingKind::Branch { true_term, false_term } => {
167+
MappingKind::Branch { true_bcb, false_bcb } => {
163168
branch_regions.push(ffi::BranchRegion {
164169
cov_span,
165-
true_counter: ffi::Counter::from_term(true_term),
166-
false_counter: ffi::Counter::from_term(false_term),
170+
true_counter: counter_for_bcb(true_bcb),
171+
false_counter: counter_for_bcb(false_bcb),
167172
});
168173
}
169-
MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
174+
MappingKind::MCDCBranch { true_bcb, false_bcb, mcdc_params } => {
170175
mcdc_branch_regions.push(ffi::MCDCBranchRegion {
171176
cov_span,
172-
true_counter: ffi::Counter::from_term(true_term),
173-
false_counter: ffi::Counter::from_term(false_term),
177+
true_counter: counter_for_bcb(true_bcb),
178+
false_counter: counter_for_bcb(false_bcb),
174179
mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
175180
});
176181
}

‎compiler/rustc_middle/src/mir/coverage.rs

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -179,34 +179,19 @@ pub struct Expression {
179179
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
180180
pub enum MappingKind {
181181
/// Associates a normal region of code with a counter/expression/zero.
182-
Code(CovTerm),
182+
Code { bcb: BasicCoverageBlock },
183183
/// Associates a branch region with separate counters for true and false.
184-
Branch { true_term: CovTerm, false_term: CovTerm },
184+
Branch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock },
185185
/// Associates a branch region with separate counters for true and false.
186-
MCDCBranch { true_term: CovTerm, false_term: CovTerm, mcdc_params: ConditionInfo },
186+
MCDCBranch {
187+
true_bcb: BasicCoverageBlock,
188+
false_bcb: BasicCoverageBlock,
189+
mcdc_params: ConditionInfo,
190+
},
187191
/// Associates a decision region with a bitmap and number of conditions.
188192
MCDCDecision(DecisionInfo),
189193
}
190194

191-
impl MappingKind {
192-
/// Returns a copy of this mapping kind, in which all coverage terms have
193-
/// been replaced with ones returned by the given function.
194-
pub fn map_terms(&self, map_fn: impl Fn(CovTerm) -> CovTerm) -> Self {
195-
match *self {
196-
Self::Code(term) => Self::Code(map_fn(term)),
197-
Self::Branch { true_term, false_term } => {
198-
Self::Branch { true_term: map_fn(true_term), false_term: map_fn(false_term) }
199-
}
200-
Self::MCDCBranch { true_term, false_term, mcdc_params } => Self::MCDCBranch {
201-
true_term: map_fn(true_term),
202-
false_term: map_fn(false_term),
203-
mcdc_params,
204-
},
205-
Self::MCDCDecision(param) => Self::MCDCDecision(param),
206-
}
207-
}
208-
}
209-
210195
#[derive(Clone, Debug)]
211196
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
212197
pub struct Mapping {
@@ -222,10 +207,14 @@ pub struct Mapping {
222207
pub struct FunctionCoverageInfo {
223208
pub function_source_hash: u64,
224209
pub body_span: Span,
210+
225211
pub num_counters: usize,
226-
pub mcdc_bitmap_bits: usize,
227212
pub expressions: IndexVec<ExpressionId, Expression>,
213+
228214
pub mappings: Vec<Mapping>,
215+
pub term_for_bcb: IndexVec<BasicCoverageBlock, Option<CovTerm>>,
216+
217+
pub mcdc_bitmap_bits: usize,
229218
/// The depth of the deepest decision is used to know how many
230219
/// temp condbitmaps should be allocated for the function.
231220
pub mcdc_num_condition_bitmaps: usize,
@@ -329,3 +318,19 @@ impl CoverageIdsInfo {
329318
}
330319
}
331320
}
321+
322+
rustc_index::newtype_index! {
323+
/// During the `InstrumentCoverage` MIR pass, a BCB is a node in the
324+
/// "coverage graph", which is a refinement of the MIR control-flow graph
325+
/// that merges or omits some blocks that aren't relevant to coverage.
326+
///
327+
/// After that pass is complete, the coverage graph no longer exists, so a
328+
/// BCB is effectively an opaque ID.
329+
#[derive(HashStable)]
330+
#[encodable]
331+
#[orderable]
332+
#[debug_format = "bcb{}"]
333+
pub struct BasicCoverageBlock {
334+
const START_BCB = 0;
335+
}
336+
}

‎compiler/rustc_middle/src/ty/structural_impls.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ TrivialTypeTraversalImpls! {
246246
::rustc_hir::HirId,
247247
::rustc_hir::MatchSource,
248248
::rustc_target::asm::InlineAsmRegOrRegClass,
249+
crate::mir::coverage::BasicCoverageBlock,
249250
crate::mir::coverage::BlockMarkerId,
250251
crate::mir::coverage::CounterId,
251252
crate::mir::coverage::ExpressionId,

‎compiler/rustc_mir_transform/src/coverage/counters.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ pub(super) struct CoverageCounters {
135135
next_counter_id: CounterId,
136136

137137
/// Coverage counters/expressions that are associated with individual BCBs.
138-
node_counters: IndexVec<BasicCoverageBlock, Option<CovTerm>>,
138+
pub(crate) node_counters: IndexVec<BasicCoverageBlock, Option<CovTerm>>,
139139

140140
/// Table of expression data, associating each expression ID with its
141141
/// corresponding operator (+ or -) and its LHS/RHS operands.
@@ -205,10 +205,6 @@ impl CoverageCounters {
205205
counter
206206
}
207207

208-
pub(super) fn term_for_bcb(&self, bcb: BasicCoverageBlock) -> Option<CovTerm> {
209-
self.node_counters[bcb]
210-
}
211-
212208
/// Returns an iterator over all the nodes in the coverage graph that
213209
/// should have a counter-increment statement injected into MIR, along with
214210
/// each site's corresponding counter ID.

‎compiler/rustc_mir_transform/src/coverage/graph.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_data_structures::graph::dominators::Dominators;
88
use rustc_data_structures::graph::{self, DirectedGraph, StartNode};
99
use rustc_index::IndexVec;
1010
use rustc_index::bit_set::DenseBitSet;
11+
pub(crate) use rustc_middle::mir::coverage::{BasicCoverageBlock, START_BCB};
1112
use rustc_middle::mir::{self, BasicBlock, Terminator, TerminatorKind};
1213
use tracing::debug;
1314

@@ -269,15 +270,6 @@ impl graph::Predecessors for CoverageGraph {
269270
}
270271
}
271272

272-
rustc_index::newtype_index! {
273-
/// A node in the control-flow graph of CoverageGraph.
274-
#[orderable]
275-
#[debug_format = "bcb{}"]
276-
pub(crate) struct BasicCoverageBlock {
277-
const START_BCB = 0;
278-
}
279-
}
280-
281273
/// `BasicCoverageBlockData` holds the data indexed by a `BasicCoverageBlock`.
282274
///
283275
/// A `BasicCoverageBlock` (BCB) represents the maximal-length sequence of MIR `BasicBlock`s without

‎compiler/rustc_mir_transform/src/coverage/mod.rs

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,13 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
9595

9696
let coverage_counters = counters::make_bcb_counters(&graph, &bcbs_with_counter_mappings);
9797

98-
let mappings = create_mappings(&extracted_mappings, &coverage_counters);
98+
let mappings = create_mappings(&extracted_mappings);
9999
if mappings.is_empty() {
100100
// No spans could be converted into valid mappings, so skip this function.
101101
debug!("no spans could be converted into valid mappings; skipping");
102102
return;
103103
}
104+
let term_for_bcb = coverage_counters.node_counters.clone();
104105

105106
inject_coverage_statements(mir_body, &graph, &extracted_mappings, &coverage_counters);
106107

@@ -116,26 +117,24 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
116117
mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
117118
function_source_hash: hir_info.function_source_hash,
118119
body_span: hir_info.body_span,
120+
119121
num_counters: coverage_counters.num_counters(),
120-
mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits,
121122
expressions: coverage_counters.into_expressions(),
123+
122124
mappings,
125+
term_for_bcb,
126+
127+
mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits,
123128
mcdc_num_condition_bitmaps,
124129
}));
125130
}
126131

127-
/// For each coverage span extracted from MIR, create a corresponding
128-
/// mapping.
132+
/// For each coverage span extracted from MIR, create a corresponding mapping.
129133
///
130-
/// Precondition: All BCBs corresponding to those spans have been given
131-
/// coverage counters.
132-
fn create_mappings(
133-
extracted_mappings: &ExtractedMappings,
134-
coverage_counters: &CoverageCounters,
135-
) -> Vec<Mapping> {
136-
let term_for_bcb =
137-
|bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
138-
134+
/// FIXME(Zalathar): This used to be where BCBs in the extracted mappings were
135+
/// resolved to a `CovTerm`. But that is now handled elsewhere, so this
136+
/// function can potentially be simplified even further.
137+
fn create_mappings(extracted_mappings: &ExtractedMappings) -> Vec<Mapping> {
139138
// Fully destructure the mappings struct to make sure we don't miss any kinds.
140139
let ExtractedMappings {
141140
num_bcbs: _,
@@ -150,23 +149,18 @@ fn create_mappings(
150149
mappings.extend(code_mappings.iter().map(
151150
// Ordinary code mappings are the simplest kind.
152151
|&mappings::CodeMapping { span, bcb }| {
153-
let kind = MappingKind::Code(term_for_bcb(bcb));
152+
let kind = MappingKind::Code { bcb };
154153
Mapping { kind, span }
155154
},
156155
));
157156

158157
mappings.extend(branch_pairs.iter().map(
159158
|&mappings::BranchPair { span, true_bcb, false_bcb }| {
160-
let true_term = term_for_bcb(true_bcb);
161-
let false_term = term_for_bcb(false_bcb);
162-
let kind = MappingKind::Branch { true_term, false_term };
159+
let kind = MappingKind::Branch { true_bcb, false_bcb };
163160
Mapping { kind, span }
164161
},
165162
));
166163

167-
let term_for_bcb =
168-
|bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
169-
170164
// MCDC branch mappings are appended with their decisions in case decisions were ignored.
171165
mappings.extend(mcdc_degraded_branches.iter().map(
172166
|&mappings::MCDCBranch {
@@ -176,11 +170,7 @@ fn create_mappings(
176170
condition_info: _,
177171
true_index: _,
178172
false_index: _,
179-
}| {
180-
let true_term = term_for_bcb(true_bcb);
181-
let false_term = term_for_bcb(false_bcb);
182-
Mapping { kind: MappingKind::Branch { true_term, false_term }, span }
183-
},
173+
}| { Mapping { kind: MappingKind::Branch { true_bcb, false_bcb }, span } },
184174
));
185175

186176
for (decision, branches) in mcdc_mappings {
@@ -201,12 +191,10 @@ fn create_mappings(
201191
true_index: _,
202192
false_index: _,
203193
}| {
204-
let true_term = term_for_bcb(true_bcb);
205-
let false_term = term_for_bcb(false_bcb);
206194
Mapping {
207195
kind: MappingKind::MCDCBranch {
208-
true_term,
209-
false_term,
196+
true_bcb,
197+
false_bcb,
210198
mcdc_params: condition_info,
211199
},
212200
span,

‎compiler/rustc_mir_transform/src/coverage/query.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ fn coverage_ids_info<'tcx>(
103103
// to any particular point in the control-flow graph.
104104
// (Keep this in sync with the injection of `ExpressionUsed`
105105
// statements in the `InstrumentCoverage` MIR pass.)
106-
if let MappingKind::Code(CovTerm::Expression(id)) = mapping.kind {
106+
if let MappingKind::Code { bcb } = mapping.kind
107+
&& let Some(CovTerm::Expression(id)) = fn_cov_info.term_for_bcb[bcb]
108+
{
107109
expressions_seen.remove(id);
108110
}
109111
}

‎tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@
3030
+ coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Add, rhs: Counter(2) };
3131
+ coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Add, rhs: Counter(3) };
3232
+ coverage ExpressionId(2) => Expression { lhs: Counter(0), op: Subtract, rhs: Expression(1) };
33-
+ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:14:1: 15:21 (#0);
34-
+ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:16:17: 16:33 (#0);
35-
+ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17: 17:33 (#0);
36-
+ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:18:17: 18:33 (#0);
37-
+ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17: 19:33 (#0);
38-
+ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:21:2: 21:2 (#0);
33+
+ coverage Code { bcb: bcb0 } => $DIR/branch_match_arms.rs:14:1: 15:21 (#0);
34+
+ coverage Code { bcb: bcb1 } => $DIR/branch_match_arms.rs:16:17: 16:33 (#0);
35+
+ coverage Code { bcb: bcb3 } => $DIR/branch_match_arms.rs:17:17: 17:33 (#0);
36+
+ coverage Code { bcb: bcb4 } => $DIR/branch_match_arms.rs:18:17: 18:33 (#0);
37+
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:17: 19:33 (#0);
38+
+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:2: 21:2 (#0);
3939
+
4040
bb0: {
4141
+ Coverage::CounterIncrement(0);

‎tests/mir-opt/coverage/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
+ coverage body span: $DIR/instrument_coverage.rs:29:18: 31:2 (#0)
8-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:29:1: 31:2 (#0);
8+
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:1: 31:2 (#0);
99
+
1010
bb0: {
1111
+ Coverage::CounterIncrement(0);

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99

1010
+ coverage body span: $DIR/instrument_coverage.rs:14:11: 20:2 (#0)
1111
+ coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Subtract, rhs: Counter(0) };
12-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:14:1: 14:11 (#0);
13-
+ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:16:12: 16:17 (#0);
14-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:17:13: 17:18 (#0);
15-
+ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:18:10: 18:10 (#0);
16-
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:20:2: 20:2 (#0);
12+
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:14:1: 14:11 (#0);
13+
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage.rs:16:12: 16:17 (#0);
14+
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:17:13: 17:18 (#0);
15+
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:18:10: 18:10 (#0);
16+
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:20:2: 20:2 (#0);
1717
+
1818
bb0: {
1919
+ Coverage::CounterIncrement(0);

‎tests/mir-opt/coverage/instrument_coverage.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// EMIT_MIR instrument_coverage.main.InstrumentCoverage.diff
99
// CHECK-LABEL: fn main()
1010
// CHECK: coverage body span:
11-
// CHECK: coverage Code(Counter({{[0-9]+}})) =>
11+
// CHECK: coverage Code { bcb: bcb{{[0-9]+}} } =>
1212
// CHECK: bb0:
1313
// CHECK: Coverage::CounterIncrement
1414
fn main() {
@@ -22,7 +22,7 @@ fn main() {
2222
// EMIT_MIR instrument_coverage.bar.InstrumentCoverage.diff
2323
// CHECK-LABEL: fn bar()
2424
// CHECK: coverage body span:
25-
// CHECK: coverage Code(Counter({{[0-9]+}})) =>
25+
// CHECK: coverage Code { bcb: bcb{{[0-9]+}} } =>
2626
// CHECK: bb0:
2727
// CHECK: Coverage::CounterIncrement
2828
#[inline(never)]

‎tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99

1010
coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0)
1111
coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) };
12-
coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0);
13-
coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
14-
coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
15-
coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
16-
coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
12+
coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0);
13+
coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
14+
coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
15+
coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
16+
coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
1717

1818
bb0: {
1919
Coverage::CounterIncrement(0);

0 commit comments

Comments
 (0)