Auto merge of rust-lang#78267 - richkadel:llvm-coverage-counters-2.0.3r1, r=tmandry

Working expression optimization, and some improvements to branch-level source coverage

This replaces PR rust-lang#78040 after reorganizing the original commits (by request) into a more logical sequence of major changes.

Most of the work is in the MIR `transform/coverage/` directory (originally, `transform/instrument_coverage.rs`).

Note this PR includes some significant additional debugging capabilities, to help myself and any future developer working on coverage improvements or issues.

In particular, there's a new Graphviz (.dot file) output for the coverage graph (the `BasicCoverageBlock` control flow graph) that provides ways to get some very good insight into the relationships between the MIR, the coverage graph BCBs, coverage spans, and counters. (There are also some cool debugging options, available via environment variable, to alter how some data in the graph appears.)

And the code for this Graphviz view is actually generic... it can be used by any implementation of the Rust `Graph` traits.

Finally (for now), I also now output information from `llvm-cov` that shows the actual counters and spans it found in the coverage map, and their counts (from the `--debug` flag). I found this to be enormously helpful in debugging some coverage issues, so I kept it in the test results as well for additional context.

`@tmandry` `@wesleywiser`

r? `@tmandry`

Here's an example of the new coverage graph:

* Within each `BasicCoverageBlock` (BCB), you can see each `CoverageSpan` and its contributing statements (MIR `Statement`s and/or `Terminator`s)
* Each `CoverageSpan` has a `Counter` or and `Expression`, and `Expression`s show their Add/Subtract operation with nested operations. (This can be changed to show the Counter and Expression IDs instead, or in addition to, the BCB.)
* The terminators of all MIR `BasicBlock`s in the BCB, including one final `Terminator`
* If an "edge counter" is required (because we need to count an edge between blocks, in some cases) the edge's Counter or Expression is shown next to its label. (Not shown in the example below.) (FYI, Edge Counters are converted into a new MIR `BasicBlock` with `Goto`)

<img width="1116" alt="Screen Shot 2020-10-17 at 12 23 29 AM" src="https://user-images.githubusercontent.com/3827298/96331095-616cb480-100f-11eb-8212-60f2d433e2d8.png">

r? `@tmandry`
FYI: `@wesleywiser`

Author: bors

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

@@ -129,7 +129,7 @@ impl CoverageMapGenerator {
                 let (filenames_index, _) = self.filenames.insert_full(c_filename);
                 virtual_file_mapping.push(filenames_index as u32);
             }
