Auto merge of #75563 - richkadel:llvm-coverage-map-gen-5.4, r=wesleywiser

Moved coverage counter injection from BasicBlock to Statement.

As discussed on Zulip: https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Implement.20LLVM-compatible.20source-based.20cod.20compiler-team.23278

Author: bors

library/core/src/intrinsics.rs

@@ -1949,70 +1949,6 @@ extern "rust-intrinsic" {
     #[rustc_const_unstable(feature = "const_ptr_offset_from", issue = "41079")]
     pub fn ptr_offset_from<T>(ptr: *const T, base: *const T) -> isize;
 
-    /// Internal placeholder for injecting code coverage counters when the "instrument-coverage"
-    /// option is enabled. The source code region information is extracted prior to code generation,
-    /// and added to the "coverage map", which is injected into the generated code as additional
-    /// data. This intrinsic then triggers the generation of LLVM intrinsic call
-    /// `instrprof.increment`, using the remaining args (`function_source_hash` and `index`).
-    #[cfg(not(bootstrap))]
-    #[lang = "count_code_region"]
-    pub fn count_code_region(
-        function_source_hash: u64,
-        index: u32,
-        file_name: &'static str,
-        start_line: u32,
-        start_col: u32,
-        end_line: u32,
-        end_col: u32,
-    );
-
-    /// Internal marker for code coverage expressions, injected into the MIR when the
-    /// "instrument-coverage" option is enabled. This intrinsic is not converted into a
-    /// backend intrinsic call, but its arguments are extracted during the production of a
-    /// "coverage map", which is injected into the generated code, as additional data.
-    /// This marker identifies a code region and two other counters or counter expressions
-    /// whose sum is the number of times the code region was executed.
-    #[cfg(not(bootstrap))]
-    #[lang = "coverage_counter_add"]
-    pub fn coverage_counter_add(
-        index: u32,
-        left_index: u32,
-        right_index: u32,
-        file_name: &'static str,
-        start_line: u32,
-        start_col: u32,
-        end_line: u32,
-        end_col: u32,
-    );
-
-    /// This marker identifies a code region and two other counters or counter expressions
-    /// whose difference is the number of times the code region was executed.
-    /// (See `coverage_counter_add` for more information.)
-    #[cfg(not(bootstrap))]
-    #[lang = "coverage_counter_subtract"]
-    pub fn coverage_counter_subtract(
-        index: u32,
-        left_index: u32,
-        right_index: u32,
-        file_name: &'static str,
-        start_line: u32,
-        start_col: u32,
-        end_line: u32,
-        end_col: u32,
-    );
-
-    /// This marker identifies a code region to be added to the "coverage map" to indicate source
-    /// code that can never be reached.
-    /// (See `coverage_counter_add` for more information.)
-    #[cfg(not(bootstrap))]
-    pub fn coverage_unreachable(
-        file_name: &'static str,
-        start_line: u32,
-        start_col: u32,
-        end_line: u32,
-        end_col: u32,
-    );
-
     /// See documentation of `<*const T>::guaranteed_eq` for details.
     #[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")]
     pub fn ptr_guaranteed_eq<T>(ptr: *const T, other: *const T) -> bool;

src/librustc_codegen_llvm/builder.rs

@@ -1117,7 +1117,7 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         hash: &'ll Value,
         num_counters: &'ll Value,
         index: &'ll Value,
-    ) -> &'ll Value {
+    ) {
         debug!(
             "instrprof_increment() with args ({:?}, {:?}, {:?}, {:?})",
             fn_name, hash, num_counters, index
@@ -1128,13 +1128,13 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         let args = self.check_call("call", llfn, args);
 
         unsafe {
-            llvm::LLVMRustBuildCall(
+            let _ = llvm::LLVMRustBuildCall(
                 self.llbuilder,
                 llfn,
                 args.as_ptr() as *const &llvm::Value,
                 args.len() as c_uint,
                 None,
-            )
+            );
         }
     }
 

src/librustc_codegen_llvm/coverageinfo/mapgen.rs

@@ -3,14 +3,16 @@ use crate::coverageinfo;
 use crate::llvm;
 
 use llvm::coverageinfo::CounterMappingRegion;
-use rustc_codegen_ssa::coverageinfo::map::{Counter, CounterExpression, Region};
+use rustc_codegen_ssa::coverageinfo::map::{Counter, CounterExpression};
 use rustc_codegen_ssa::traits::{BaseTypeMethods, ConstMethods};
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_llvm::RustString;
-use tracing::debug;
+use rustc_middle::mir::coverage::CodeRegion;
 
 use std::ffi::CString;
 
+use tracing::debug;
+
 /// Generates and exports the Coverage Map.
 ///
 /// This Coverage Map complies with Coverage Mapping Format version 3 (zero-based encoded as 2),
@@ -91,7 +93,7 @@ impl CoverageMapGenerator {
     fn write_coverage_mappings(
         &mut self,
         expressions: Vec<CounterExpression>,
-        counter_regions: impl Iterator<Item = (Counter, &'tcx Region<'tcx>)>,
+        counter_regions: impl Iterator<Item = (Counter, &'a CodeRegion)>,
         coverage_mappings_buffer: &RustString,
     ) {
         let mut counter_regions = counter_regions.collect::<Vec<_>>();
@@ -104,22 +106,22 @@ impl CoverageMapGenerator {
         let mut current_file_name = None;
         let mut current_file_id = 0;
 
-        // Convert the list of (Counter, Region) pairs to an array of `CounterMappingRegion`, sorted
+        // Convert the list of (Counter, CodeRegion) pairs to an array of `CounterMappingRegion`, sorted
         // by filename and position. Capture any new files to compute the `CounterMappingRegion`s
         // `file_id` (indexing files referenced by the current function), and construct the
         // function-specific `virtual_file_mapping` from `file_id` to its index in the module's
         // `filenames` array.
         counter_regions.sort_unstable_by_key(|(_counter, region)| *region);
         for (counter, region) in counter_regions {
-            let Region { file_name, start_line, start_col, end_line, end_col } = *region;
-            let same_file = current_file_name.as_ref().map_or(false, |p| p == file_name);
+            let CodeRegion { file_name, start_line, start_col, end_line, end_col } = *region;
+            let same_file = current_file_name.as_ref().map_or(false, |p| *p == file_name);
             if !same_file {
                 if current_file_name.is_some() {
                     current_file_id += 1;
                 }
-                current_file_name = Some(file_name.to_string());
-                let c_filename =
-                    CString::new(file_name).expect("null error converting filename to C string");
+                current_file_name = Some(file_name);
+                let c_filename = CString::new(file_name.to_string())
+                    .expect("null error converting filename to C string");
                 debug!("  file_id: {} = '{:?}'", current_file_id, c_filename);
                 let (filenames_index, _) = self.filenames.insert_full(c_filename);
                 virtual_file_mapping.push(filenames_index as u32);

src/librustc_codegen_llvm/coverageinfo/mod.rs

@@ -5,34 +5,38 @@ use crate::common::CodegenCx;
 
 use libc::c_uint;
 use llvm::coverageinfo::CounterMappingRegion;
-use rustc_codegen_ssa::coverageinfo::map::{CounterExpression, ExprKind, FunctionCoverage, Region};
+use rustc_codegen_ssa::coverageinfo::map::{CounterExpression, FunctionCoverage};
 use rustc_codegen_ssa::traits::{
     BaseTypeMethods, CoverageInfoBuilderMethods, CoverageInfoMethods, MiscMethods, StaticMethods,
 };
 use rustc_data_structures::fx::FxHashMap;
 use rustc_llvm::RustString;
+use rustc_middle::mir::coverage::{
+    CodeRegion, CounterValueReference, ExpressionOperandId, InjectedExpressionIndex, Op,
+};
 use rustc_middle::ty::Instance;
-use tracing::debug;
 
 use std::cell::RefCell;
 use std::ffi::CString;
 
+use tracing::debug;
+
 pub mod mapgen;
 
 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<'tcx>>>,
+    pub(crate) function_coverage_map: RefCell<FxHashMap<Instance<'tcx>, FunctionCoverage>>,
 }
 
 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<'tcx>> {
+    pub fn take_function_coverage_map(&self) -> FxHashMap<Instance<'tcx>, FunctionCoverage> {
         self.function_coverage_map.replace(FxHashMap::default())
     }
 }
@@ -58,11 +62,11 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         &mut self,
         instance: Instance<'tcx>,
         function_source_hash: u64,
-        id: u32,
-        region: Region<'tcx>,
+        id: CounterValueReference,
+        region: CodeRegion,
     ) {
         debug!(
-            "adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={}, \
+            "adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={:?}, \
              at {:?}",
             instance, function_source_hash, id, region,
         );
@@ -76,25 +80,25 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
     fn add_counter_expression_region(
         &mut self,
         instance: Instance<'tcx>,
-        id_descending_from_max: u32,
-        lhs: u32,
-        op: ExprKind,
-        rhs: u32,
-        region: Region<'tcx>,
+        id: InjectedExpressionIndex,
+        lhs: ExpressionOperandId,
+        op: Op,
+        rhs: ExpressionOperandId,
+        region: CodeRegion,
     ) {
         debug!(
-            "adding counter expression to coverage_regions: instance={:?}, id={}, {} {:?} {}, \
+            "adding counter expression to coverage_regions: instance={:?}, id={:?}, {:?} {:?} {:?}, \
              at {:?}",
-            instance, id_descending_from_max, lhs, op, rhs, region,
+            instance, id, lhs, op, rhs, region,
         );
         let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
         coverage_regions
             .entry(instance)
             .or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
-            .add_counter_expression(id_descending_from_max, lhs, op, rhs, region);
+            .add_counter_expression(id, lhs, op, rhs, region);
     }
 
-    fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: Region<'tcx>) {
+    fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion) {
         debug!(
             "adding unreachable code to coverage_regions: instance={:?}, at {:?}",
             instance, region,

src/librustc_codegen_llvm/intrinsic.rs

@@ -11,22 +11,18 @@ use rustc_ast as ast;
 use rustc_codegen_ssa::base::{compare_simd_types, to_immediate, wants_msvc_seh};
 use rustc_codegen_ssa::common::span_invalid_monomorphization_error;
 use rustc_codegen_ssa::common::{IntPredicate, TypeKind};
-use rustc_codegen_ssa::coverageinfo;
 use rustc_codegen_ssa::glue;
 use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
 use rustc_codegen_ssa::mir::place::PlaceRef;
 use rustc_codegen_ssa::traits::*;
 use rustc_codegen_ssa::MemFlags;
 use rustc_hir as hir;
-use rustc_middle::mir::coverage;
-use rustc_middle::mir::Operand;
 use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt};
 use rustc_middle::ty::{self, Ty};
 use rustc_middle::{bug, span_bug};
 use rustc_span::{sym, symbol::kw, Span, Symbol};
 use rustc_target::abi::{self, HasDataLayout, LayoutOf, Primitive};
 use rustc_target::spec::PanicStrategy;
-use tracing::debug;
 
 use std::cmp::Ordering;
 use std::iter;
@@ -83,85 +79,13 @@ fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: Symbol) -> Option<&'ll Va
 }
 
 impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
-    fn is_codegen_intrinsic(
-        &mut self,
-        intrinsic: Symbol,
-        args: &Vec<Operand<'tcx>>,
-        caller_instance: ty::Instance<'tcx>,
-    ) -> bool {
-        let mut is_codegen_intrinsic = true;
-        // Set `is_codegen_intrinsic` to `false` to bypass `codegen_intrinsic_call()`.
-
-        // FIXME(richkadel): Make sure to add coverage analysis tests on a crate with
-        // external crate dependencies, where:
-        //   1. Both binary and dependent crates are compiled with `-Zinstrument-coverage`
-        //   2. Only binary is compiled with `-Zinstrument-coverage`
-        //   3. Only dependent crates are compiled with `-Zinstrument-coverage`
-        match intrinsic {
-            sym::count_code_region => {
-                use coverage::count_code_region_args::*;
-                self.add_counter_region(
-                    caller_instance,
-                    op_to_u64(&args[FUNCTION_SOURCE_HASH]),
-                    op_to_u32(&args[COUNTER_ID]),
-                    coverageinfo::Region::new(
-                        op_to_str_slice(&args[FILE_NAME]),
-                        op_to_u32(&args[START_LINE]),
-                        op_to_u32(&args[START_COL]),
-                        op_to_u32(&args[END_LINE]),
-                        op_to_u32(&args[END_COL]),
-                    ),
-                );
-            }
-            sym::coverage_counter_add | sym::coverage_counter_subtract => {
-                is_codegen_intrinsic = false;
-                use coverage::coverage_counter_expression_args::*;
-                self.add_counter_expression_region(
-                    caller_instance,
-                    op_to_u32(&args[EXPRESSION_ID]),
-                    op_to_u32(&args[LEFT_ID]),
-                    if intrinsic == sym::coverage_counter_add {
-                        coverageinfo::ExprKind::Add
-                    } else {
-                        coverageinfo::ExprKind::Subtract
-                    },
-                    op_to_u32(&args[RIGHT_ID]),
-                    coverageinfo::Region::new(
-                        op_to_str_slice(&args[FILE_NAME]),
-                        op_to_u32(&args[START_LINE]),
-                        op_to_u32(&args[START_COL]),
-                        op_to_u32(&args[END_LINE]),
-                        op_to_u32(&args[END_COL]),
-                    ),
-                );
-            }
-            sym::coverage_unreachable => {
-                is_codegen_intrinsic = false;
-                use coverage::coverage_unreachable_args::*;
-                self.add_unreachable_region(
-                    caller_instance,
-                    coverageinfo::Region::new(
-                        op_to_str_slice(&args[FILE_NAME]),
-                        op_to_u32(&args[START_LINE]),
-                        op_to_u32(&args[START_COL]),
-                        op_to_u32(&args[END_LINE]),
-                        op_to_u32(&args[END_COL]),
-                    ),
-                );
-            }
-            _ => {}
-        }
-        is_codegen_intrinsic
-    }
-
     fn codegen_intrinsic_call(
         &mut self,
         instance: ty::Instance<'tcx>,
         fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
         args: &[OperandRef<'tcx, &'ll Value>],
         llresult: &'ll Value,
         span: Span,
-        caller_instance: ty::Instance<'tcx>,
     ) {
         let tcx = self.tcx;
         let callee_ty = instance.ty(tcx, ty::ParamEnv::reveal_all());
@@ -213,21 +137,6 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
                 let llfn = self.get_intrinsic(&("llvm.debugtrap"));
                 self.call(llfn, &[], None)
             }
-            sym::count_code_region => {
-                use coverage::count_code_region_args::*;
-                let coverageinfo = tcx.coverageinfo(caller_instance.def_id());
-
-                let fn_name = self.create_pgo_func_name_var(caller_instance);
-                let hash = args[FUNCTION_SOURCE_HASH].immediate();
-                let num_counters = self.const_u32(coverageinfo.num_counters);
-                let index = args[COUNTER_ID].immediate();
-                debug!(
-                    "translating Rust intrinsic `count_code_region()` to LLVM intrinsic: \
-                    instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})",
-                    fn_name, hash, num_counters, index,
-                );
-                self.instrprof_increment(fn_name, hash, num_counters, index)
-            }
             sym::va_start => self.va_start(args[0].immediate()),
             sym::va_end => self.va_end(args[0].immediate()),
             sym::va_copy => {
@@ -2238,15 +2147,3 @@ fn float_type_width(ty: Ty<'_>) -> Option<u64> {
         _ => None,
     }
 }
-
-fn op_to_str_slice<'tcx>(op: &Operand<'tcx>) -> &'tcx str {
-    Operand::value_from_const(op).try_to_str_slice().expect("Value is &str")
-}
-
-fn op_to_u32<'tcx>(op: &Operand<'tcx>) -> u32 {
-    Operand::scalar_from_const(op).to_u32().expect("Scalar is u32")
-}
-
-fn op_to_u64<'tcx>(op: &Operand<'tcx>) -> u64 {
-    Operand::scalar_from_const(op).to_u64().expect("Scalar is u64")
-}

src/librustc_codegen_ssa/coverageinfo/ffi.rs

@@ -1,4 +1,4 @@
-use super::map::{CounterValueReference, MappedExpressionIndex};
+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)]