about summary refs log tree commit diff
path: root/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-10-03 18:36:21 +0000
committerbors <bors@rust-lang.org>2023-10-03 18:36:21 +0000
commit36aab8df0ab8a76f1c4f95ce8becefdd8a6fe526 (patch)
treea3e01fc97d6ef483471e82a089ee2cd9a2c30185 /compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
parent268d6250299b4f08f8e669a2889fdf71388d1f39 (diff)
parent053c4f94a098be5fbb80ca881b01cc8e9c64a8a5 (diff)
downloadrust-36aab8df0ab8a76f1c4f95ce8becefdd8a6fe526.tar.gz
rust-36aab8df0ab8a76f1c4f95ce8becefdd8a6fe526.zip
Auto merge of #115301 - Zalathar:regions-vec, r=davidtwco
coverage: Allow each coverage statement to have multiple code regions

The original implementation of coverage instrumentation was built around the assumption that a coverage counter/expression would be associated with *up to one* code region. When it was discovered that *multiple* regions would sometimes need to share a counter, a workaround was found: for the remaining regions, the instrumentor would create a fresh expression that adds zero  to the existing counter/expression.

That got the job done, but resulted in some awkward code, and produces unnecessarily complicated coverage maps in the final binary.

---

This PR removes that tension by changing `StatementKind::Coverage`'s code region field from `Option<CodeRegion>` to `Vec<CodeRegion>`.

The changes on the codegen side are fairly straightforward. As long as each `CoverageKind::Counter` only injects one `llvm.instrprof.increment`, the rest of coverage codegen is happy to handle multiple regions mapped to the same counter/expression, with only minor option-to-vec adjustments.

On the instrumentor/mir-transform side, we can get rid of the code that creates extra (x + 0) expressions. Instead we gather all of the code regions associated with a single BCB, and inject them all into one coverage statement.

---

There are several patches here but they can be divided in to three phases:
- Preparatory work
- Actually switching over to multiple regions per coverage statement
- Cleaning up

So viewing the patches individually may be easier.
Diffstat (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs')
-rw-r--r--compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs45
1 files changed, 9 insertions, 36 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
index c70cb670e96..dd2ce9b525b 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
@@ -89,9 +89,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
     /// `function_coverage_map` (keyed by function `Instance`) during codegen.
     /// But in this case, since the unused function was _not_ previously
     /// codegenned, collect the coverage `CodeRegion`s from the MIR and add
-    /// them. The first `CodeRegion` is used to add a single counter, with the
-    /// same counter ID used in the injected `instrprof.increment` intrinsic
-    /// call. Since the function is never called, all other `CodeRegion`s can be
+    /// them. Since the function is never called, all of its `CodeRegion`s can be
     /// added as `unreachable_region`s.
     fn define_unused_fn(&self, def_id: DefId) {
         let instance = declare_unused_fn(self, def_id);
@@ -110,25 +108,15 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
             .entry(instance)
             .or_insert_with(|| FunctionCoverage::new(bx.tcx(), instance));
 
-        let Coverage { kind, code_region } = coverage.clone();
-        match kind {
+        let Coverage { kind, code_regions } = coverage;
+        match *kind {
             CoverageKind::Counter { function_source_hash, id } => {
                 debug!(
                     "ensuring function source hash is set for instance={:?}; function_source_hash={}",
                     instance, function_source_hash,
                 );
                 func_coverage.set_function_source_hash(function_source_hash);
-
-                if let Some(code_region) = code_region {
-                    // Note: Some counters do not have code regions, but may still be referenced
-                    // from expressions. In that case, don't add the counter to the coverage map,
-                    // but do inject the counter intrinsic.
-                    debug!(
-                        "adding counter to coverage_map: instance={:?}, id={:?}, region={:?}",
-                        instance, id, code_region,
-                    );
-                    func_coverage.add_counter(id, code_region);
-                }
+                func_coverage.add_counter(id, code_regions);
                 // We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,
                 // as that needs an exclusive borrow.
                 drop(coverage_map);
@@ -146,20 +134,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
                 bx.instrprof_increment(fn_name, hash, num_counters, index);
             }
             CoverageKind::Expression { id, lhs, op, rhs } => {
-                debug!(
-                    "adding counter expression to coverage_map: instance={:?}, id={:?}, {:?} {:?} {:?}; region: {:?}",
-                    instance, id, lhs, op, rhs, code_region,
-                );
-                func_coverage.add_counter_expression(id, lhs, op, rhs, code_region);
+                func_coverage.add_counter_expression(id, lhs, op, rhs, code_regions);
             }
             CoverageKind::Unreachable => {
-                let code_region =
-                    code_region.expect("unreachable regions always have code regions");
-                debug!(
-                    "adding unreachable code to coverage_map: instance={:?}, at {:?}",
-                    instance, code_region,
-                );
-                func_coverage.add_unreachable_region(code_region);
+                func_coverage.add_unreachable_regions(code_regions);
             }
         }
     }
@@ -227,14 +205,9 @@ fn add_unused_function_coverage<'tcx>(
     let tcx = cx.tcx;
 
     let mut function_coverage = FunctionCoverage::unused(tcx, instance);
-    for (index, &code_region) in tcx.covered_code_regions(def_id).iter().enumerate() {
-        if index == 0 {
-            // Insert at least one real counter so the LLVM CoverageMappingReader will find expected
-            // definitions.
-            function_coverage.add_counter(UNUSED_FUNCTION_COUNTER_ID, code_region.clone());
-        } else {
-            function_coverage.add_unreachable_region(code_region.clone());
-        }
+    for &code_region in tcx.covered_code_regions(def_id) {
+        let code_region = std::slice::from_ref(code_region);
+        function_coverage.add_unreachable_regions(code_region);
     }
 
     if let Some(coverage_context) = cx.coverage_context() {