coverage: Consistently remove unused counter IDs from expressions/mappings

Author: Zalathar

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

@@ -102,7 +102,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
         // have zero as both of their operands, and will therefore always have
         // a value of zero. Other expressions that refer to these as operands
         // can have those operands replaced with `CovTerm::Zero`.
-        let mut zero_expressions = FxIndexSet::default();
+        let mut zero_expressions = ZeroExpressions::default();
 
         // Simplify a copy of each expression based on lower-numbered expressions,
         // and then update the set of always-zero expressions if necessary.
@@ -131,16 +131,16 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
                 )
             };
 
-            // If an operand refers to an expression that is always zero, then
-            // that operand can be replaced with `CovTerm::Zero`.
-            let maybe_set_operand_to_zero = |operand: &mut CovTerm| match *operand {
-                CovTerm::Expression(id) => {
+            // If an operand refers to a counter or expression that is always
+            // zero, then that operand can be replaced with `CovTerm::Zero`.
+            let maybe_set_operand_to_zero = |operand: &mut CovTerm| {
+                if let CovTerm::Expression(id) = *operand {
                     assert_operand_expression_is_lower(id);
-                    if zero_expressions.contains(&id) {
-                        *operand = CovTerm::Zero;
-                    }
                 }
-                _ => (),
+
+                if is_zero_term(&self.counters_seen, &zero_expressions, *operand) {
+                    *operand = CovTerm::Zero;
+                }
             };
             maybe_set_operand_to_zero(&mut lhs);
             maybe_set_operand_to_zero(&mut rhs);
@@ -159,7 +159,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
             }
         }
 
-        ZeroExpressions(zero_expressions)
+        zero_expressions
     }
 
     pub(crate) fn into_finished(self) -> FunctionCoverage<'tcx> {
@@ -205,19 +205,14 @@ impl<'tcx> FunctionCoverage<'tcx> {
         // thing on the Rust side unless we're confident we can do much better.
         // (See `CounterExpressionsMinimizer` in `CoverageMappingWriter.cpp`.)
 
-        let counter_from_operand = |operand: CovTerm| match operand {
-            CovTerm::Expression(id) if self.zero_expressions.contains(id) => Counter::ZERO,
-            _ => Counter::from_term(operand),
-        };
-
         self.function_coverage_info.expressions.iter().map(move |&Expression { lhs, op, rhs }| {
             CounterExpression {
-                lhs: counter_from_operand(lhs),
+                lhs: self.counter_for_term(lhs),
                 kind: match op {
                     Op::Add => ExprKind::Add,
                     Op::Subtract => ExprKind::Subtract,
                 },
-                rhs: counter_from_operand(rhs),
+                rhs: self.counter_for_term(rhs),
             }
         })
     }
@@ -227,34 +222,49 @@ impl<'tcx> FunctionCoverage<'tcx> {
     pub(crate) fn counter_regions(
         &self,
     ) -> impl Iterator<Item = (Counter, &CodeRegion)> + ExactSizeIterator {
-        // Historically, mappings were stored directly in counter/expression
-        // statements in MIR, and MIR optimizations would sometimes remove them.
-        // That's mostly no longer true, so now we detect cases where that would
-        // have happened, and zero out the corresponding mappings here instead.
-        let counter_for_term = move |term: CovTerm| {
-            let force_to_zero = match term {
-                CovTerm::Counter(id) => !self.counters_seen.contains(id),
-                CovTerm::Expression(id) => self.zero_expressions.contains(id),
-                CovTerm::Zero => false,
-            };
-            if force_to_zero { Counter::ZERO } else { Counter::from_term(term) }
-        };
-
         self.function_coverage_info.mappings.iter().map(move |mapping| {
             let &Mapping { term, ref code_region } = mapping;
-            let counter = counter_for_term(term);
+            let counter = self.counter_for_term(term);
             (counter, code_region)
         })
     }
+
+    fn counter_for_term(&self, term: CovTerm) -> Counter {
+        if is_zero_term(&self.counters_seen, &self.zero_expressions, term) {
+            Counter::ZERO
+        } else {
+            Counter::from_term(term)
+        }
+    }
 }
 
 /// Set of expression IDs that are known to always evaluate to zero.
 /// Any mapping or expression operand that refers to these expressions can have
 /// that reference replaced with a constant zero value.
