Skip to content

Commit

Permalink
coverage: Track used functions in a set instead of a map
Browse files Browse the repository at this point in the history
This patch dismantles what was left of `FunctionCoverage` in `map_data.rs`,
replaces `function_coverage_map` with a set, and overhauls how we prepare
covfun records for unused functions.
  • Loading branch information
Zalathar committed Dec 15, 2024
1 parent a611a40 commit 855ebdf
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 87 deletions.
24 changes: 0 additions & 24 deletions compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

This file was deleted.

77 changes: 29 additions & 48 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use tracing::debug;

use crate::common::CodegenCx;
use crate::coverageinfo::llvm_cov;
use crate::coverageinfo::map_data::FunctionCoverage;
use crate::coverageinfo::mapgen::covfun::prepare_covfun_record;
use crate::llvm;

Expand Down Expand Up @@ -48,34 +47,28 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {

debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name());

// In order to show that unused functions have coverage counts of zero (0), LLVM requires the
// functions exist. Generate synthetic functions with a (required) single counter, and add the
// MIR `Coverage` code regions to the `function_coverage_map`, before calling
// `ctx.take_function_coverage_map()`.
if cx.codegen_unit.is_code_coverage_dead_code_cgu() {
add_unused_functions(cx);
}

// FIXME(#132395): Can this be none even when coverage is enabled?
let function_coverage_map = match cx.coverage_cx {
Some(ref cx) => cx.take_function_coverage_map(),
let instances_used = match cx.coverage_cx {
Some(ref cx) => cx.instances_used.borrow(),
None => return,
};
if function_coverage_map.is_empty() {
// This CGU has no functions with coverage instrumentation.
return;
}

let mut global_file_table = GlobalFileTable::new();

let covfun_records = function_coverage_map
.into_iter()
.filter_map(|(instance, function_coverage)| {
let is_used = function_coverage.is_used();
prepare_covfun_record(tcx, &mut global_file_table, instance, is_used)
})
let mut covfun_records = instances_used
.iter()
.copied()
.filter_map(|instance| prepare_covfun_record(tcx, &mut global_file_table, instance, true))
.collect::<Vec<_>>();

// In a single designated CGU, also prepare covfun records for functions
// in this crate that were instrumented for coverage, but are unused.
if cx.codegen_unit.is_code_coverage_dead_code_cgu() {
covfun_records.extend(gather_unused_functions(cx).into_iter().filter_map(|instance| {
prepare_covfun_record(tcx, &mut global_file_table, instance, false)
}));
}

// If there are no covfun records for this CGU, don't generate a covmap record.
// Emitting a covmap record without any covfun records causes `llvm-cov` to
// fail when generating coverage reports, and if there are no covfun records
Expand Down Expand Up @@ -251,7 +244,7 @@ fn generate_covmap_record<'ll>(cx: &CodegenCx<'ll, '_>, version: u32, filenames_
/// coverage map (in a single designated CGU) so that we still emit coverage mappings for them.
/// We also end up adding their symbol names to a special global array that LLVM will include in
/// its embedded coverage data.
fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
fn gather_unused_functions<'tcx>(cx: &CodegenCx<'_, 'tcx>) -> Vec<ty::Instance<'tcx>> {
assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu());

let tcx = cx.tcx;
Expand All @@ -268,20 +261,17 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
&& !usage.used_via_inlining.contains(&def_id.to_def_id())
};

// Scan for unused functions that were instrumented for coverage.
for def_id in tcx.mir_keys(()).iter().copied().filter(|&def_id| is_unused_fn(def_id)) {
// Get the coverage info from MIR, skipping functions that were never instrumented.
let body = tcx.optimized_mir(def_id);
let Some(function_coverage_info) = body.function_coverage_info.as_deref() else { continue };
// FIXME(79651): Consider trying to filter out dummy instantiations of
// unused generic functions from library crates, because they can produce
// "unused instantiation" in coverage reports even when they are actually
// used by some downstream crate in the same binary.

// FIXME(79651): Consider trying to filter out dummy instantiations of
// unused generic functions from library crates, because they can produce
// "unused instantiation" in coverage reports even when they are actually
// used by some downstream crate in the same binary.

debug!("generating unused fn: {def_id:?}");
add_unused_function_coverage(cx, def_id, function_coverage_info);
}
tcx.mir_keys(())
.iter()
.copied()
.filter(|&def_id| is_unused_fn(def_id))
.map(|def_id| make_dummy_instance(tcx, def_id))
.collect::<Vec<_>>()
}

