Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit caa8172

Browse filesBrowse files
borsbors
committed
Auto merge of rust-lang#133723 - jhpratt:rollup-pyfespf, r=jhpratt
Rollup of 5 pull requests Successful merges: - rust-lang#131416 (Mark `slice::copy_from_slice` unstably const) - rust-lang#131784 (Stabilize unsigned and float variants of `num_midpoint` feature) - rust-lang#133446 (coverage: Use a query to identify which counter/expression IDs are used) - rust-lang#133711 (add isatty doc alias for `is_terminal`) - rust-lang#133712 (rust_analyzer_settings: force use of 'nightly' toolchain) r? `@ghost` `@rustbot` modify labels: rollup
2 parents a522d78 + 76183a5 commit caa8172
Copy full SHA for caa8172

File tree

Expand file treeCollapse file tree

20 files changed

+163
-156
lines changed
Filter options
Expand file treeCollapse file tree

20 files changed

+163
-156
lines changed

‎compiler/rustc_codegen_llvm/src/context.rs

Copy file name to clipboardExpand all lines: compiler/rustc_codegen_llvm/src/context.rs
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ pub(crate) struct CodegenCx<'ll, 'tcx> {
8282

8383
pub isize_ty: &'ll Type,
8484

85-
/// Extra codegen state needed when coverage instrumentation is enabled.
86-
pub coverage_cx: Option<coverageinfo::CrateCoverageContext<'ll, 'tcx>>,
85+
/// Extra per-CGU codegen state needed when coverage instrumentation is enabled.
86+
pub coverage_cx: Option<coverageinfo::CguCoverageContext<'ll, 'tcx>>,
8787
pub dbg_cx: Option<debuginfo::CodegenUnitDebugContext<'ll, 'tcx>>,
8888

8989
eh_personality: Cell<Option<&'ll Value>>,
@@ -525,7 +525,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
525525
let (llcx, llmod) = (&*llvm_module.llcx, llvm_module.llmod());
526526

527527
let coverage_cx =
528-
tcx.sess.instrument_coverage().then(coverageinfo::CrateCoverageContext::new);
528+
tcx.sess.instrument_coverage().then(coverageinfo::CguCoverageContext::new);
529529

530530
let dbg_cx = if tcx.sess.opts.debuginfo != DebugInfo::None {
531531
let dctx = debuginfo::CodegenUnitDebugContext::new(llmod);
@@ -576,7 +576,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
576576
/// Extra state that is only available when coverage instrumentation is enabled.
577577
#[inline]
578578
#[track_caller]
579-
pub(crate) fn coverage_cx(&self) -> &coverageinfo::CrateCoverageContext<'ll, 'tcx> {
579+
pub(crate) fn coverage_cx(&self) -> &coverageinfo::CguCoverageContext<'ll, 'tcx> {
580580
self.coverage_cx.as_ref().expect("only called when coverage instrumentation is enabled")
581581
}
582582

‎compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

Copy file name to clipboardExpand all lines: compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
+15-57Lines changed: 15 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use rustc_data_structures::captures::Captures;
22
use rustc_data_structures::fx::FxIndexSet;
33
use rustc_index::bit_set::BitSet;
4+
use rustc_middle::mir::CoverageIdsInfo;
45
use rustc_middle::mir::coverage::{
56
CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
67
SourceRegion,
78
};
89
use rustc_middle::ty::Instance;
9-
use tracing::{debug, instrument};
10+
use tracing::debug;
1011

1112
use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
1213

@@ -16,39 +17,33 @@ use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
1617
pub(crate) struct FunctionCoverageCollector<'tcx> {
1718
/// Coverage info that was attached to this function by the instrumentor.
1819
function_coverage_info: &'tcx FunctionCoverageInfo,
20+
ids_info: &'tcx CoverageIdsInfo,
1921
is_used: bool,
20-
21-
/// Tracks which counters have been seen, so that we can identify mappings
22-
/// to counters that were optimized out, and set them to zero.
23-
counters_seen: BitSet<CounterId>,
24-
/// Contains all expression IDs that have been seen in an `ExpressionUsed`
25-
/// coverage statement, plus all expression IDs that aren't directly used
26-
/// by any mappings (and therefore do not have expression-used statements).
27-
/// After MIR traversal is finished, we can conclude that any IDs missing
28-
/// from this set must have had their statements deleted by MIR opts.
29-
expressions_seen: BitSet<ExpressionId>,
3022
}
3123

3224
impl<'tcx> FunctionCoverageCollector<'tcx> {
3325
/// Creates a new set of coverage data for a used (called) function.
3426
pub(crate) fn new(
3527
instance: Instance<'tcx>,
3628
function_coverage_info: &'tcx FunctionCoverageInfo,
29+
ids_info: &'tcx CoverageIdsInfo,
3730
) -> Self {
38-
Self::create(instance, function_coverage_info, true)
31+
Self::create(instance, function_coverage_info, ids_info, true)
3932
}
4033

4134
/// Creates a new set of coverage data for an unused (never called) function.
4235
pub(crate) fn unused(
4336
instance: Instance<'tcx>,
4437
function_coverage_info: &'tcx FunctionCoverageInfo,
38+
ids_info: &'tcx CoverageIdsInfo,
4539
) -> Self {
46-
Self::create(instance, function_coverage_info, false)
40+
Self::create(instance, function_coverage_info, ids_info, false)
4741
}
4842

4943
fn create(
5044
instance: Instance<'tcx>,
5145
function_coverage_info: &'tcx FunctionCoverageInfo,
46+
ids_info: &'tcx CoverageIdsInfo,
5247
is_used: bool,
5348
) -> Self {
5449
let num_counters = function_coverage_info.num_counters;
@@ -58,44 +53,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
5853
num_counters={num_counters}, num_expressions={num_expressions}, is_used={is_used}"
5954
);
6055

61-
// Create a filled set of expression IDs, so that expressions not
62-
// directly used by mappings will be treated as "seen".
63-
// (If they end up being unused, LLVM will delete them for us.)
64-
let mut expressions_seen = BitSet::new_filled(num_expressions);
65-
// For each expression ID that is directly used by one or more mappings,
66-
// mark it as not-yet-seen. This indicates that we expect to see a
67-
// corresponding `ExpressionUsed` statement during MIR traversal.
68-
for mapping in function_coverage_info.mappings.iter() {
69-
// Currently we only worry about ordinary code mappings.
70-
// For branch and MC/DC mappings, expressions might not correspond
71-
// to any particular point in the control-flow graph.
72-
// (Keep this in sync with the injection of `ExpressionUsed`
73-
// statements in the `InstrumentCoverage` MIR pass.)
74-
if let MappingKind::Code(term) = mapping.kind
75-
&& let CovTerm::Expression(id) = term
76-
{
77-
expressions_seen.remove(id);
78-
}
79-
}
80-
81-
Self {
82-
function_coverage_info,
83-
is_used,
84-
counters_seen: BitSet::new_empty(num_counters),
85-
expressions_seen,
86-
}
87-
}
88-
89-
/// Marks a counter ID as having been seen in a counter-increment statement.
90-
#[instrument(level = "debug", skip(self))]
91-
pub(crate) fn mark_counter_id_seen(&mut self, id: CounterId) {
92-
self.counters_seen.insert(id);
93-
}
94-
95-
/// Marks an expression ID as having been seen in an expression-used statement.
96-
#[instrument(level = "debug", skip(self))]
97-
pub(crate) fn mark_expression_id_seen(&mut self, id: ExpressionId) {
98-
self.expressions_seen.insert(id);
56+
Self { function_coverage_info, ids_info, is_used }
9957
}
10058

10159
/// Identify expressions that will always have a value of zero, and note
@@ -117,7 +75,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
11775
// (By construction, expressions can only refer to other expressions
11876
// that have lower IDs, so one pass is sufficient.)
11977
for (id, expression) in self.function_coverage_info.expressions.iter_enumerated() {
120-
if !self.expressions_seen.contains(id) {
78+
if !self.is_used || !self.ids_info.expressions_seen.contains(id) {
12179
// If an expression was not seen, it must have been optimized away,
12280
// so any operand that refers to it can be replaced with zero.
12381
zero_expressions.insert(id);
@@ -146,7 +104,7 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
146104
assert_operand_expression_is_lower(id);
147105
}
148106

149-
if is_zero_term(&self.counters_seen, &zero_expressions, *operand) {
107+
if is_zero_term(&self.ids_info.counters_seen, &zero_expressions, *operand) {
150108
*operand = CovTerm::Zero;
151109
}
152110
};
@@ -172,17 +130,17 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
172130

173131
pub(crate) fn into_finished(self) -> FunctionCoverage<'tcx> {
174132
let zero_expressions = self.identify_zero_expressions();
175-
let FunctionCoverageCollector { function_coverage_info, is_used, counters_seen, .. } = self;
133+
let FunctionCoverageCollector { function_coverage_info, ids_info, is_used, .. } = self;
176134

177-
FunctionCoverage { function_coverage_info, is_used, counters_seen, zero_expressions }
135+
FunctionCoverage { function_coverage_info, ids_info, is_used, zero_expressions }
178136
}
179137
}
180138

181139
pub(crate) struct FunctionCoverage<'tcx> {
182140
pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo,
141+
ids_info: &'tcx CoverageIdsInfo,
183142
is_used: bool,
184143

185-
counters_seen: BitSet<CounterId>,
186144
zero_expressions: ZeroExpressions,
187145
}
188146

@@ -238,7 +196,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
238196
}
239197