+#[derive(Default)]
 struct ZeroExpressions(FxIndexSet<ExpressionId>);
 
 impl ZeroExpressions {
+    fn insert(&mut self, id: ExpressionId) {
+        self.0.insert(id);
+    }
+
     fn contains(&self, id: ExpressionId) -> bool {
         self.0.contains(&id)
     }
 }
+
+/// Returns `true` if the given term is known to have a value of zero, taking
+/// into account knowledge of which counters are unused and which expressions
+/// are always zero.
+fn is_zero_term(
+    counters_seen: &BitSet<CounterId>,
+    zero_expressions: &ZeroExpressions,
+    term: CovTerm,
+) -> bool {
+    match term {
+        CovTerm::Zero => true,
+        CovTerm::Counter(id) => !counters_seen.contains(id),
+        CovTerm::Expression(id) => zero_expressions.contains(id),
+    }
+}

tests/coverage-map/fn_sig_into_try.cov-map

@@ -7,47 +7,47 @@ Number of file 0 mappings: 1
 - Code(Counter(0)) at (prev + 10, 1) to (start + 4, 2)
 
 Function name: fn_sig_into_try::b
-Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 10, 01, 02, 0f, 00, 02, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02]
+Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 10, 01, 02, 0f, 00, 02, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 2
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
+- expression 1 operands: lhs = Zero, rhs = Expression(0, Sub)
 Number of file 0 mappings: 4
 - Code(Counter(0)) at (prev + 16, 1) to (start + 2, 15)
 - Code(Zero) at (prev + 2, 15) to (start + 0, 16)
 - Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 12)
-    = (c0 - c1)
+    = (c0 - Zero)
 - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2)
-    = (c1 + (c0 - c1))
+    = (Zero + (c0 - Zero))
 
 Function name: fn_sig_into_try::c
-Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 16, 01, 02, 17, 00, 02, 17, 00, 18, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02]
+Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 16, 01, 02, 17, 00, 02, 17, 00, 18, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 2
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
+- expression 1 operands: lhs = Zero, rhs = Expression(0, Sub)
 Number of file 0 mappings: 4
 - Code(Counter(0)) at (prev + 22, 1) to (start + 2, 23)
 - Code(Zero) at (prev + 2, 23) to (start + 0, 24)
 - Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 12)
-    = (c0 - c1)
+    = (c0 - Zero)
 - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2)
-    = (c1 + (c0 - c1))
+    = (Zero + (c0 - Zero))
 
 Function name: fn_sig_into_try::d
-Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 1c, 01, 03, 0f, 00, 03, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02]
+Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 1c, 01, 03, 0f, 00, 03, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 2
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
+- expression 1 operands: lhs = Zero, rhs = Expression(0, Sub)
 Number of file 0 mappings: 4
 - Code(Counter(0)) at (prev + 28, 1) to (start + 3, 15)
 - Code(Zero) at (prev + 3, 15) to (start + 0, 16)
 - Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 12)
-    = (c0 - c1)
+    = (c0 - Zero)
 - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2)
-    = (c1 + (c0 - c1))
+    = (Zero + (c0 - Zero))
 

tests/coverage-map/status-quo/bad_counter_ids.cov-map

@@ -24,15 +24,15 @@ Number of file 0 mappings: 2
 - Code(Zero) at (prev + 3, 1) to (start + 0, 2)
 
 Function name: bad_counter_ids::eq_bad_message
