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
This commit is contained in:
Dorian Péron 2024-04-10 10:13:04 +00:00
parent ae8c023983
commit 60ca9b6e29
5 changed files with 82 additions and 18 deletions

View File

@ -206,7 +206,8 @@ fn ensure_mcdc_parameters<'ll, 'tcx>(
let fn_name = bx.get_pgo_func_name_var(instance); let fn_name = bx.get_pgo_func_name_var(instance);
let hash = bx.const_u64(function_coverage_info.function_source_hash); let hash = bx.const_u64(function_coverage_info.function_source_hash);
let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes); let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes);
let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, 1_u32); let max_decision_depth = function_coverage_info.mcdc_max_decision_depth;
let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, max_decision_depth as u32);
bx.coverage_context() bx.coverage_context()
.expect("already checked above") .expect("already checked above")
.mcdc_condition_bitmap_map .mcdc_condition_bitmap_map

View File

@ -275,6 +275,9 @@ pub struct FunctionCoverageInfo {
pub mcdc_bitmap_bytes: u32, pub mcdc_bitmap_bytes: u32,
pub expressions: IndexVec<ExpressionId, Expression>, pub expressions: IndexVec<ExpressionId, Expression>,
pub mappings: Vec<Mapping>, pub mappings: Vec<Mapping>,
/// The depth of the deepest decision is used to know how many
/// temp condbitmaps should be allocated for the function.
pub mcdc_max_decision_depth: u16,
} }
/// Branch information recorded during THIR-to-MIR lowering, and stored in MIR. /// Branch information recorded during THIR-to-MIR lowering, and stored in MIR.

View File

@ -101,10 +101,14 @@ fn fetch_mcdc_condition_info(
tcx: TyCtxt<'_>, tcx: TyCtxt<'_>,
true_marker: BlockMarkerId, true_marker: BlockMarkerId,
false_marker: BlockMarkerId, false_marker: BlockMarkerId,
) -> Option<ConditionInfo> { ) -> Option<(u16, ConditionInfo)> {
let mcdc_state = self.mcdc_state.as_mut()?; let mcdc_state = self.mcdc_state.as_mut()?;
let decision_depth = mcdc_state.decision_depth();
let (mut condition_info, decision_result) = let (mut condition_info, decision_result) =
mcdc_state.take_condition(true_marker, false_marker); mcdc_state.take_condition(true_marker, false_marker);
// take_condition() returns Some for decision_result when the decision stack
// is empty, i.e. when all the conditions of the decision were instrumented,
// and the decision is "complete".
if let Some(decision) = decision_result { if let Some(decision) = decision_result {
match decision.conditions_num { match decision.conditions_num {
0 => { 0 => {
@ -131,7 +135,7 @@ fn fetch_mcdc_condition_info(
} }
} }
} }
condition_info condition_info.map(|cond_info| (decision_depth, cond_info))
} }
fn add_two_way_branch<'tcx>( fn add_two_way_branch<'tcx>(
@ -199,17 +203,32 @@ pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
/// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged. /// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged.
const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6; const MAX_CONDITIONS_NUM_IN_DECISION: usize = 6;
struct MCDCState { #[derive(Default)]
struct MCDCDecisionCtx {
/// To construct condition evaluation tree. /// To construct condition evaluation tree.
decision_stack: VecDeque<ConditionInfo>, decision_stack: VecDeque<ConditionInfo>,
processing_decision: Option<MCDCDecisionSpan>, processing_decision: Option<MCDCDecisionSpan>,
} }
struct MCDCState {
decision_ctx_stack: Vec<MCDCDecisionCtx>,
}
impl MCDCState { impl MCDCState {
fn new_if_enabled(tcx: TyCtxt<'_>) -> Option<Self> { fn new_if_enabled(tcx: TyCtxt<'_>) -> Option<Self> {
tcx.sess tcx.sess
.instrument_coverage_mcdc() .instrument_coverage_mcdc()
.then(|| Self { decision_stack: VecDeque::new(), processing_decision: None }) .then(|| Self { decision_ctx_stack: vec![MCDCDecisionCtx::default()] })
}
/// Decision depth is given as a u16 to reduce the size of the `CoverageKind`,
/// as it is very unlikely that the depth ever reaches 2^16.
#[inline]
fn decision_depth(&self) -> u16 {
u16::try_from(
self.decision_ctx_stack.len().checked_sub(1).expect("Unexpected empty decision stack"),
)
.expect("decision depth did not fit in u16, this is likely to be an instrumentation error")
} }
// At first we assign ConditionIds for each sub expression. // At first we assign ConditionIds for each sub expression.
@ -253,20 +272,23 @@ fn new_if_enabled(tcx: TyCtxt<'_>) -> Option<Self> {
// - 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". // - 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".
// - 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". // - 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".
fn record_conditions(&mut self, op: LogicalOp, span: Span) { fn record_conditions(&mut self, op: LogicalOp, span: Span) {
let decision = match self.processing_decision.as_mut() { let decision_depth = self.decision_depth();
let decision_ctx =
self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack");
let decision = match decision_ctx.processing_decision.as_mut() {
Some(decision) => { Some(decision) => {
decision.span = decision.span.to(span); decision.span = decision.span.to(span);
decision decision
} }
None => self.processing_decision.insert(MCDCDecisionSpan { None => decision_ctx.processing_decision.insert(MCDCDecisionSpan {
span, span,
conditions_num: 0, conditions_num: 0,
end_markers: vec![], end_markers: vec![],
decision_depth: 0, decision_depth,
}), }),
}; };
let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default();
let lhs_id = if parent_condition.condition_id == ConditionId::NONE { let lhs_id = if parent_condition.condition_id == ConditionId::NONE {
decision.conditions_num += 1; decision.conditions_num += 1;
ConditionId::from(decision.conditions_num) ConditionId::from(decision.conditions_num)
@ -306,8 +328,8 @@ fn record_conditions(&mut self, op: LogicalOp, span: Span) {
} }
}; };
// We visit expressions tree in pre-order, so place the left-hand side on the top. // We visit expressions tree in pre-order, so place the left-hand side on the top.
self.decision_stack.push_back(rhs); decision_ctx.decision_stack.push_back(rhs);
self.decision_stack.push_back(lhs); decision_ctx.decision_stack.push_back(lhs);
} }
fn take_condition( fn take_condition(
@ -315,10 +337,12 @@ fn take_condition(
true_marker: BlockMarkerId, true_marker: BlockMarkerId,
false_marker: BlockMarkerId, false_marker: BlockMarkerId,
) -> (Option<ConditionInfo>, Option<MCDCDecisionSpan>) { ) -> (Option<ConditionInfo>, Option<MCDCDecisionSpan>) {
let Some(condition_info) = self.decision_stack.pop_back() else { let decision_ctx =
self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack");
let Some(condition_info) = decision_ctx.decision_stack.pop_back() else {
return (None, None); return (None, None);
}; };
let Some(decision) = self.processing_decision.as_mut() else { let Some(decision) = decision_ctx.processing_decision.as_mut() else {
bug!("Processing decision should have been created before any conditions are taken"); bug!("Processing decision should have been created before any conditions are taken");
}; };
if condition_info.true_next_id == ConditionId::NONE { if condition_info.true_next_id == ConditionId::NONE {
@ -328,8 +352,8 @@ fn take_condition(
decision.end_markers.push(false_marker); decision.end_markers.push(false_marker);
} }
if self.decision_stack.is_empty() { if decision_ctx.decision_stack.is_empty() {
(Some(condition_info), self.processing_decision.take()) (Some(condition_info), decision_ctx.processing_decision.take())
} else { } else {
(Some(condition_info), None) (Some(condition_info), None)
} }
@ -365,14 +389,17 @@ pub(crate) fn visit_coverage_branch_condition(
|block| branch_info.inject_block_marker(&mut self.cfg, source_info, block); |block| branch_info.inject_block_marker(&mut self.cfg, source_info, block);
let true_marker = inject_block_marker(then_block); let true_marker = inject_block_marker(then_block);
let false_marker = inject_block_marker(else_block); let false_marker = inject_block_marker(else_block);
let condition_info = let (decision_depth, condition_info) = branch_info
branch_info.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker); .fetch_mcdc_condition_info(self.tcx, true_marker, false_marker)
.map_or((0, None), |(decision_depth, condition_info)| {
(decision_depth, Some(condition_info))
});
branch_info.mcdc_branch_spans.push(MCDCBranchSpan { branch_info.mcdc_branch_spans.push(MCDCBranchSpan {
span: source_info.span, span: source_info.span,
condition_info, condition_info,
true_marker, true_marker,
false_marker, false_marker,
decision_depth: 0, decision_depth,
}); });
return; return;
} }
@ -387,4 +414,20 @@ pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp,
mcdc_state.record_conditions(logical_op, span); mcdc_state.record_conditions(logical_op, span);
} }
} }
pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_state) = branch_info.mcdc_state.as_mut()
{
mcdc_state.decision_ctx_stack.push(MCDCDecisionCtx::default());
};
}
pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_state) = branch_info.mcdc_state.as_mut()
{
mcdc_state.decision_ctx_stack.pop().expect("Unexpected empty decision stack");
};
}
} }

