Expression optimization is nearly working

I need to change how I determine if a target from a SwitchInt exits the
loop or not, but this almost worked (and did successfully compile and
create the right coverage). It just didn't select the right targets for
the optimized expressions.

Author: richkadel

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -27,7 +27,7 @@ const COVMAP_VAR_ALIGN_BYTES: usize = 8;
 
 /// A context object for maintaining all state needed by the coverageinfo module.
 pub struct CrateCoverageContext<'tcx> {
-    // Coverage region data for each instrumented function identified by DefId.
+    // Coverage data for each instrumented function identified by DefId.
     pub(crate) function_coverage_map: RefCell<FxHashMap<Instance<'tcx>, FunctionCoverage>>,
 }
 
@@ -58,53 +58,50 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         unsafe { llvm::LLVMRustCoverageCreatePGOFuncNameVar(llfn, mangled_fn_name.as_ptr()) }
     }
 
-    fn add_counter_region(
+    fn add_coverage_counter(
         &mut self,
         instance: Instance<'tcx>,
         function_source_hash: u64,
         id: CounterValueReference,
         region: CodeRegion,
     ) {
         debug!(
-            "adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={:?}, \
+            "adding counter to coverage_map: instance={:?}, function_source_hash={}, id={:?}, \
              at {:?}",
             instance, function_source_hash, id, region,
         );
-        let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
-        coverage_regions
+        let mut coverage_map = self.coverage_context().function_coverage_map.borrow_mut();
+        coverage_map
             .entry(instance)
             .or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
             .add_counter(function_source_hash, id, region);
     }
 
-    fn add_counter_expression_region(
+    fn add_coverage_counter_expression(
         &mut self,
         instance: Instance<'tcx>,
         id: InjectedExpressionIndex,
         lhs: ExpressionOperandId,
         op: Op,
         rhs: ExpressionOperandId,
-        region: CodeRegion,
+        region: Option<CodeRegion>,
     ) {
         debug!(
-            "adding counter expression to coverage_regions: instance={:?}, id={:?}, {:?} {:?} {:?}, \
-             at {:?}",
+            "adding counter expression to coverage_map: instance={:?}, id={:?}, {:?} {:?} {:?}; \
+             region: {:?}",
             instance, id, lhs, op, rhs, region,
         );
-        let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
-        coverage_regions
+        let mut coverage_map = self.coverage_context().function_coverage_map.borrow_mut();
+        coverage_map
             .entry(instance)
             .or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
             .add_counter_expression(id, lhs, op, rhs, region);
     }
 
-    fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion) {
-        debug!(
-            "adding unreachable code to coverage_regions: instance={:?}, at {:?}",
-            instance, region,
-        );
-        let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
-        coverage_regions
+    fn add_coverage_unreachable(&mut self, instance: Instance<'tcx>, region: CodeRegion) {
+        debug!("adding unreachable code to coverage_map: instance={:?}, at {:?}", instance, region,);
+        let mut coverage_map = self.coverage_context().function_coverage_map.borrow_mut();
+        coverage_map
             .entry(instance)
             .or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
             .add_unreachable_region(region);

compiler/rustc_codegen_ssa/src/coverageinfo/map.rs

@@ -9,11 +9,11 @@ use rustc_middle::ty::Instance;
 use rustc_middle::ty::TyCtxt;
 
 #[derive(Clone, Debug)]
-pub struct ExpressionRegion {
+pub struct Expression {
     lhs: ExpressionOperandId,
     op: Op,
     rhs: ExpressionOperandId,
-    region: CodeRegion,
+    region: Option<CodeRegion>,
 }
 
 /// Collects all of the coverage regions associated with (a) injected counters, (b) counter
@@ -31,7 +31,7 @@ pub struct ExpressionRegion {
 pub struct FunctionCoverage {
     source_hash: u64,
     counters: IndexVec<CounterValueReference, Option<CodeRegion>>,
-    expressions: IndexVec<InjectedExpressionIndex, Option<ExpressionRegion>>,
+    expressions: IndexVec<InjectedExpressionIndex, Option<Expression>>,
     unreachable_regions: Vec<CodeRegion>,
 }
 
@@ -78,11 +78,11 @@ impl FunctionCoverage {
         lhs: ExpressionOperandId,
         op: Op,
         rhs: ExpressionOperandId,
-        region: CodeRegion,
+        region: Option<CodeRegion>,
     ) {
         let expression_index = self.expression_index(u32::from(expression_id));
         self.expressions[expression_index]
-            .replace(ExpressionRegion { lhs, op, rhs, region })
+            .replace(Expression { lhs, op, rhs, region })
             .expect_none("add_counter_expression called with duplicate `id_descending_from_max`");
     }
 
@@ -135,9 +135,9 @@ impl FunctionCoverage {
         // will be set, and after that, `new_indexes` will only be accessed using those same
         // indexes.
 
-        // Note that an `ExpressionRegion`s at any given index can include other expressions as
+        // Note that an `Expression`s at any given index can include other expressions as
         // operands, but expression operands can only come from the subset of expressions having
-        // `expression_index`s lower than the referencing `ExpressionRegion`. Therefore, it is
+        // `expression_index`s lower than the referencing `Expression`. Therefore, it is
         // reasonable to look up the new index of an expression operand while the `new_indexes`
         // vector is only complete up to the current `ExpressionIndex`.
         let id_to_counter =
@@ -162,15 +162,15 @@ impl FunctionCoverage {
                 }
             };
 
-        for (original_index, expression_region) in
+        for (original_index, expression) in
             self.expressions.iter_enumerated().filter_map(|(original_index, entry)| {
                 // Option::map() will return None to filter out missing expressions. This may happen
                 // if, for example, a MIR-instrumented expression is removed during an optimization.
-                entry.as_ref().map(|region| (original_index, region))
+                entry.as_ref().map(|expression| (original_index, expression))
             })
         {
-            let region = &expression_region.region;
-            let ExpressionRegion { lhs, op, rhs, .. } = *expression_region;
+            let optional_region = &expression.region;
+            let Expression { lhs, op, rhs, .. } = *expression;
 
             if let Some(Some((lhs_counter, rhs_counter))) =
                 id_to_counter(&new_indexes, lhs).map(|lhs_counter| {
@@ -190,12 +190,14 @@ impl FunctionCoverage {
                     rhs_counter,
                 );
                 debug!(
-                    "Adding expression {:?} = {:?} at {:?}",
-                    mapped_expression_index, expression, region
+                    "Adding expression {:?} = {:?}, region: {:?}",
+                    mapped_expression_index, expression, optional_region
                 );
                 counter_expressions.push(expression);
                 new_indexes[original_index] = mapped_expression_index;
-                expression_regions.push((Counter::expression(mapped_expression_index), region));
+                if let Some(region) = optional_region {
+                    expression_regions.push((Counter::expression(mapped_expression_index), region));
+                }
             }
         }
         (counter_expressions, expression_regions.into_iter())

compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs

@@ -10,7 +10,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         let Coverage { kind, code_region } = coverage;
         match kind {
             CoverageKind::Counter { function_source_hash, id } => {
-                bx.add_counter_region(self.instance, function_source_hash, id, code_region);
+                bx.add_coverage_counter(
+                    self.instance,
+                    function_source_hash,
+                    id,
+                    code_region.expect("counters always have code regions"),
+                );
 
                 let coverageinfo = bx.tcx().coverageinfo(self.instance.def_id());
 
@@ -25,10 +30,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                 bx.instrprof_increment(fn_name, hash, num_counters, id);
             }
             CoverageKind::Expression { id, lhs, op, rhs } => {
-                bx.add_counter_expression_region(self.instance, id, lhs, op, rhs, code_region);
+                bx.add_coverage_counter_expression(self.instance, id, lhs, op, rhs, code_region);
             }
             CoverageKind::Unreachable => {
-                bx.add_unreachable_region(self.instance, code_region);
+                bx.add_coverage_unreachable(
+                    self.instance,
+                    code_region.expect("unreachable regions always have code regions"),
+                );
             }
         }
     }

compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs

@@ -9,23 +9,23 @@ pub trait CoverageInfoMethods: BackendTypes {
 pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
     fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value;
 
-    fn add_counter_region(
+    fn add_coverage_counter(
         &mut self,
         instance: Instance<'tcx>,
         function_source_hash: u64,
         id: CounterValueReference,
         region: CodeRegion,
     );
 
-    fn add_counter_expression_region(
+    fn add_coverage_counter_expression(
         &mut self,
         instance: Instance<'tcx>,
         id: InjectedExpressionIndex,
         lhs: ExpressionOperandId,
         op: Op,
         rhs: ExpressionOperandId,
-        region: CodeRegion,
+        region: Option<CodeRegion>,
     );
 
-    fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion);
+    fn add_coverage_unreachable(&mut self, instance: Instance<'tcx>, region: CodeRegion);
 }

compiler/rustc_middle/src/mir/mod.rs

@@ -1626,7 +1626,7 @@ impl Debug for Statement<'_> {
 #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
 pub struct Coverage {
     pub kind: CoverageKind,
-    pub code_region: CodeRegion,
+    pub code_region: Option<CodeRegion>,
 }
 
 ///////////////////////////////////////////////////////////////////////////