optimized replacement of counters with expressions plus new BCB graphviz

Author: Rich Kadel

compiler/rustc_codegen_ssa/src/coverageinfo/map.rs

@@ -38,6 +38,10 @@ pub struct FunctionCoverage {
 impl FunctionCoverage {
     pub fn new<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self {
         let coverageinfo = tcx.coverageinfo(instance.def_id());
+        debug!(
+            "FunctionCoverage::new(instance={:?}) has coverageinfo={:?}",
+            instance, coverageinfo
+        );
         Self {
             source_hash: 0, // will be set with the first `add_counter()`
             counters: IndexVec::from_elem_n(None, coverageinfo.num_counters as usize),
@@ -150,13 +154,41 @@ impl FunctionCoverage {
                     self.counters
                         .get(index)
                         .unwrap() // pre-validated
+                        // TODO(richkadel): is it really pre-validated?
+                        // What if I add some counters that never get added to the map, and they are
+                        // larger than the number of counters in the MIR (as seems to happen with expressions below?)
                         .as_ref()
                         .map(|_| Counter::counter_value_reference(index))
                 } else {
                     let index = self.expression_index(u32::from(id));
+                    // TODO(richkadel): remove this debug
+                    debug!(
+                        "id_to_counter expression id={:?}, self.expressions.get(index={:?}) = {:?}",
+                        id,
+                        index,
+                        self.expressions.get(index)
+                    );
                     self.expressions
                         .get(index)
+                        // TODO(richkadel): Now some tests generate segfault, and other tests hit this out of range error
+                        // Some expressions reference blocks that ended up not needing counters.
+                        // Can we assume the expression is no longer relevant? If not, then instrument_counters
+                        // transform pass will need to figure this out earlier (MAYBE IT SHOULD ANYWAY?)
+                        // and if the counter is needed for an expression that can no longer be resolved,
+                        // create a new make_counter() right there?
+                        //
+                        // MUST FIX!
+                        //
+                        // It looks like the segfault is at:
+                        //
+                        //   /usr/local/google/home/richkadel/rust/src/llvm-project/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:93
+                        //   AdjustedExpressionIDs[ID] = 1;
+                        //
+                        // I think we have expressions with operand IDs that don't exist as either counters or expressions, and that's breaking
+                        // the LLVM code.
+                        // TODO(richkadel): replace expect() with unwrap_or()?
                         .expect("expression id is out of range")
+                        //                        .unwrap_or(&None)
                         .as_ref()
                         .map(|_| Counter::expression(new_indexes[index]))
                 }
@@ -198,6 +230,12 @@ impl FunctionCoverage {
                 if let Some(region) = optional_region {
                     expression_regions.push((Counter::expression(mapped_expression_index), region));
                 }
+            } else {
+                debug!(
+                    "Ignoring expression with one or more missing operands: \
+                    original_index={:?}, lhs={:?}, op={:?}, rhs={:?}, region={:?}",
+                    original_index, lhs, op, rhs, optional_region,
+                )
             }
         }
         (counter_expressions, expression_regions.into_iter())

compiler/rustc_graphviz/src/lib.rs

@@ -643,6 +643,7 @@ where
     }
     if options.contains(&RenderOption::DarkTheme) {
         graph_attrs.push(r#"bgcolor="black""#);
+        graph_attrs.push(r#"fontcolor="white""#);
         content_attrs.push(r#"color="white""#);
         content_attrs.push(r#"fontcolor="white""#);
     }

compiler/rustc_middle/src/mir/coverage.rs

@@ -64,14 +64,28 @@ impl From<CounterValueReference> for ExpressionOperandId {
     }
 }
 
+impl From<&mut CounterValueReference> for ExpressionOperandId {
+    #[inline]
+    fn from(v: &mut CounterValueReference) -> ExpressionOperandId {
+        ExpressionOperandId::from(v.as_u32())
+    }
+}
+
 impl From<InjectedExpressionIndex> for ExpressionOperandId {
     #[inline]
     fn from(v: InjectedExpressionIndex) -> ExpressionOperandId {
         ExpressionOperandId::from(v.as_u32())
     }
 }
 
-#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
+impl From<&mut InjectedExpressionIndex> for ExpressionOperandId {
+    #[inline]
+    fn from(v: &mut InjectedExpressionIndex) -> ExpressionOperandId {
+        ExpressionOperandId::from(v.as_u32())
+    }
+}
+
+#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
 pub enum CoverageKind {
     Counter {
         function_source_hash: u64,
@@ -88,14 +102,51 @@ pub enum CoverageKind {
 
 impl CoverageKind {
     pub fn as_operand_id(&self) -> ExpressionOperandId {
+        use CoverageKind::*;
         match *self {
-            CoverageKind::Counter { id, .. } => ExpressionOperandId::from(id),
-            CoverageKind::Expression { id, .. } => ExpressionOperandId::from(id),
-            CoverageKind::Unreachable => {
+            Counter { id, .. } => ExpressionOperandId::from(id),
+            Expression { id, .. } => ExpressionOperandId::from(id),
+            Unreachable => {
                 bug!("Unreachable coverage cannot be part of an expression")
             }
         }
     }
+
+    pub fn is_counter(&self) -> bool {
+        match self {
+            Self::Counter { .. } => true,
+            _ => false,
+        }
+    }
+
+    pub fn is_expression(&self) -> bool {
+        match self {
+            Self::Expression { .. } => true,
+            _ => false,
+        }
+    }
+
+    pub fn is_unreachable(&self) -> bool {
+        *self == Self::Unreachable
+    }
+}
+
+impl Debug for CoverageKind {
+    fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
+        use CoverageKind::*;
+        match self {
+            Counter { id, .. } => write!(fmt, "Counter({:?})", id.index()),
+            Expression { id, lhs, op, rhs } => write!(
+                fmt,
+                "Expression({:?}) = {} {} {}",
+                id.index(),
+                lhs.index(),
+                if *op == Op::Add { "+" } else { "-" },
+                rhs.index(),
+            ),
+            Unreachable => write!(fmt, "Unreachable"),
+        }
+    }
 }
 
 #[derive(Clone, TyEncodable, TyDecodable, HashStable, TypeFoldable, PartialEq, Eq, PartialOrd, Ord)]

compiler/rustc_middle/src/mir/mod.rs

@@ -1601,21 +1601,10 @@ impl Debug for Statement<'_> {
                 write!(fmt, "AscribeUserType({:?}, {:?}, {:?})", place, variance, c_ty)
             }
             Coverage(box ref coverage) => {
-                let rgn = &coverage.code_region;
-                match coverage.kind {
-                    CoverageKind::Counter { id, .. } => {
-                        write!(fmt, "Coverage::Counter({:?}) for {:?}", id.index(), rgn)
-                    }
-                    CoverageKind::Expression { id, lhs, op, rhs } => write!(
-                        fmt,
-                        "Coverage::Expression({:?}) = {} {} {} for {:?}",
-                        id.index(),
-                        lhs.index(),
-                        if op == coverage::Op::Add { "+" } else { "-" },
-                        rhs.index(),
-                        rgn
-                    ),
-                    CoverageKind::Unreachable => write!(fmt, "Coverage::Unreachable for {:?}", rgn),
+                if let Some(rgn) = &coverage.code_region {
+                    write!(fmt, "Coverage::{:?} for {:?}", coverage.kind, rgn)
+                } else {
+                    write!(fmt, "Coverage::{:?}", coverage.kind)
                 }
             }
             Nop => write!(fmt, "nop"),