coverage. Add BlockMarker for decisions and inject mcdc statements

Author: zhuyunxing

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -224,19 +224,27 @@ impl CounterMappingRegion {
                 end_line,
                 end_col,
             ),
-            MappingKind::Branch { true_term, false_term, mcdc_params: condition_info } => {
-                Self::branch_region(
-                    Counter::from_term(true_term),
-                    Counter::from_term(false_term),
-                    condition_info,
-                    local_file_id,
-                    start_line,
-                    start_col,
-                    end_line,
-                    end_col,
-                )
-            }
-            MappingKind::Decision(decision_info) => Self::decision_region(
+            MappingKind::Branch { true_term, false_term } => Self::branch_region(
+                Counter::from_term(true_term),
+                Counter::from_term(false_term),
+                None,
+                local_file_id,
+                start_line,
+                start_col,
+                end_line,
+                end_col,
+            ),
+            MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => Self::branch_region(
+                Counter::from_term(true_term),
+                Counter::from_term(false_term),
+                Some(mcdc_params),
+                local_file_id,
+                start_line,
+                start_col,
+                end_line,
+                end_col,
+            ),
+            MappingKind::MCDCDecision(decision_info) => Self::decision_region(
                 decision_info,
                 local_file_id,
                 start_line,
@@ -272,14 +280,17 @@ impl CounterMappingRegion {
     pub(crate) fn branch_region(
         counter: Counter,
         false_counter: Counter,
-        condition_info: ConditionInfo,
+        condition_info: Option<ConditionInfo>,
         file_id: u32,
         start_line: u32,
         start_col: u32,
         end_line: u32,
         end_col: u32,
     ) -> Self {
-        let mcdc_params = mcdc::Parameters::Branch(condition_info.into());
+        let mcdc_params = condition_info
+            .map(mcdc::BranchParameters::from)
+            .map(mcdc::Parameters::Branch)
+            .unwrap_or(mcdc::Parameters::None);
         let kind = match mcdc_params {
             mcdc::Parameters::None => RegionKind::BranchRegion,
             mcdc::Parameters::Branch(_) => RegionKind::MCDCBranchRegion,
@@ -307,10 +318,12 @@ impl CounterMappingRegion {
         end_line: u32,
         end_col: u32,
     ) -> Self {
+        let mcdc_params = mcdc::Parameters::Decision(decision_info.into());
+
         Self {
             counter: Counter::ZERO,
             false_counter: Counter::ZERO,
-            mcdc_params: mcdc::Parameters::Decision(decision_info.into()),
+            mcdc_params,
             file_id,
             expanded_file_id: 0,
             start_line,

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -98,12 +98,8 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
             return;
         };
 
-        match *kind {
-            // Ensure mcdc parameters are properly initialized for these operations.
-            CoverageKind::UpdateCondBitmap { .. } | CoverageKind::UpdateTestVector { .. } => {
-                ensure_mcdc_parameters(bx, instance, function_coverage_info)
-            }
-            _ => {}
+        if function_coverage_info.mcdc_bitmap_bytes > 0 {
+            ensure_mcdc_parameters(bx, instance, function_coverage_info);
         }
 
         let Some(coverage_context) = bx.coverage_context() else { return };
@@ -147,23 +143,27 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
             CoverageKind::ExpressionUsed { id } => {
                 func_coverage.mark_expression_id_seen(id);
             }
-
-            CoverageKind::UpdateCondBitmap { id, value } => {
+            CoverageKind::CondBitmapUpdate { id, value, .. } => {
                 drop(coverage_map);
+                assert_ne!(
+                    id.as_u32(),
+                    0,
+                    "ConditionId of evaluated conditions should never be zero"
+                );
                 let cond_bitmap = coverage_context
                     .try_get_mcdc_condition_bitmap(&instance)
-                    .expect("mcdc cond bitmap must be set before being updated");
+                    .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);
                 let fn_name = bx.get_pgo_func_name_var(instance);
                 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::UpdateTestVector { bitmap_idx } => {
+            CoverageKind::TestVectorBitmapUpdate { bitmap_idx } => {
                 drop(coverage_map);
                 let cond_bitmap = coverage_context
-                    .try_get_mcdc_condition_bitmap(&instance)
-                    .expect("mcdc cond bitmap must be set before being updated");
+                                    .try_get_mcdc_condition_bitmap(&instance)
+                                    .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");
                 assert!(

compiler/rustc_middle/src/mir/coverage.rs

@@ -112,9 +112,7 @@ pub enum CoverageKind {
     ///
     /// If this statement does not survive MIR optimizations, any mappings that
     /// refer to this counter can have those references simplified to zero.
-    CounterIncrement {
-        id: CounterId,
-    },
+    CounterIncrement { id: CounterId },
 
     /// Marks the point in MIR control-flow represented by a coverage expression.
     ///
@@ -124,18 +122,23 @@ pub enum CoverageKind {
     /// (This is only inserted for expression IDs that are directly used by
     /// mappings. Intermediate expressions with no direct mappings are
     /// retained/zeroed based on whether they are transitively used.)
-    ExpressionUsed {
-        id: ExpressionId,
-    },
-
-    UpdateCondBitmap {
-        id: ConditionId,
-        value: bool,
-    },
-
-    UpdateTestVector {
-        bitmap_idx: u32,
-    },
+    ExpressionUsed { id: ExpressionId },
+
+    /// Marks the point in MIR control flow represented by a evaluated condition.
+    ///
+    /// This is eventually lowered to `llvm.instrprof.mcdc.condbitmap.update` in LLVM IR.
+    ///
+    /// If this statement does not survive MIR optimizations, the condition would never be
+    /// taken as evaluated.
+    CondBitmapUpdate { id: ConditionId, value: bool },
+
+    /// Marks the point in MIR control flow represented by a evaluated decision.
+    ///
+    /// This is eventually lowered to `llvm.instrprof.mcdc.tvbitmap.update` in LLVM IR.
+    ///
+    /// If this statement does not survive MIR optimizations, the decision would never be
+    /// taken as evaluated.
+    TestVectorBitmapUpdate { bitmap_idx: u32 },
 }
 
 impl Debug for CoverageKind {
@@ -146,10 +149,12 @@ 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()),
-            UpdateCondBitmap { id, value } => {
-                write!(fmt, "UpdateCondBitmap({:?}, {value})", id.index())
+            CondBitmapUpdate { id, value } => {
+                write!(fmt, "CondBitmapUpdate({:?}, {:?})", id.index(), value)
+            }
+            TestVectorBitmapUpdate { bitmap_idx } => {
+                write!(fmt, "TestVectorUpdate({:?})", bitmap_idx)
             }
-            UpdateTestVector { bitmap_idx } => write!(fmt, "UpdateTestVector({:?})", bitmap_idx),
         }
     }
 }
@@ -205,9 +210,11 @@ pub enum MappingKind {
     /// Associates a normal region of code with a counter/expression/zero.
     Code(CovTerm),
     /// Associates a branch region with separate counters for true and false.
-    Branch { true_term: CovTerm, false_term: CovTerm, mcdc_params: ConditionInfo },
+    Branch { true_term: CovTerm, false_term: CovTerm },
+    /// Associates a branch region with separate counters for true and false.
+    MCDCBranch { true_term: CovTerm, false_term: CovTerm, mcdc_params: ConditionInfo },
     /// Associates a decision region with a bitmap and number of conditions.
-    Decision(DecisionInfo),
+    MCDCDecision(DecisionInfo),
 }
 
 impl MappingKind {
@@ -219,7 +226,8 @@ impl MappingKind {
         match *self {
             Self::Code(term) => one(term),
             Self::Branch { true_term, false_term, .. } => two(true_term, false_term),
-            Self::Decision(_) => zero(),
+            Self::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term),
+            Self::MCDCDecision(_) => zero(),
         }
     }
 
@@ -228,12 +236,15 @@ impl MappingKind {
     pub fn map_terms(&self, map_fn: impl Fn(CovTerm) -> CovTerm) -> Self {
         match *self {
             Self::Code(term) => Self::Code(map_fn(term)),
-            Self::Branch { true_term, false_term, mcdc_params } => Self::Branch {
+            Self::Branch { true_term, false_term } => {
+                Self::Branch { true_term: map_fn(true_term), false_term: map_fn(false_term) }
+            }
+            Self::MCDCBranch { true_term, false_term, mcdc_params } => Self::MCDCBranch {
                 true_term: map_fn(true_term),
                 false_term: map_fn(false_term),
                 mcdc_params,
             },
-            Self::Decision(param) => Self::Decision(param),
+            Self::MCDCDecision(param) => Self::MCDCDecision(param),
         }
     }
 }
@@ -267,7 +278,6 @@ pub struct BranchInfo {
     /// data structures without having to scan the entire body first.
     pub num_block_markers: usize,
     pub branch_spans: Vec<BranchSpan>,
-    pub mcdc_bitmap_bytes_num: u32,
     pub decision_spans: Vec<DecisionSpan>,
 }
 
@@ -309,5 +319,6 @@ pub struct DecisionInfo {
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct DecisionSpan {
     pub span: Span,
-    pub mcdc_params: DecisionInfo,
+    pub conditions_num: u16,
+    pub join_marker: BlockMarkerId,
 }