Skip to content

Commit 541d4e8

Browse files
committed
coverage: Track used functions in a set instead of a map
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.
1 parent d34c365 commit 541d4e8

File tree

3 files changed

+37
-90
lines changed

3 files changed

+37
-90
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

-26
This file was deleted.

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

+33-49
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use tracing::debug;
1818

1919
use crate::common::CodegenCx;
2020
use crate::coverageinfo::llvm_cov;
21-
use crate::coverageinfo::map_data::FunctionCoverage;
2221
use crate::coverageinfo::mapgen::covfun::prepare_covfun_record;
2322
use crate::llvm;
2423

@@ -49,23 +48,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
4948

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

52-
// In order to show that unused functions have coverage counts of zero (0), LLVM requires the
53-
// functions exist. Generate synthetic functions with a (required) single counter, and add the
54-
// MIR `Coverage` code regions to the `function_coverage_map`, before calling
55-
// `ctx.take_function_coverage_map()`.
56-
if cx.codegen_unit.is_code_coverage_dead_code_cgu() {
57-
add_unused_functions(cx);
58-
}
59-
6051
// FIXME(#132395): Can this be none even when coverage is enabled?
61-
let function_coverage_map = match cx.coverage_cx {
62-
Some(ref cx) => cx.take_function_coverage_map(),
52+
let instances_used = match cx.coverage_cx {
53+
Some(ref cx) => cx.instances_used.borrow(),
6354
None => return,
6455
};
65-
if function_coverage_map.is_empty() {
66-
// This CGU has no functions with coverage instrumentation.
67-
return;
68-
}
6956

7057
// The order of entries in this global file table needs to be deterministic,
7158
// and ideally should also be independent of the details of stable-hashing,
@@ -74,18 +61,27 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
7461
// are satisfied, the order can be arbitrary.
7562
let mut global_file_table = GlobalFileTable::new();
7663

77-
let covfun_records = function_coverage_map
78-
.into_iter()
64+
let mut covfun_records = instances_used
65+
.iter()
66+
.copied()
7967
// Sort by symbol name, so that the global file table is built in an
8068
// order that doesn't depend on the stable-hash-based order in which
8169
// instances were visited during codegen.
82-
.sorted_by_cached_key(|&(instance, _)| tcx.symbol_name(instance).name)
83-
.filter_map(|(instance, function_coverage)| {
84-
let is_used = function_coverage.is_used();
85-
prepare_covfun_record(tcx, &mut global_file_table, instance, is_used)
86-
})
70+
.sorted_by_cached_key(|&instance| tcx.symbol_name(instance).name)
71+
.filter_map(|instance| prepare_covfun_record(tcx, &mut global_file_table, instance, true))
8772
.collect::<Vec<_>>();
8873

74+
// In a single designated CGU, also prepare covfun records for functions
75+
// in this crate that were instrumented for coverage, but are unused.
76+
if cx.codegen_unit.is_code_coverage_dead_code_cgu() {
77+
let mut unused_instances = gather_unused_function_instances(cx);
78+
// Sort the unused instances by symbol name, for the same reason as the used ones.
79+
unused_instances.sort_by_cached_key(|&instance| tcx.symbol_name(instance).name);
80+
covfun_records.extend(unused_instances.into_iter().filter_map(|instance| {
81+
prepare_covfun_record(tcx, &mut global_file_table, instance, false)
82+
}));
83+
}
84+
8985
// If there are no covfun records for this CGU, don't generate a covmap record.
9086
// Emitting a covmap record without any covfun records causes `llvm-cov` to
9187
// fail when generating coverage reports, and if there are no covfun records
@@ -261,7 +257,7 @@ fn generate_covmap_record<'ll>(cx: &CodegenCx<'ll, '_>, version: u32, filenames_
261257
/// coverage map (in a single designated CGU) so that we still emit coverage mappings for them.
262258
/// We also end up adding their symbol names to a special global array that LLVM will include in
263259
/// its embedded coverage data.
264-
fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
260+
fn gather_unused_function_instances<'tcx>(cx: &CodegenCx<'_, 'tcx>) -> Vec<ty::Instance<'tcx>> {
265261
assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu());
266262

267263
let tcx = cx.tcx;
@@ -279,20 +275,17 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
279275
&& !usage.used_via_inlining.contains(&d)
280276
};
281277

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

