From 3c2f48ede9ce927950d74832722e6f716a69d974 Mon Sep 17 00:00:00 2001 From: Dorian Péron Date: Mon, 8 Apr 2024 14:23:50 +0000 Subject: mcdc-coverage: Add possibility for codegen llvm to handle several condition bitmaps --- compiler/rustc_codegen_llvm/src/builder.rs | 31 ++++++++++++++-------- .../rustc_codegen_llvm/src/coverageinfo/mod.rs | 25 ++++++++++++----- 2 files changed, 38 insertions(+), 18 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 4c2bdb2f5ec..bd09797a677 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -27,6 +27,7 @@ use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange}; use rustc_target::spec::{HasTargetSpec, SanitizerSet, Target}; use smallvec::SmallVec; use std::borrow::Cow; +use std::ffi::CString; use std::iter; use std::ops::Deref; use std::ptr; @@ -1708,7 +1709,8 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { fn_name: &'ll Value, hash: &'ll Value, bitmap_bytes: &'ll Value, - ) -> &'ll Value { + max_decision_depth: u32, + ) -> Vec<&'ll Value> { debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes); assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later"); @@ -1721,6 +1723,8 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { let args = &[fn_name, hash, bitmap_bytes]; let args = self.check_call("call", llty, llfn, args); + let mut cond_bitmaps = vec![]; + unsafe { let _ = llvm::LLVMRustBuildCall( self.llbuilder, @@ -1732,17 +1736,22 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { 0 as c_uint, ); // Create condition bitmap named `mcdc.addr`. - let mut bx = Builder::with_cx(self.cx); - bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn())); - let cond_bitmap = { - let alloca = - llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), c"mcdc.addr".as_ptr()); - llvm::LLVMSetAlignment(alloca, 4); - alloca - }; - bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi); - cond_bitmap + for i in 0..=max_decision_depth { + let mut bx = Builder::with_cx(self.cx); + bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn())); + + let name = CString::new(format!("mcdc.addr.{i}")).unwrap(); + let cond_bitmap = { + let alloca = + llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), name.as_ptr()); + llvm::LLVMSetAlignment(alloca, 4); + alloca + }; + bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi); + cond_bitmaps.push(cond_bitmap); + } } + cond_bitmaps } pub(crate) fn mcdc_tvbitmap_update( diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 085ce15d81f..03fc2ae425b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -30,7 +30,7 @@ pub struct CrateCoverageContext<'ll, 'tcx> { pub(crate) function_coverage_map: RefCell, FunctionCoverageCollector<'tcx>>>, pub(crate) pgo_func_name_var_map: RefCell, &'ll llvm::Value>>, - pub(crate) mcdc_condition_bitmap_map: RefCell, &'ll llvm::Value>>, + pub(crate) mcdc_condition_bitmap_map: RefCell, Vec<&'ll llvm::Value>>>, } impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { @@ -49,9 +49,20 @@ impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> { } /// LLVM use a temp value to record evaluated mcdc test vector of each decision, which is called condition bitmap. - /// This value is named `mcdc.addr` (same as clang) and is a 32-bit integer. - fn try_get_mcdc_condition_bitmap(&self, instance: &Instance<'tcx>) -> Option<&'ll llvm::Value> { - self.mcdc_condition_bitmap_map.borrow().get(instance).copied() + /// In order to handle nested decisions, several condition bitmaps can be + /// allocated for a function body. + /// These values are named `mcdc.addr.{i}` and are a 32-bit integers. + /// They respectively hold the condition bitmaps for decisions with a depth of `i`. + fn try_get_mcdc_condition_bitmap( + &self, + instance: &Instance<'tcx>, + decision_depth: u16, + ) -> Option<&'ll llvm::Value> { + self.mcdc_condition_bitmap_map + .borrow() + .get(instance) + .and_then(|bitmap_map| bitmap_map.get(decision_depth as usize)) + .copied() // Dereference Option<&&Value> to Option<&Value> } } @@ -151,7 +162,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { "ConditionId of evaluated conditions should never be zero" ); let cond_bitmap = coverage_context - .try_get_mcdc_condition_bitmap(&instance) + .try_get_mcdc_condition_bitmap(&instance, 0) .expect("mcdc cond bitmap should have been allocated for updating"); let cond_loc = bx.const_i32(id.as_u32() as i32 - 1); let bool_value = bx.const_bool(value); @@ -162,7 +173,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::TestVectorBitmapUpdate { bitmap_idx } => { drop(coverage_map); let cond_bitmap = coverage_context - .try_get_mcdc_condition_bitmap(&instance) + .try_get_mcdc_condition_bitmap(&instance, 0) .expect("mcdc cond bitmap should have been allocated for merging into the global bitmap"); let bitmap_bytes = bx.tcx().coverage_ids_info(instance.def).mcdc_bitmap_bytes; assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range"); @@ -195,7 +206,7 @@ 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); + let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, 1_u32); bx.coverage_context() .expect("already checked above") .mcdc_condition_bitmap_map -- cgit 1.4.1-3-g733a5 From ae8c023983247044838a1a8b5f6fb5dd606fa68d Mon Sep 17 00:00:00 2001 From: Dorian Péron Date: Mon, 8 Apr 2024 14:44:25 +0000 Subject: mcdc-coverage: Add decision_depth field in structs Add decision_depth field to TVBitmapUpdate/CondBitmapUpdate statements Add decision_depth field to BcbMappingKinds MCDCBranch and MCDCDecision Add decision_depth field to MCDCBranchSpan and MCDCDecisionSpan --- .../rustc_codegen_llvm/src/coverageinfo/mod.rs | 8 ++--- compiler/rustc_middle/src/mir/coverage.rs | 20 +++++++++---- compiler/rustc_middle/src/mir/pretty.rs | 17 +++++++---- compiler/rustc_mir_build/src/build/coverageinfo.rs | 2 ++ compiler/rustc_mir_transform/src/coverage/mod.rs | 35 ++++++++++++---------- compiler/rustc_mir_transform/src/coverage/spans.rs | 8 ++++- .../src/coverage/spans/from_mir.rs | 27 ++++++++++++----- 7 files changed, 78 insertions(+), 39 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 03fc2ae425b..eb1071448ce 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -154,7 +154,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::ExpressionUsed { id } => { func_coverage.mark_expression_id_seen(id); } - CoverageKind::CondBitmapUpdate { id, value, .. } => { + CoverageKind::CondBitmapUpdate { id, value, decision_depth } => { drop(coverage_map); assert_ne!( id.as_u32(), @@ -162,7 +162,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { "ConditionId of evaluated conditions should never be zero" ); let cond_bitmap = coverage_context - .try_get_mcdc_condition_bitmap(&instance, 0) + .try_get_mcdc_condition_bitmap(&instance, decision_depth) .expect("mcdc cond bitmap should have been allocated for updating"); let cond_loc = bx.const_i32(id.as_u32() as i32 - 1); let bool_value = bx.const_bool(value); @@ -170,10 +170,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { let hash = bx.const_u64(function_coverage_info.function_source_hash); bx.mcdc_condbitmap_update(fn_name, hash, cond_loc, cond_bitmap, bool_value); } - CoverageKind::TestVectorBitmapUpdate { bitmap_idx } => { + CoverageKind::TestVectorBitmapUpdate { bitmap_idx, decision_depth } => { drop(coverage_map); let cond_bitmap = coverage_context - .try_get_mcdc_condition_bitmap(&instance, 0) + .try_get_mcdc_condition_bitmap(&instance, decision_depth) .expect("mcdc cond bitmap should have been allocated for merging into the global bitmap"); let bitmap_bytes = bx.tcx().coverage_ids_info(instance.def).mcdc_bitmap_bytes; assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range"); diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 04011fd4194..6383074742d 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -132,7 +132,7 @@ pub enum CoverageKind { /// /// If this statement does not survive MIR optimizations, the condition would never be /// taken as evaluated. - CondBitmapUpdate { id: ConditionId, value: bool }, + CondBitmapUpdate { id: ConditionId, value: bool, decision_depth: u16 }, /// Marks the point in MIR control flow represented by a evaluated decision. /// @@ -140,7 +140,7 @@ pub enum CoverageKind { /// /// If this statement does not survive MIR optimizations, the decision would never be /// taken as evaluated. - TestVectorBitmapUpdate { bitmap_idx: u32 }, + TestVectorBitmapUpdate { bitmap_idx: u32, decision_depth: u16 }, } impl Debug for CoverageKind { @@ -151,11 +151,17 @@ impl Debug for CoverageKind { BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()), CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), - CondBitmapUpdate { id, value } => { - write!(fmt, "CondBitmapUpdate({:?}, {:?})", id.index(), value) + CondBitmapUpdate { id, value, decision_depth } => { + write!( + fmt, + "CondBitmapUpdate({:?}, {:?}, depth={:?})", + id.index(), + value, + decision_depth + ) } - TestVectorBitmapUpdate { bitmap_idx } => { - write!(fmt, "TestVectorUpdate({:?})", bitmap_idx) + TestVectorBitmapUpdate { bitmap_idx, decision_depth } => { + write!(fmt, "TestVectorUpdate({:?}, depth={:?})", bitmap_idx, decision_depth) } } } @@ -319,6 +325,7 @@ pub struct MCDCBranchSpan { pub condition_info: Option, pub true_marker: BlockMarkerId, pub false_marker: BlockMarkerId, + pub decision_depth: u16, } #[derive(Copy, Clone, Debug)] @@ -334,4 +341,5 @@ pub struct MCDCDecisionSpan { pub span: Span, pub conditions_num: usize, pub end_markers: Vec, + pub decision_depth: u16, } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 7a91d7383e5..32cd3183410 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -496,20 +496,27 @@ fn write_coverage_branch_info( )?; } - for coverage::MCDCBranchSpan { span, condition_info, true_marker, false_marker } in - mcdc_branch_spans + for coverage::MCDCBranchSpan { + span, + condition_info, + true_marker, + false_marker, + decision_depth, + } in mcdc_branch_spans { writeln!( w, - "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", + "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?}, depth: {decision_depth:?} }} => {span:?}", condition_info.map(|info| info.condition_id) )?; } - for coverage::MCDCDecisionSpan { span, conditions_num, end_markers } in mcdc_decision_spans { + for coverage::MCDCDecisionSpan { span, conditions_num, end_markers, decision_depth } in + mcdc_decision_spans + { writeln!( w, - "{INDENT}coverage mcdc decision {{ conditions_num: {conditions_num:?}, end: {end_markers:?} }} => {span:?}" + "{INDENT}coverage mcdc decision {{ conditions_num: {conditions_num:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}" )?; } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 9e9ccd3dc2d..1818be2c1bd 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -262,6 +262,7 @@ impl MCDCState { span, conditions_num: 0, end_markers: vec![], + decision_depth: 0, }), }; @@ -371,6 +372,7 @@ impl Builder<'_, '_> { condition_info, true_marker, false_marker, + decision_depth: 0, }); return; } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 0b15c52c281..a4c9e5ac0bc 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -145,16 +145,17 @@ fn create_mappings<'tcx>( |BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match *bcb_mapping_kind { BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), - BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info: None } => { - MappingKind::Branch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - } - } + BcbMappingKind::MCDCBranch { + true_bcb, false_bcb, condition_info: None, .. + } => MappingKind::Branch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + }, BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info: Some(mcdc_params), + .. } => MappingKind::MCDCBranch { true_term: term_for_bcb(true_bcb), false_term: term_for_bcb(false_bcb), @@ -246,24 +247,28 @@ fn inject_mcdc_statements<'tcx>( } // Inject test vector update first because `inject_statement` always insert new statement at head. - for (end_bcbs, bitmap_idx) in + for (end_bcbs, bitmap_idx, decision_depth) in coverage_spans.mappings.iter().filter_map(|mapping| match &mapping.kind { - BcbMappingKind::MCDCDecision { end_bcbs, bitmap_idx, .. } => { - Some((end_bcbs, *bitmap_idx)) + BcbMappingKind::MCDCDecision { end_bcbs, bitmap_idx, decision_depth, .. } => { + Some((end_bcbs, *bitmap_idx, *decision_depth)) } _ => None, }) { for end in end_bcbs { let end_bb = basic_coverage_blocks[*end].leader_bb(); - inject_statement(mir_body, CoverageKind::TestVectorBitmapUpdate { bitmap_idx }, end_bb); + inject_statement( + mir_body, + CoverageKind::TestVectorBitmapUpdate { bitmap_idx, decision_depth }, + end_bb, + ); } } - for (true_bcb, false_bcb, condition_id) in + for (true_bcb, false_bcb, condition_id, decision_depth) in coverage_spans.mappings.iter().filter_map(|mapping| match mapping.kind { - BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => { - Some((true_bcb, false_bcb, condition_info?.condition_id)) + BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info, decision_depth } => { + Some((true_bcb, false_bcb, condition_info?.condition_id, decision_depth)) } _ => None, }) @@ -271,13 +276,13 @@ fn inject_mcdc_statements<'tcx>( let true_bb = basic_coverage_blocks[true_bcb].leader_bb(); inject_statement( mir_body, - CoverageKind::CondBitmapUpdate { id: condition_id, value: true }, + CoverageKind::CondBitmapUpdate { id: condition_id, value: true, decision_depth }, true_bb, ); let false_bb = basic_coverage_blocks[false_bcb].leader_bb(); inject_statement( mir_body, - CoverageKind::CondBitmapUpdate { id: condition_id, value: false }, + CoverageKind::CondBitmapUpdate { id: condition_id, value: false, decision_depth }, false_bb, ); } diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 88f18b72085..b64b1212cec 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -26,9 +26,15 @@ pub(super) enum BcbMappingKind { /// If `None`, this actually represents a normal branch mapping inserted /// for code that was too complex for MC/DC. condition_info: Option, + decision_depth: u16, }, /// Associates a mcdc decision with its join BCB. - MCDCDecision { end_bcbs: BTreeSet, bitmap_idx: u32, conditions_num: u16 }, + MCDCDecision { + end_bcbs: BTreeSet, + bitmap_idx: u32, + conditions_num: u16, + decision_depth: u16, + }, } #[derive(Debug)] diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 64f21d74b1c..142dfc17fb8 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -453,15 +453,25 @@ pub(super) fn extract_mcdc_mappings( Some((span, true_bcb, false_bcb)) }; - let mcdc_branch_filter_map = - |&MCDCBranchSpan { span: raw_span, true_marker, false_marker, condition_info }| { - check_branch_bcb(raw_span, true_marker, false_marker).map( - |(span, true_bcb, false_bcb)| BcbMapping { - kind: BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info }, - span, + let mcdc_branch_filter_map = |&MCDCBranchSpan { + span: raw_span, + true_marker, + false_marker, + condition_info, + decision_depth, + }| { + check_branch_bcb(raw_span, true_marker, false_marker).map(|(span, true_bcb, false_bcb)| { + BcbMapping { + kind: BcbMappingKind::MCDCBranch { + true_bcb, + false_bcb, + condition_info, + decision_depth, }, - ) - }; + span, + } + }) + }; let mut next_bitmap_idx = 0; @@ -482,6 +492,7 @@ pub(super) fn extract_mcdc_mappings( end_bcbs, bitmap_idx, conditions_num: decision.conditions_num as u16, + decision_depth: decision.decision_depth, }, span, }) -- cgit 1.4.1-3-g733a5 From 60ca9b6e29975ab36e5460d761d99681bef0f130 Mon Sep 17 00:00:00 2001 From: Dorian Péron Date: Wed, 10 Apr 2024 10:13:04 +0000 Subject: 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 --- .../rustc_codegen_llvm/src/coverageinfo/mod.rs | 3 +- compiler/rustc_middle/src/mir/coverage.rs | 3 + compiler/rustc_mir_build/src/build/coverageinfo.rs | 77 +++++++++++++++++----- compiler/rustc_mir_build/src/build/matches/mod.rs | 6 ++ compiler/rustc_mir_transform/src/coverage/mod.rs | 11 ++++ 5 files changed, 82 insertions(+), 18 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src') 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, pub mappings: Vec, + /// 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 { + ) -> 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, processing_decision: Option, } +struct MCDCState { + decision_ctx_stack: Vec, +} + impl MCDCState { fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { 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, Option) { - 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, })); } -- cgit 1.4.1-3-g733a5