Skip to content

Commit 6c9da0d

Browse files
committed
mcdc-coverage: Get decision_depth from THIR lowering
Use decision context stack to handle nested decisions: - Introduce MCDCDecisionCtx - Use a stack of MCDCDecisionCtx to handle nested decisions
1 parent 7f13445 commit 6c9da0d

File tree

5 files changed

+78
-18
lines changed

5 files changed

+78
-18
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ fn ensure_mcdc_parameters<'ll, 'tcx>(
203203
let fn_name = bx.get_pgo_func_name_var(instance);
204204
let hash = bx.const_u64(function_coverage_info.function_source_hash);
205205
let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes);
206-
let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, 1_u32);
206+
let max_decision_depth = function_coverage_info.mcdc_max_decision_depth;
207+
let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, max_decision_depth as u32);
207208
bx.coverage_context()
208209
.expect("already checked above")
209210
.mcdc_condition_bitmap_map

compiler/rustc_middle/src/mir/coverage.rs

+3
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,9 @@ pub struct FunctionCoverageInfo {
269269
pub mcdc_bitmap_bytes: u32,
270270
pub expressions: IndexVec<ExpressionId, Expression>,
271271
pub mappings: Vec<Mapping>,
272+
/// The depth of the deepest decision is used to know how many
273+
/// temp condbitmaps should be allocated for the function.
274+
pub mcdc_max_decision_depth: u16,
272275
}
273276

274277
/// Branch information recorded during THIR-to-MIR lowering, and stored in MIR.

compiler/rustc_mir_build/src/build/coverageinfo.rs