View File

@ -151,8 +151,14 @@ fn then_else_break_inner(
let mut block = block; let mut block = block;
let temp_scope = args.temp_scope_override.unwrap_or_else(|| this.local_scope()); let temp_scope = args.temp_scope_override.unwrap_or_else(|| this.local_scope());
let mutability = Mutability::Mut; let mutability = Mutability::Mut;
// Increment the decision depth, in case we encounter boolean expressions
// further down.
this.mcdc_increment_depth_if_enabled();
let place = let place =
unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability)); unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability));
this.mcdc_decrement_depth_if_enabled();
let operand = Operand::Move(Place::from(place)); let operand = Operand::Move(Place::from(place));
let then_block = this.cfg.start_new_block(); let then_block = this.cfg.start_new_block();

View File

@ -102,12 +102,23 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
inject_mcdc_statements(mir_body, &basic_coverage_blocks, &coverage_spans); inject_mcdc_statements(mir_body, &basic_coverage_blocks, &coverage_spans);
let mcdc_max_decision_depth = coverage_spans
.mappings
.iter()
.filter_map(|bcb_mapping| match bcb_mapping.kind {
BcbMappingKind::MCDCDecision { decision_depth, .. } => Some(decision_depth),
_ => None,
})
.max()
.unwrap_or(0);
mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
function_source_hash: hir_info.function_source_hash, function_source_hash: hir_info.function_source_hash,
num_counters: coverage_counters.num_counters(), num_counters: coverage_counters.num_counters(),
mcdc_bitmap_bytes: coverage_spans.test_vector_bitmap_bytes(), mcdc_bitmap_bytes: coverage_spans.test_vector_bitmap_bytes(),
expressions: coverage_counters.into_expressions(), expressions: coverage_counters.into_expressions(),
mappings, mappings,
mcdc_max_decision_depth,
})); }));
} }