struct UsageSets<'tcx> {
Expand Down Expand Up @@ -346,16 +336,11 @@ fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> {
UsageSets { all_mono_items, used_via_inlining, missing_own_coverage }
}

fn add_unused_function_coverage<'tcx>(
cx: &CodegenCx<'_, 'tcx>,
def_id: LocalDefId,
function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo,
) {
let tcx = cx.tcx;
let def_id = def_id.to_def_id();
fn make_dummy_instance<'tcx>(tcx: TyCtxt<'tcx>, local_def_id: LocalDefId) -> ty::Instance<'tcx> {
let def_id = local_def_id.to_def_id();

// Make a dummy instance that fills in all generics with placeholders.
let instance = ty::Instance::new(
ty::Instance::new(
def_id,
ty::GenericArgs::for_item(tcx, def_id, |param, _| {
if let ty::GenericParamDefKind::Lifetime = param.kind {
Expand All @@ -364,9 +349,5 @@ fn add_unused_function_coverage<'tcx>(
tcx.mk_param_from_def(param)
}
}),
);

// An unused function's mappings will all be rewritten to map to zero.
let function_coverage = FunctionCoverage::new_unused(function_coverage_info);
cx.coverage_cx().function_coverage_map.borrow_mut().insert(instance, function_coverage);
)
}
19 changes: 4 additions & 15 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,24 @@ use rustc_abi::Size;
use rustc_codegen_ssa::traits::{
BuilderMethods, ConstCodegenMethods, CoverageInfoBuilderMethods, MiscCodegenMethods,
};
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::ty::Instance;
use rustc_middle::ty::layout::HasTyCtxt;
use tracing::{debug, instrument};

use crate::builder::Builder;
use crate::common::CodegenCx;
use crate::coverageinfo::map_data::FunctionCoverage;
use crate::llvm;

pub(crate) mod ffi;
mod llvm_cov;
pub(crate) mod map_data;
mod mapgen;

/// Extra per-CGU context/state needed for coverage instrumentation.
pub(crate) struct CguCoverageContext<'ll, 'tcx> {
/// Coverage data for each instrumented function identified by DefId.
pub(crate) function_coverage_map: RefCell<FxIndexMap<Instance<'tcx>, FunctionCoverage<'tcx>>>,
pub(crate) instances_used: RefCell<FxIndexSet<Instance<'tcx>>>,
pub(crate) pgo_func_name_var_map: RefCell<FxHashMap<Instance<'tcx>, &'ll llvm::Value>>,
pub(crate) mcdc_condition_bitmap_map: RefCell<FxHashMap<Instance<'tcx>, Vec<&'ll llvm::Value>>>,

Expand All @@ -34,17 +32,13 @@ pub(crate) struct CguCoverageContext<'ll, 'tcx> {
impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> {
pub(crate) fn new() -> Self {
Self {
function_coverage_map: Default::default(),
instances_used: RefCell::<FxIndexSet<_>>::default(),
pgo_func_name_var_map: Default::default(),
mcdc_condition_bitmap_map: Default::default(),
covfun_section_name: Default::default(),
}
}

fn take_function_coverage_map(&self) -> FxIndexMap<Instance<'tcx>, FunctionCoverage<'tcx>> {
self.function_coverage_map.replace(FxIndexMap::default())
}

/// LLVM use a temp value to record evaluated mcdc test vector of each decision, which is
/// called condition bitmap. In order to handle nested decisions, several condition bitmaps can
/// be allocated for a function body. These values are named `mcdc.addr.{i}` and are a 32-bit
Expand Down Expand Up @@ -157,12 +151,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
// Mark the instance as used in this CGU, for coverage purposes.
// This includes functions that were not partitioned into this CGU,
// but were MIR-inlined into one of this CGU's functions.
coverage_cx.function_coverage_map.borrow_mut().entry(instance).or_insert_with(|| {
FunctionCoverage::new_used(
function_coverage_info,
bx.tcx.coverage_ids_info(instance.def),
)
});
coverage_cx.instances_used.borrow_mut().insert(instance);

match *kind {
CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!(
Expand Down

0 comments on commit 855ebdf

Please sign in to comment.