-            debug!("Adding counter {:?} to map for {:?}", counter, region,);
+            debug!("Adding counter {:?} to map for {:?}", counter, region);
             mapping_regions.push(CounterMappingRegion::code_region(
                 counter,
                 current_file_id,

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -12,7 +12,7 @@ use rustc_codegen_ssa::traits::{
 use rustc_data_structures::fx::FxHashMap;
 use rustc_llvm::RustString;
 use rustc_middle::mir::coverage::{
-    CodeRegion, CounterValueReference, ExpressionOperandId, InjectedExpressionIndex, Op,
+    CodeRegion, CounterValueReference, ExpressionOperandId, InjectedExpressionId, Op,
 };
 use rustc_middle::ty::Instance;
 
@@ -27,16 +27,16 @@ 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.
-    pub(crate) function_coverage_map: RefCell<FxHashMap<Instance<'tcx>, FunctionCoverage>>,
+    // Coverage data for each instrumented function identified by DefId.
+    pub(crate) function_coverage_map: RefCell<FxHashMap<Instance<'tcx>, FunctionCoverage<'tcx>>>,
 }
 
 impl<'tcx> CrateCoverageContext<'tcx> {
     pub fn new() -> Self {
         Self { function_coverage_map: Default::default() }
     }
 
-    pub fn take_function_coverage_map(&self) -> FxHashMap<Instance<'tcx>, FunctionCoverage> {
+    pub fn take_function_coverage_map(&self) -> FxHashMap<Instance<'tcx>, FunctionCoverage<'tcx>> {
         self.function_coverage_map.replace(FxHashMap::default())
     }
 }
@@ -58,47 +58,66 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         unsafe { llvm::LLVMRustCoverageCreatePGOFuncNameVar(llfn, mangled_fn_name.as_ptr()) }
     }
 
-    fn add_counter_region(
+    fn set_function_source_hash(
         &mut self,
         instance: Instance<'tcx>,
         function_source_hash: u64,
+    ) -> bool {
+        if let Some(coverage_context) = self.coverage_context() {
+            debug!(
+                "ensuring function source hash is set for instance={:?}; function_source_hash={}",
+                instance, function_source_hash,
+            );
+            let mut coverage_map = coverage_context.function_coverage_map.borrow_mut();
+            coverage_map
+                .entry(instance)
+                .or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
+                .set_function_source_hash(function_source_hash);
+            true
+        } else {
+            false
+        }
+    }
+
+    fn add_coverage_counter(
+        &mut self,
+        instance: Instance<'tcx>,
         id: CounterValueReference,
         region: CodeRegion,
     ) -> bool {
         if let Some(coverage_context) = self.coverage_context() {
             debug!(
-                "adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={:?}, \
-                at {:?}",
-                instance, function_source_hash, id, region,
+                "adding counter to coverage_map: instance={:?}, id={:?}, region={:?}",
+                instance, id, region,
             );
-            let mut coverage_regions = coverage_context.function_coverage_map.borrow_mut();
-            coverage_regions
+            let mut coverage_map = 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);
+                .add_counter(id, region);
             true
         } else {
             false
         }
     }
 
-    fn add_counter_expression_region(
+    fn add_coverage_counter_expression(
         &mut self,
         instance: Instance<'tcx>,
-        id: InjectedExpressionIndex,
+        id: InjectedExpressionId,
         lhs: ExpressionOperandId,
         op: Op,
         rhs: ExpressionOperandId,
-        region: CodeRegion,
+        region: Option<CodeRegion>,
     ) -> bool {
         if let Some(coverage_context) = self.coverage_context() {
             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 = coverage_context.function_coverage_map.borrow_mut();
-            coverage_regions
+            let mut coverage_map = 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);
@@ -108,14 +127,14 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         }
     }
 
-    fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool {
+    fn add_coverage_unreachable(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool {
         if let Some(coverage_context) = self.coverage_context() {
             debug!(
-                "adding unreachable code to coverage_regions: instance={:?}, at {:?}",
+                "adding unreachable code to coverage_map: instance={:?}, at {:?}",
                 instance, region,
             );
-            let mut coverage_regions = coverage_context.function_coverage_map.borrow_mut();
-            coverage_regions
+            let mut coverage_map = 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/ffi.rs

@@ -3,7 +3,7 @@ use rustc_middle::mir::coverage::{CounterValueReference, MappedExpressionIndex};
 /// Aligns with [llvm::coverage::Counter::CounterKind](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L91)
 #[derive(Copy, Clone, Debug)]
 #[repr(C)]
-enum CounterKind {
+pub enum CounterKind {
     Zero = 0,
     CounterValueReference = 1,
     Expression = 2,
@@ -23,8 +23,8 @@ enum CounterKind {
 #[repr(C)]
 pub struct Counter {
     // Important: The layout (order and types of fields) must match its C++ counterpart.
-    kind: CounterKind,
-    id: u32,
+    pub kind: CounterKind,
+    pub id: u32,
 }
 
 impl Counter {
@@ -55,9 +55,9 @@ pub enum ExprKind {
 #[derive(Copy, Clone, Debug)]
 #[repr(C)]
 pub struct CounterExpression {
-    kind: ExprKind,
-    lhs: Counter,
-    rhs: Counter,
+    pub kind: ExprKind,
+    pub lhs: Counter,
+    pub rhs: Counter,
 }
 
 impl CounterExpression {