coverage: Make expression simplification non-destructive

Author: Zalathar

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

@@ -27,6 +27,9 @@ pub struct FunctionCoverage<'tcx> {
     /// that might be introduced by MIR inlining.
     counters_seen: BitSet<CounterId>,
     expressions: IndexVec<ExpressionId, Option<Expression>>,
+    /// Tracks which expressions are known to always have a value of zero.
+    /// Only updated during the finalize step.
+    zero_expressions: FxIndexSet<ExpressionId>,
     mappings: Vec<Mapping>,
 }
 
@@ -63,6 +66,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
             is_used,
             counters_seen: BitSet::new_empty(num_counters),
             expressions: IndexVec::from_elem_n(None, num_expressions),
+            zero_expressions: FxIndexSet::default(),
             mappings: Vec::new(),
         }
     }
@@ -134,39 +138,44 @@ impl<'tcx> FunctionCoverage<'tcx> {
     }
 
     pub(crate) fn finalize(&mut self) {
-        self.simplify_expressions();
+        self.update_zero_expressions();
 
         // Sort all of the collected mappings into a predictable order.
         // (Mappings have a total order, so an unstable sort should be fine.)
         self.mappings.sort_unstable();
     }
 
-    /// Perform some simplifications to make the final coverage mappings
-    /// slightly smaller.
+    /// Identify expressions that will always have a value of zero, and note
+    /// their IDs in `zero_expressions`. Mappings that refer to a zero expression
+    /// can instead become mappings to a constant zero value.
     ///
     /// This method mainly exists to preserve the simplifications that were
     /// already being performed by the Rust-side expression renumbering, so that
     /// the resulting coverage mappings don't get worse.
-    fn simplify_expressions(&mut self) {
+    fn update_zero_expressions(&mut self) {
         // The set of expressions that either were optimized out entirely, or
         // 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 zero_expressions = &mut self.zero_expressions;
 
-        // For each expression, perform simplifications based on lower-numbered
-        // expressions, and then update the set of always-zero expressions if
-        // necessary.
+        // Simplify a copy of each expression based on lower-numbered expressions,
+        // and then update the set of always-zero expressions if necessary.
         // (By construction, expressions can only refer to other expressions
-        // that have lower IDs, so one simplification pass is sufficient.)
-        for (id, maybe_expression) in self.expressions.iter_enumerated_mut() {
+        // that have lower IDs, so one pass is sufficient.)
+        for (id, maybe_expression) in self.expressions.iter_enumerated() {
             let Some(expression) = maybe_expression else {
                 // If an expression is missing, it must have been optimized away,
                 // so any operand that refers to it can be replaced with zero.
                 zero_expressions.insert(id);
                 continue;
             };
 
+            // We don't need to simplify the actual expression data in the
+            // expressions list; we can just simplify a temporary copy and then
+            // use that to update the set of always-zero expressions.
+            let mut expression = expression.clone();
+
             // 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 {
@@ -207,6 +216,11 @@ 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.expressions
             .iter()
             .map(|expression| match expression {
@@ -220,12 +234,12 @@ impl<'tcx> FunctionCoverage<'tcx> {
                 &Some(Expression { lhs, op, rhs, .. }) => {
                     // Convert the operands and operator as normal.
                     CounterExpression::new(
-                        Counter::from_term(lhs),
+                        counter_from_operand(lhs),
                         match op {
                             Op::Add => ExprKind::Add,
                             Op::Subtract => ExprKind::Subtract,
                         },
-                        Counter::from_term(rhs),
+                        counter_from_operand(rhs),
                     )
                 }
             })