-Raw bytes (21): 0x[01, 01, 01, 01, 05, 03, 01, 28, 01, 02, 0f, 02, 02, 20, 00, 2b, 00, 01, 01, 00, 02]
+Raw bytes (21): 0x[01, 01, 01, 01, 00, 03, 01, 28, 01, 02, 0f, 02, 02, 20, 00, 2b, 00, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 1
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
 Number of file 0 mappings: 3
 - Code(Counter(0)) at (prev + 40, 1) to (start + 2, 15)
 - Code(Expression(0, Sub)) at (prev + 2, 32) to (start + 0, 43)
-    = (c0 - c1)
+    = (c0 - Zero)
 - Code(Zero) at (prev + 1, 1) to (start + 0, 2)
 
 Function name: bad_counter_ids::eq_good
@@ -74,25 +74,25 @@ Number of file 0 mappings: 3
 - Code(Zero) at (prev + 1, 1) to (start + 0, 2)
 
 Function name: bad_counter_ids::ne_good
-Raw bytes (16): 0x[01, 01, 01, 01, 05, 02, 01, 19, 01, 02, 1f, 02, 03, 01, 00, 02]
+Raw bytes (16): 0x[01, 01, 01, 01, 00, 02, 01, 19, 01, 02, 1f, 02, 03, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 1
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
 Number of file 0 mappings: 2
 - Code(Counter(0)) at (prev + 25, 1) to (start + 2, 31)
 - Code(Expression(0, Sub)) at (prev + 3, 1) to (start + 0, 2)
-    = (c0 - c1)
+    = (c0 - Zero)
 
 Function name: bad_counter_ids::ne_good_message
-Raw bytes (21): 0x[01, 01, 01, 01, 05, 03, 01, 1e, 01, 02, 0f, 00, 02, 20, 00, 2b, 02, 01, 01, 00, 02]
+Raw bytes (21): 0x[01, 01, 01, 01, 00, 03, 01, 1e, 01, 02, 0f, 00, 02, 20, 00, 2b, 02, 01, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 1
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
 Number of file 0 mappings: 3
 - Code(Counter(0)) at (prev + 30, 1) to (start + 2, 15)
 - Code(Zero) at (prev + 2, 32) to (start + 0, 43)
 - Code(Expression(0, Sub)) at (prev + 1, 1) to (start + 0, 2)
-    = (c0 - c1)
+    = (c0 - Zero)
 

tests/coverage-map/status-quo/inline-dead.cov-map

@@ -7,19 +7,19 @@ Number of file 0 mappings: 1
 - Code(Zero) at (prev + 25, 1) to (start + 2, 2)
 
 Function name: inline_dead::live::<false>
-Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 10, 01, 01, 09, 00, 02, 09, 00, 0f, 02, 02, 09, 00, 0a, 07, 02, 01, 00, 02]
+Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 10, 01, 01, 09, 00, 02, 09, 00, 0f, 02, 02, 09, 00, 0a, 07, 02, 01, 00, 02]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 2
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub)
+- expression 0 operands: lhs = Counter(0), rhs = Zero
+- expression 1 operands: lhs = Zero, rhs = Expression(0, Sub)
 Number of file 0 mappings: 4
 - Code(Counter(0)) at (prev + 16, 1) to (start + 1, 9)
 - Code(Zero) at (prev + 2, 9) to (start + 0, 15)
 - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 10)
-    = (c0 - c1)
+    = (c0 - Zero)
 - Code(Expression(1, Add)) at (prev + 2, 1) to (start + 0, 2)
-    = (c1 + (c0 - c1))
+    = (Zero + (c0 - Zero))
 
 Function name: inline_dead::main
 Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 01, 03, 0d, 01, 07, 06, 02, 02]
@@ -31,15 +31,15 @@ Number of file 0 mappings: 2
 - Code(Counter(0)) at (prev + 7, 6) to (start + 2, 2)
 
 Function name: inline_dead::main::{closure#0}
-Raw bytes (23): 0x[01, 01, 02, 09, 06, 01, 05, 03, 01, 07, 17, 00, 18, 00, 02, 0d, 00, 0e, 03, 02, 05, 00, 06]
+Raw bytes (23): 0x[01, 01, 02, 00, 06, 01, 00, 03, 01, 07, 17, 00, 18, 00, 02, 0d, 00, 0e, 03, 02, 05, 00, 06]
 Number of files: 1
 - file 0 => global file 1
 Number of expressions: 2
-- expression 0 operands: lhs = Counter(2), rhs = Expression(1, Sub)
-- expression 1 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 0 operands: lhs = Zero, rhs = Expression(1, Sub)
+- expression 1 operands: lhs = Counter(0), rhs = Zero
 Number of file 0 mappings: 3
 - Code(Counter(0)) at (prev + 7, 23) to (start + 0, 24)
 - Code(Zero) at (prev + 2, 13) to (start + 0, 14)
 - Code(Expression(0, Add)) at (prev + 2, 5) to (start + 0, 6)
-    = (c2 + (c0 - c1))
+    = (Zero + (c0 - Zero))