diff options
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs | 3 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/coverage.rs | 3 | ||||
| -rw-r--r-- | compiler/rustc_mir_build/src/build/coverageinfo.rs | 77 | ||||
| -rw-r--r-- | compiler/rustc_mir_build/src/build/matches/mod.rs | 6 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/mod.rs | 11 |
5 files changed, 82 insertions, 18 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index eb1071448ce..679c6e1a2ff 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -206,7 +206,8 @@ fn ensure_mcdc_parameters<'ll, 'tcx>( let fn_name = bx.get_pgo_func_name_var(instance); let hash = bx.const_u64(function_coverage_info.function_source_hash); 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() .expect("already checked above") .mcdc_condition_bitmap_map diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 6383074742d..a3fa69160a7 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -275,6 +275,9 @@ pub struct FunctionCoverageInfo { pub mcdc_bitmap_bytes: u32, pub expressions: IndexVec<ExpressionId, Expression>, 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. diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 1818be2c1bd..a1c20fcfd83 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -101,10 +101,14 @@ impl BranchInfoBuilder { tcx: TyCtxt<'_>, true_marker: BlockMarkerId, false_marker: BlockMarkerId, - ) -> Option<ConditionInfo> { + ) -> Option<(u16, ConditionInfo)> { let mcdc_state = self.mcdc_state.as_mut()?; + let decision_depth = mcdc_state.decision_depth(); let (mut condition_info, decision_result) = 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 { match decision.conditions_num { 0 => { @@ -131,7 +135,7 @@ impl BranchInfoBuilder { } } } - condition_info + condition_info.map(|cond_info| (decision_depth, cond_info)) } fn add_two_way_branch<'tcx>( @@ -199,17 +203,32 @@ impl BranchInfoBuilder { /// 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; -struct MCDCState { +#[derive(Default)] +struct MCDCDecisionCtx { /// To construct condition evaluation tree. decision_stack: VecDeque<ConditionInfo>, processing_decision: Option<MCDCDecisionSpan>, } +struct MCDCState { + decision_ctx_stack: Vec<MCDCDecisionCtx>, +} + impl MCDCState { fn new_if_enabled(tcx: TyCtxt<'_>) -> Option<Self> { tcx.sess .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. @@ -253,20 +272,23 @@ impl MCDCState { // - 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". 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) => { decision.span = decision.span.to(span); decision } - None => self.processing_decision.insert(MCDCDecisionSpan { + None => decision_ctx.processing_decision.insert(MCDCDecisionSpan { span, conditions_num: 0, 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 { decision.conditions_num += 1; ConditionId::from(decision.conditions_num) @@ -306,8 +328,8 @@ impl MCDCState { } }; // We visit expressions tree in pre-order, so place the left-hand side on the top. - self.decision_stack.push_back(rhs); - self.decision_stack.push_back(lhs); + decision_ctx.decision_stack.push_back(rhs); + decision_ctx.decision_stack.push_back(lhs); } fn take_condition( @@ -315,10 +337,12 @@ impl MCDCState { true_marker: BlockMarkerId, false_marker: BlockMarkerId, ) -> (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); }; - 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"); }; if condition_info.true_next_id == ConditionId::NONE { @@ -328,8 +352,8 @@ impl MCDCState { decision.end_markers.push(false_marker); } - if self.decision_stack.is_empty() { - (Some(condition_info), self.processing_decision.take()) + if decision_ctx.decision_stack.is_empty() { + (Some(condition_info), decision_ctx.processing_decision.take()) } else { (Some(condition_info), None) } @@ -365,14 +389,17 @@ impl Builder<'_, '_> { |block| branch_info.inject_block_marker(&mut self.cfg, source_info, block); let true_marker = inject_block_marker(then_block); let false_marker = inject_block_marker(else_block); - let condition_info = - branch_info.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker); + let (decision_depth, condition_info) = branch_info + .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 { span: source_info.span, condition_info, true_marker, false_marker, - decision_depth: 0, + decision_depth, }); return; } @@ -387,4 +414,20 @@ impl Builder<'_, '_> { 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"); + }; + } } diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index f46dceeeedf..b36e97194c9 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -151,8 +151,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let mut block = block; let temp_scope = args.temp_scope_override.unwrap_or_else(|| this.local_scope()); let mutability = Mutability::Mut; + + // Increment the decision depth, in case we encounter boolean expressions + // further down. + this.mcdc_increment_depth_if_enabled(); let place = 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 then_block = this.cfg.start_new_block(); diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index a4c9e5ac0bc..159c099fac5 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -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); + 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 { function_source_hash: hir_info.function_source_hash, num_counters: coverage_counters.num_counters(), mcdc_bitmap_bytes: coverage_spans.test_vector_bitmap_bytes(), expressions: coverage_counters.into_expressions(), mappings, + mcdc_max_decision_depth, })); } |