240198
fn is_zero_term(&self, term: CovTerm) -> bool {
241-
is_zero_term(&self.counters_seen, &self.zero_expressions, term)
199+
!self.is_used || is_zero_term(&self.ids_info.counters_seen, &self.zero_expressions, term)
242200
}
243201
}
244202

‎compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

Copy file name to clipboardExpand all lines: compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
+6-3Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -535,9 +535,12 @@ fn add_unused_function_coverage<'tcx>(
535535
}),
536536
);
537537

538-
// An unused function's mappings will automatically be rewritten to map to
539-
// zero, because none of its counters/expressions are marked as seen.
540-
let function_coverage = FunctionCoverageCollector::unused(instance, function_coverage_info);
538+
// An unused function's mappings will all be rewritten to map to zero.
539+
let function_coverage = FunctionCoverageCollector::unused(
540+
instance,
541+
function_coverage_info,
542+
tcx.coverage_ids_info(instance.def),
543+
);
541544

542545
cx.coverage_cx().function_coverage_map.borrow_mut().insert(instance, function_coverage);
543546
}

‎compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

Copy file name to clipboardExpand all lines: compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
+30-27Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ mod llvm_cov;
2121
pub(crate) mod map_data;
2222
mod mapgen;
2323

24-
/// A context object for maintaining all state needed by the coverageinfo module.
25-
pub(crate) struct CrateCoverageContext<'ll, 'tcx> {
24+
/// Extra per-CGU context/state needed for coverage instrumentation.
25+
pub(crate) struct CguCoverageContext<'ll, 'tcx> {
2626
/// Coverage data for each instrumented function identified by DefId.
2727
pub(crate) function_coverage_map:
2828
RefCell<FxIndexMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>>>,
@@ -32,7 +32,7 @@ pub(crate) struct CrateCoverageContext<'ll, 'tcx> {
3232
covfun_section_name: OnceCell<CString>,
3333
}
3434

35-
impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> {
35+
impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> {
3636
pub(crate) fn new() -> Self {
3737
Self {
3838
function_coverage_map: Default::default(),
@@ -143,39 +143,42 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
143143

144144
let bx = self;
145145

146+
// Due to LocalCopy instantiation or MIR inlining, coverage statements
147+
// can end up in a crate that isn't doing coverage instrumentation.
148+
// When that happens, we currently just discard those statements, so
149+
// the corresponding code will be undercounted.
150+
// FIXME(Zalathar): Find a better solution for mixed-coverage builds.
151+
let Some(coverage_cx) = &bx.cx.coverage_cx else { return };
152+
146153
let Some(function_coverage_info) =
147154
bx.tcx.instance_mir(instance.def).function_coverage_info.as_deref()
148155
else {
149156
debug!("function has a coverage statement but no coverage info");
150157
return;
151158
};
152159

153-
// FIXME(#132395): Unwrapping `coverage_cx` here has led to ICEs in the
154-
// wild, so keep this early-return until we understand why.
155-
let mut coverage_map = match bx.coverage_cx {
156-
Some(ref cx) => cx.function_coverage_map.borrow_mut(),
157-
None => return,
158-
};
159-
let func_coverage = coverage_map
160-
.entry(instance)
161-
.or_insert_with(|| FunctionCoverageCollector::new(instance, function_coverage_info));
160+
// Mark the instance as used in this CGU, for coverage purposes.
161+
// This includes functions that were not partitioned into this CGU,
162+
// but were MIR-inlined into one of this CGU's functions.
163+
coverage_cx.function_coverage_map.borrow_mut().entry(instance).or_insert_with(|| {
164+
FunctionCoverageCollector::new(
165+
instance,
166+
function_coverage_info,
167+
bx.tcx.coverage_ids_info(instance.def),
168+
)
169+
});
162170

163171
match *kind {
164172
CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!(
165173
"marker statement {kind:?} should have been removed by CleanupPostBorrowck"
166174
),
167175
CoverageKind::CounterIncrement { id } => {
168-
func_coverage.mark_counter_id_seen(id);
169-
// We need to explicitly drop the `RefMut` before calling into
170-
// `instrprof_increment`, as that needs an exclusive borrow.
171-
drop(coverage_map);
172-
173176
// The number of counters passed to `llvm.instrprof.increment` might
174177
// be smaller than the number originally inserted by the instrumentor,
175178
// if some high-numbered counters were removed by MIR optimizations.
176179
// If so, LLVM's profiler runtime will use fewer physical counters.
177180
let num_counters =
178-
bx.tcx().coverage_ids_info(instance.def).max_counter_id.as_u32() + 1;
181+
bx.tcx().coverage_ids_info(instance.def).num_counters_after_mir_opts();
179182
assert!(
180183
num_counters as usize <= function_coverage_info.num_counters,
181184
"num_counters disagreement: query says {num_counters} but function info only has {}",
@@ -192,23 +195,23 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
192195
);
193196
bx.instrprof_increment(fn_name, hash, num_counters, index);
194197
}
195-
CoverageKind::ExpressionUsed { id } => {
196-
func_coverage.mark_expression_id_seen(id);
198+
CoverageKind::ExpressionUsed { id: _ } => {
199+
// Expression-used statements are markers that are handled by
200+
// `coverage_ids_info`, so there's nothing to codegen here.
197201
}
198202
CoverageKind::CondBitmapUpdate { index, decision_depth } => {
199-
drop(coverage_map);
200-
let cond_bitmap = bx
201-
.coverage_cx()
203+
let cond_bitmap = coverage_cx
202204
.try_get_mcdc_condition_bitmap(&instance, decision_depth)
203205
.expect("mcdc cond bitmap should have been allocated for updating");
204206
let cond_index = bx.const_i32(index as i32);
205207
bx.mcdc_condbitmap_update(cond_index, cond_bitmap);
206208
}
207209
CoverageKind::TestVectorBitmapUpdate { bitmap_idx, decision_depth } => {
208-
drop(coverage_map);
209-
let cond_bitmap = bx.coverage_cx()
210-
.try_get_mcdc_condition_bitmap(&instance, decision_depth)
211-
.expect("mcdc cond bitmap should have been allocated for merging into the global bitmap");
210+
let cond_bitmap =
211+
coverage_cx.try_get_mcdc_condition_bitmap(&instance, decision_depth).expect(
212+
"mcdc cond bitmap should have been allocated for merging \
213+
into the global bitmap",
214+
);
212215
assert!(
213216
bitmap_idx as usize <= function_coverage_info.mcdc_bitmap_bits,
214217
"bitmap index of the decision out of range"

‎compiler/rustc_middle/src/mir/coverage.rs

Copy file name to clipboardExpand all lines: compiler/rustc_middle/src/mir/coverage.rs
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ rustc_index::newtype_index! {
2828
#[derive(HashStable)]
2929
#[encodable]
3030
#[orderable]
31-
#[max = 0xFFFF_FFFF]
3231
#[debug_format = "CounterId({})"]
3332
pub struct CounterId {}
3433
}
@@ -46,7 +45,6 @@ rustc_index::newtype_index! {
4645
#[derive(HashStable)]
4746
#[encodable]
4847
#[orderable]
49-
#[max = 0xFFFF_FFFF]
5048
#[debug_format = "ExpressionId({})"]
5149
pub struct ExpressionId {}
5250
}

‎compiler/rustc_middle/src/mir/query.rs

Copy file name to clipboardExpand all lines: compiler/rustc_middle/src/mir/query.rs
+13-3Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_abi::{FieldIdx, VariantIdx};
88
use rustc_data_structures::fx::FxIndexMap;
99
use rustc_errors::ErrorGuaranteed;
1010
use rustc_hir::def_id::LocalDefId;
11-
use rustc_index::bit_set::BitMatrix;
11+
use rustc_index::bit_set::{BitMatrix, BitSet};
1212
use rustc_index::{Idx, IndexVec};
1313
use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable};
1414
use rustc_span::Span;
@@ -359,12 +359,22 @@ pub struct DestructuredConstant<'tcx> {
359359
/// Used by the `coverage_ids_info` query.
360360
#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)]
361361
pub struct CoverageIdsInfo {
362-
/// Coverage codegen needs to know the highest counter ID that is ever
362+
pub counters_seen: BitSet<mir::coverage::CounterId>,
363+
pub expressions_seen: BitSet<mir::coverage::ExpressionId>,
364+
}
365+
366+
impl CoverageIdsInfo {
367+
/// Coverage codegen needs to know how many coverage counters are ever
363368
/// incremented within a function, so that it can set the `num-counters`
364369
/// argument of the `llvm.instrprof.increment` intrinsic.
365370
///
366371
/// This may be less than the highest counter ID emitted by the
367372
/// InstrumentCoverage MIR pass, if the highest-numbered counter increments
368373
/// were removed by MIR optimizations.
369-
pub max_counter_id: mir::coverage::CounterId,
374+
pub fn num_counters_after_mir_opts(&self) -> u32 {
375+
// FIXME(Zalathar): Currently this treats an unused counter as "used"
376+
// if its ID is less than that of the highest counter that really is
377+
// used. Fixing this would require adding a renumbering step somewhere.
378+
self.counters_seen.last_set_in(..).map_or(0, |max| max.as_u32() + 1)
379+
}
370380
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.