Auto merge of #83307 - richkadel:cov-unused-functions-1.1, r=tmandry

coverage bug fixes and optimization support

Adjusted LLVM codegen for code compiled with `-Zinstrument-coverage` to
address multiple, somewhat related issues.

Fixed a significant flaw in prior coverage solution: Every counter
generated a new counter variable, but there should have only been one
counter variable per function. This appears to have bloated .profraw
files significantly. (For a small program, it increased the size by
about 40%. I have not tested large programs, but there is anecdotal
evidence that profraw files were way too large. This is a good fix,
regardless, but hopefully it also addresses related issues.

Fixes: #82144

Invalid LLVM coverage data produced when compiled with -C opt-level=1

Existing tests now work up to at least `opt-level=3`. This required a
detailed analysis of the LLVM IR, comparisons with Clang C++ LLVM IR
when compiled with coverage, and a lot of trial and error with codegen
adjustments.

The biggest hurdle was figuring out how to continue to support coverage
results for unused functions and generics. Rust's coverage results have
three advantages over Clang's coverage results:

1. Rust's coverage map does not include any overlapping code regions,
   making coverage counting unambiguous.
2. Rust generates coverage results (showing zero counts) for all unused
   functions, including generics. (Clang does not generate coverage for
   uninstantiated template functions.)
3. Rust's unused functions produce minimal stubbed functions in LLVM IR,
   sufficient for including in the coverage results; while Clang must
   generate the complete LLVM IR for each unused function, even though
   it will never be called.

This PR removes the previous hack of attempting to inject coverage into
some other existing function instance, and generates dedicated instances
for each unused function. This change, and a few other adjustments
(similar to what is required for `-C link-dead-code`, but with lower
impact), makes it possible to support LLVM optimizations.

Fixes: #79651

Coverage report: "Unexecuted instantiation:..." for a generic function
from multiple crates

Fixed by removing the aforementioned hack. Some "Unexecuted
instantiation" notices are unavoidable, as explained in the
`used_crate.rs` test, but `-Zinstrument-coverage` has new options to
back off support for either unused generics, or all unused functions,
which avoids the notice, at the cost of less coverage of unused
functions.

Fixes: #82875

Invalid LLVM coverage data produced with crate brotli_decompressor

Fixed by disabling the LLVM function attribute that forces inlining, if
`-Z instrument-coverage` is enabled. This attribute is applied to
Rust functions with `#[inline(always)], and in some cases, the forced
inlining breaks coverage instrumentation and reports.

FYI: `@wesleywiser`

r? `@tmandry`

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -1746,7 +1746,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
     );
 
     // OBJECT-FILES-NO, AUDIT-ORDER
-    if sess.opts.cg.profile_generate.enabled() || sess.opts.debugging_opts.instrument_coverage {
+    if sess.opts.cg.profile_generate.enabled() || sess.instrument_coverage() {
         cmd.pgo_gen();
     }
 

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -188,9 +188,7 @@ fn exported_symbols_provider_local(
         }
     }
 
-    if tcx.sess.opts.debugging_opts.instrument_coverage
-        || tcx.sess.opts.cg.profile_generate.enabled()
-    {
+    if tcx.sess.instrument_coverage() || tcx.sess.opts.cg.profile_generate.enabled() {
         // These are weak symbols that point to the profile version and the
         // profile name, which need to be treated as exported so LTO doesn't nix
         // them.

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -176,7 +176,7 @@ impl ModuleConfig {
 
                     // The rustc option `-Zinstrument_coverage` injects intrinsic calls to
                     // `llvm.instrprof.increment()`, which requires the LLVM `instrprof` pass.
-                    if sess.opts.debugging_opts.instrument_coverage {
+                    if sess.instrument_coverage() {
                         passes.push("instrprof".to_owned());
                     }
                     passes

compiler/rustc_codegen_ssa/src/coverageinfo/map.rs

@@ -31,27 +31,44 @@ pub struct Expression {
 pub struct FunctionCoverage<'tcx> {
     instance: Instance<'tcx>,
     source_hash: u64,
+    is_used: bool,
     counters: IndexVec<CounterValueReference, Option<CodeRegion>>,
     expressions: IndexVec<InjectedExpressionIndex, Option<Expression>>,
     unreachable_regions: Vec<CodeRegion>,
 }
 
 impl<'tcx> FunctionCoverage<'tcx> {
+    /// Creates a new set of coverage data for a used (called) function.
     pub fn new(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self {
+        Self::create(tcx, instance, true)
+    }
+
+    /// Creates a new set of coverage data for an unused (never called) function.
+    pub fn unused(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self {
+        Self::create(tcx, instance, false)
+    }
+
+    fn create(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, is_used: bool) -> Self {
         let coverageinfo = tcx.coverageinfo(instance.def_id());
         debug!(
-            "FunctionCoverage::new(instance={:?}) has coverageinfo={:?}",
-            instance, coverageinfo
+            "FunctionCoverage::new(instance={:?}) has coverageinfo={:?}. is_used={}",
+            instance, coverageinfo, is_used
         );
         Self {
             instance,
             source_hash: 0, // will be set with the first `add_counter()`
+            is_used,
             counters: IndexVec::from_elem_n(None, coverageinfo.num_counters as usize),
             expressions: IndexVec::from_elem_n(None, coverageinfo.num_expressions as usize),
             unreachable_regions: Vec::new(),
         }
     }
 
+    /// Returns true for a used (called) function, and false for an unused function.
+    pub fn is_used(&self) -> bool {
+        self.is_used
+    }
+
     /// Sets the function source hash value. If called multiple times for the same function, all
     /// calls should have the same hash value.
     pub fn set_function_source_hash(&mut self, source_hash: u64) {
@@ -128,8 +145,8 @@ impl<'tcx> FunctionCoverage<'tcx> {
         &'a self,
     ) -> (Vec<CounterExpression>, impl Iterator<Item = (Counter, &'a CodeRegion)>) {
         assert!(
-            self.source_hash != 0,
-            "No counters provided the source_hash for function: {:?}",
+            self.source_hash != 0 || !self.is_used,
+            "No counters provided the source_hash for used function: {:?}",
             self.instance
         );
 

compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs

@@ -33,7 +33,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
                     let coverageinfo = bx.tcx().coverageinfo(instance.def_id());
 
-                    let fn_name = bx.create_pgo_func_name_var(instance);
+                    let fn_name = bx.get_pgo_func_name_var(instance);
                     let hash = bx.const_u64(function_source_hash);
                     let num_counters = bx.const_u32(coverageinfo.num_counters);
                     let index = bx.const_u32(u32::from(id));

compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs

@@ -1,14 +1,26 @@
 use super::BackendTypes;
+use rustc_hir::def_id::DefId;
 use rustc_middle::mir::coverage::*;
 use rustc_middle::ty::Instance;
 
-pub trait CoverageInfoMethods: BackendTypes {
+pub trait CoverageInfoMethods<'tcx>: BackendTypes {
     fn coverageinfo_finalize(&self);
+
+    /// Codegen a small function that will never be called, with one counter
+    /// that will never be incremented, that gives LLVM coverage tools a
+    /// function definition it needs in order to resolve coverage map references
+    /// to unused functions. This is necessary so unused functions will appear
+    /// as uncovered (coverage execution count `0`) in LLVM coverage reports.
+    fn define_unused_fn(&self, def_id: DefId);
+
+    /// For LLVM codegen, returns a function-specific `Value` for a global
+    /// string, to hold the function name passed to LLVM intrinsic
+    /// `instrprof.increment()`. The `Value` is only created once per instance.
+    /// Multiple invocations with the same instance return the same `Value`.
+    fn get_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value;
 }
 
 pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
-    fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value;
-
     /// Returns true if the function source hash was added to the coverage map (even if it had
     /// already been added, for this instance). Returns false *only* if `-Z instrument-coverage` is
     /// not enabled (a coverage map is not being generated).

compiler/rustc_codegen_ssa/src/traits/mod.rs

@@ -58,7 +58,7 @@ pub trait CodegenMethods<'tcx>:
     + MiscMethods<'tcx>
     + ConstMethods<'tcx>
     + StaticMethods
-    + CoverageInfoMethods
+    + CoverageInfoMethods<'tcx>
     + DebugInfoMethods<'tcx>
     + AsmMethods
     + PreDefineMethods<'tcx>
@@ -74,7 +74,7 @@ impl<'tcx, T> CodegenMethods<'tcx> for T where
         + MiscMethods<'tcx>
         + ConstMethods<'tcx>
         + StaticMethods
-        + CoverageInfoMethods
+        + CoverageInfoMethods<'tcx>
         + DebugInfoMethods<'tcx>
         + AsmMethods
         + PreDefineMethods<'tcx>