+56-17
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,14 @@ impl BranchInfoBuilder {
101101
tcx: TyCtxt<'_>,
102102
true_marker: BlockMarkerId,
103103
false_marker: BlockMarkerId,
104-
) -> Option<ConditionInfo> {
104+
) -> Option<(u16, ConditionInfo)> {
105105
let mcdc_state = self.mcdc_state.as_mut()?;
106+
let decision_depth = mcdc_state.decision_depth();
106107
let (mut condition_info, decision_result) =
107108
mcdc_state.take_condition(true_marker, false_marker);
109+
// take_condition() returns Some for decision_result when the decision stack
110+
// is empty, i.e. when all the conditions of the decision were instrumented,
111+
// and the decision is "complete".
108112
if let Some(decision) = decision_result {
109113
match decision.conditions_num {
110114
0 => {
@@ -131,7 +135,7 @@ impl BranchInfoBuilder {
131135
}
132136
}
133137
}
134-
condition_info
138+
condition_info.map(|cond_info| (decision_depth, cond_info))
135139
}
136140

137141
fn add_two_way_branch<'tcx>(
@@ -199,17 +203,28 @@ impl BranchInfoBuilder {
199203
/// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged.
200204
const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6;
201205

202-
struct MCDCState {
206+
#[derive(Default)]
207+
struct MCDCDecisionCtx {
203208
/// To construct condition evaluation tree.
204209
decision_stack: VecDeque<ConditionInfo>,
205210
processing_decision: Option<MCDCDecisionSpan>,
206211
}
207212

213+
struct MCDCState {
214+
decision_ctx_stack: Vec<MCDCDecisionCtx>,
215+
}
216+
208217
impl MCDCState {
209218
fn new_if_enabled(tcx: TyCtxt<'_>) -> Option<Self> {
210219
tcx.sess
211220
.instrument_coverage_mcdc()
212-
.then(|| Self { decision_stack: VecDeque::new(), processing_decision: None })
221+
.then(|| Self { decision_ctx_stack: vec![MCDCDecisionCtx::default()] })
222+
}
223+
224+
#[inline]
225+
fn decision_depth(&self) -> u16 {
226+
self.decision_ctx_stack.len().checked_sub(1).expect("Unexpected empty decision stack")
227+
as u16
213228
}
214229

215230
// At first we assign ConditionIds for each sub expression.
@@ -253,20 +268,23 @@ impl MCDCState {
253268
// - If the op is AND, the "false_next" of LHS and RHS should be the parent's "false_next". While "true_next" of the LHS is the RHS, the "true next" of RHS is the parent's "true_next".
254269
// - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next".
255270
fn record_conditions(&mut self, op: LogicalOp, span: Span) {
256-
let decision = match self.processing_decision.as_mut() {
271+
let decision_depth = self.decision_depth();
272+
let decision_ctx =
273+
self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack");
274+
let decision = match decision_ctx.processing_decision.as_mut() {
257275
Some(decision) => {
258276
decision.span = decision.span.to(span);
259277
decision
260278
}
261-
None => self.processing_decision.insert(MCDCDecisionSpan {
279+
None => decision_ctx.processing_decision.insert(MCDCDecisionSpan {
262280
span,
263281
conditions_num: 0,
264282
end_markers: vec![],
265-
decision_depth: 0,
283+
decision_depth: decision_depth,
266284
}),
267285
};
268286

269-
let parent_condition = self.decision_stack.pop_back().unwrap_or_default();
287+
let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default();
270288
let lhs_id = if parent_condition.condition_id == ConditionId::NONE {
271289
decision.conditions_num += 1;
272290
ConditionId::from(decision.conditions_num)
@@ -306,19 +324,21 @@ impl MCDCState {
306324
}
307325
};
308326
// We visit expressions tree in pre-order, so place the left-hand side on the top.
309-
self.decision_stack.push_back(rhs);
310-
self.decision_stack.push_back(lhs);
327+
decision_ctx.decision_stack.push_back(rhs);
328+
decision_ctx.decision_stack.push_back(lhs);
311329
}
312330

313331
fn take_condition(
314332
&mut self,
315333
true_marker: BlockMarkerId,
316334
false_marker: BlockMarkerId,
317335
) -> (Option<ConditionInfo>, Option<MCDCDecisionSpan>) {
318-
let Some(condition_info) = self.decision_stack.pop_back() else {
336+
let decision_ctx =
337+
self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack");
338+
let Some(condition_info) = decision_ctx.decision_stack.pop_back() else {
319339
return (None, None);
320340
};
321-
let Some(decision) = self.processing_decision.as_mut() else {
341+
let Some(decision) = decision_ctx.processing_decision.as_mut() else {
322342
bug!("Processing decision should have been created before any conditions are taken");
323343
};
324344
if condition_info.true_next_id == ConditionId::NONE {
@@ -328,8 +348,8 @@ impl MCDCState {
328348
decision.end_markers.push(false_marker);
329349
}
330350

331-
if self.decision_stack.is_empty() {
332-
(Some(condition_info), self.processing_decision.take())
351+
if decision_ctx.decision_stack.is_empty() {
352+
(Some(condition_info), decision_ctx.processing_decision.take())
333353
} else {
334354
(Some(condition_info), None)
335355
}
@@ -365,14 +385,17 @@ impl Builder<'_, '_> {
365385
|block| branch_info.inject_block_marker(&mut self.cfg, source_info, block);
366386
let true_marker = inject_block_marker(then_block);
367387
let false_marker = inject_block_marker(else_block);
368-
let condition_info =
369-
branch_info.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker);
388+
let (decision_depth, condition_info) = branch_info
389+
.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker)
390+
.map_or((0, None), |(decision_depth, condition_info)| {
391+
(decision_depth, Some(condition_info))
392+
});
370393
branch_info.mcdc_branch_spans.push(MCDCBranchSpan {
371394
span: source_info.span,
372395
condition_info,
373396
true_marker,
374397
false_marker,
375-
decision_depth: 0,
398+
decision_depth,
376399
});
377400
return;
378401
}
@@ -387,4 +410,20 @@ impl Builder<'_, '_> {
387410
mcdc_state.record_conditions(logical_op, span);
388411
}
389412
}
413+
414+
pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) {
415+
if let Some(branch_info) = self.coverage_branch_info.as_mut()
416+
&& let Some(mcdc_state) = branch_info.mcdc_state.as_mut()
417+
{
418+
mcdc_state.decision_ctx_stack.push(MCDCDecisionCtx::default());
419+
};
420+
}
421+
422+
pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) {
423+
if let Some(branch_info) = self.coverage_branch_info.as_mut()
424+
&& let Some(mcdc_state) = branch_info.mcdc_state.as_mut()
425+
{
426+
mcdc_state.decision_ctx_stack.pop().expect("Unexpected empty decision stack");
427+
};
428+
}
390429
}

compiler/rustc_mir_build/src/build/matches/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
151151
let mut block = block;
152152
let temp_scope = args.temp_scope_override.unwrap_or_else(|| this.local_scope());
153153
let mutability = Mutability::Mut;
154+
155+
// Increment the decision depth, in case we encounter boolean expressions
156+
// further down.
157+
this.mcdc_increment_depth_if_enabled();
154158
let place =
155159
unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability));
160+
this.mcdc_decrement_depth_if_enabled();
161+
156162
let operand = Operand::Move(Place::from(place));
157163

158164
let then_block = this.cfg.start_new_block();

compiler/rustc_mir_transform/src/coverage/mod.rs

+11
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,23 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
102102

103103
inject_mcdc_statements(mir_body, &basic_coverage_blocks, &coverage_spans);
104104

105+
let mcdc_max_decision_depth = coverage_spans
106+
.mappings
107+
.iter()
108+
.filter_map(|bcb_mapping| match bcb_mapping.kind {
109+
BcbMappingKind::MCDCDecision { decision_depth, .. } => Some(decision_depth),
110+
_ => None,
111+
})
112+
.max()
113+
.unwrap_or(0);
114+
105115
mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
106116
function_source_hash: hir_info.function_source_hash,
107117
num_counters: coverage_counters.num_counters(),
108118
mcdc_bitmap_bytes: coverage_spans.test_vector_bitmap_bytes(),
109119
expressions: coverage_counters.into_expressions(),
110120
mappings,
121+
mcdc_max_decision_depth,
111122
}));
112123
}
113124

0 commit comments

Comments
 (0)