288-
// FIXME(79651): Consider trying to filter out dummy instantiations of
289-
// unused generic functions from library crates, because they can produce
290-
// "unused instantiation" in coverage reports even when they are actually
291-
// used by some downstream crate in the same binary.
292-
293-
debug!("generating unused fn: {def_id:?}");
294-
add_unused_function_coverage(cx, def_id, function_coverage_info);
295-
}
283+
tcx.mir_keys(())
284+
.iter()
285+
.copied()
286+
.filter(|&def_id| is_unused_fn(def_id))
287+
.map(|def_id| make_dummy_instance(tcx, def_id))
288+
.collect::<Vec<_>>()
296289
}
297290

298291
struct UsageSets<'tcx> {
@@ -357,16 +350,11 @@ fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> {
357350
UsageSets { all_mono_items, used_via_inlining, missing_own_coverage }
358351
}
359352

360-
fn add_unused_function_coverage<'tcx>(
361-
cx: &CodegenCx<'_, 'tcx>,
362-
def_id: LocalDefId,
363-
function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo,
364-
) {
365-
let tcx = cx.tcx;
366-
let def_id = def_id.to_def_id();
353+
fn make_dummy_instance<'tcx>(tcx: TyCtxt<'tcx>, local_def_id: LocalDefId) -> ty::Instance<'tcx> {
354+
let def_id = local_def_id.to_def_id();
367355

368356
// Make a dummy instance that fills in all generics with placeholders.
369-
let instance = ty::Instance::new(
357+
ty::Instance::new(
370358
def_id,
371359
ty::GenericArgs::for_item(tcx, def_id, |param, _| {
372360
if let ty::GenericParamDefKind::Lifetime = param.kind {
@@ -375,9 +363,5 @@ fn add_unused_function_coverage<'tcx>(
375363
tcx.mk_param_from_def(param)
376364
}
377365
}),
378-
);
379-
380-
// An unused function's mappings will all be rewritten to map to zero.
381-
let function_coverage = FunctionCoverage::new_unused(function_coverage_info);
382-
cx.coverage_cx().function_coverage_map.borrow_mut().insert(instance, function_coverage);
366+
)
383367
}

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

+4-15
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,24 @@ use rustc_abi::Size;
55
use rustc_codegen_ssa::traits::{
66
BuilderMethods, ConstCodegenMethods, CoverageInfoBuilderMethods, MiscCodegenMethods,
77
};
8-
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
8+
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
99
use rustc_middle::mir::coverage::CoverageKind;
1010
use rustc_middle::ty::Instance;
1111
use rustc_middle::ty::layout::HasTyCtxt;
1212
use tracing::{debug, instrument};
1313

1414
use crate::builder::Builder;
1515
use crate::common::CodegenCx;
16-
use crate::coverageinfo::map_data::FunctionCoverage;
1716
use crate::llvm;
1817

1918
pub(crate) mod ffi;
2019
mod llvm_cov;
21-
pub(crate) mod map_data;
2220
mod mapgen;
2321

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

@@ -34,17 +32,13 @@ pub(crate) struct CguCoverageContext<'ll, 'tcx> {
3432
impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> {
3533
pub(crate) fn new() -> Self {
3634
Self {
37-
function_coverage_map: Default::default(),
35+
instances_used: RefCell::<FxIndexSet<_>>::default(),
3836
pgo_func_name_var_map: Default::default(),
3937
mcdc_condition_bitmap_map: Default::default(),
4038
covfun_section_name: Default::default(),
4139
}
4240
}
4341

44-
fn take_function_coverage_map(&self) -> FxIndexMap<Instance<'tcx>, FunctionCoverage<'tcx>> {
45-
self.function_coverage_map.replace(FxIndexMap::default())
46-
}
47-
4842
/// LLVM use a temp value to record evaluated mcdc test vector of each decision, which is
4943
/// called condition bitmap. In order to handle nested decisions, several condition bitmaps can
5044
/// be allocated for a function body. These values are named `mcdc.addr.{i}` and are a 32-bit
@@ -157,12 +151,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
157151
// Mark the instance as used in this CGU, for coverage purposes.
158152
// This includes functions that were not partitioned into this CGU,
159153
// but were MIR-inlined into one of this CGU's functions.
160-
coverage_cx.function_coverage_map.borrow_mut().entry(instance).or_insert_with(|| {
161-
FunctionCoverage::new_used(
162-
function_coverage_info,
163-
bx.tcx.coverage_ids_info(instance.def),
164-
)
165-
});
154+
coverage_cx.instances_used.borrow_mut().insert(instance);
166155

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

0 commit comments

Comments
 (0)