nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-01 06:51:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

Make coverage counter IDs count up from 0, not 1

Browse Source 
Operand types are now tracked explicitly, so there is no need to reserve ID 0
for the special always-zero counter.

As part of the renumbering, this change fixes an off-by-one error in the way
counters were counted by the `coverageinfo` query. As a result, functions
should now have exactly the number of counters they actually need, instead of
always having an extra counter that is never used.
This commit is contained in:
Zalathar 2023-06-29 12:36:19 +10:00
parent f103db894f
commit 3920e07f0b
12 changed files with 43 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
View File

@ -1,4 +1,4 @@
use rustc_middle::mir::coverage::{CounterValueReference, MappedExpressionIndex}; use rustc_middle::mir::coverage::{CounterId, MappedExpressionIndex};
/// Must match the layout of `LLVMRustCounterKind`. /// Must match the layout of `LLVMRustCounterKind`.
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
@ -36,11 +36,9 @@ impl Counter {
Self { kind: CounterKind::Zero, id: 0 } Self { kind: CounterKind::Zero, id: 0 }
} }
/// Constructs a new `Counter` of kind `CounterValueReference`, and converts /// Constructs a new `Counter` of kind `CounterValueReference`.
/// the given 1-based counter_id to the required 0-based equivalent for pub fn counter_value_reference(counter_id: CounterId) -> Self {
/// the `Counter` encoding. Self { kind: CounterKind::CounterValueReference, id: counter_id.as_u32() }
pub fn counter_value_reference(counter_id: CounterValueReference) -> Self {
Self { kind: CounterKind::CounterValueReference, id: counter_id.zero_based_index() }
} }
/// Constructs a new `Counter` of kind `Expression`. /// Constructs a new `Counter` of kind `Expression`.

19
compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
View File

@ -3,7 +3,7 @@ pub use super::ffi::*;
use rustc_index::{IndexSlice, IndexVec}; use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::bug; use rustc_middle::bug;
use rustc_middle::mir::coverage::{ use rustc_middle::mir::coverage::{
CodeRegion, CounterValueReference, ExpressionId, MappedExpressionIndex, Op, Operand, CodeRegion, CounterId, ExpressionId, MappedExpressionIndex, Op, Operand,
}; };
use rustc_middle::ty::Instance; use rustc_middle::ty::Instance;
use rustc_middle::ty::TyCtxt; use rustc_middle::ty::TyCtxt;
@ -32,7 +32,7 @@ pub struct FunctionCoverage<'tcx> {
instance: Instance<'tcx>, instance: Instance<'tcx>,
source_hash: u64, source_hash: u64,
is_used: bool, is_used: bool,
counters: IndexVec<CounterValueReference, Option<CodeRegion>>, counters: IndexVec<CounterId, Option<CodeRegion>>,
expressions: IndexVec<ExpressionId, Option<Expression>>, expressions: IndexVec<ExpressionId, Option<Expression>>,
unreachable_regions: Vec<CodeRegion>, unreachable_regions: Vec<CodeRegion>,
} }
@ -80,7 +80,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
} }
/// Adds a code region to be counted by an injected counter intrinsic. /// Adds a code region to be counted by an injected counter intrinsic.
pub fn add_counter(&mut self, id: CounterValueReference, region: CodeRegion) { pub fn add_counter(&mut self, id: CounterId, region: CodeRegion) {
if let Some(previous_region) = self.counters[id].replace(region.clone()) { if let Some(previous_region) = self.counters[id].replace(region.clone()) {
assert_eq!(previous_region, region, "add_counter: code region for id changed"); assert_eq!(previous_region, region, "add_counter: code region for id changed");
} }
@ -192,18 +192,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
type NewIndexes = IndexSlice<ExpressionId, Option<MappedExpressionIndex>>; type NewIndexes = IndexSlice<ExpressionId, Option<MappedExpressionIndex>>;
let id_to_counter = |new_indexes: &NewIndexes, operand: Operand| match operand { let id_to_counter = |new_indexes: &NewIndexes, operand: Operand| match operand {
Operand::Zero => Some(Counter::zero()), Operand::Zero => Some(Counter::zero()),
Operand::Counter(id) => { Operand::Counter(id) => Some(Counter::counter_value_reference(id)),
debug_assert!(
id.index() > 0,
"Operand::Counter indexes for counters are 1-based, but this id={}",
id.index()
);
// Note: Some codegen-injected Counters may be only referenced by `Expression`s,
// and may not have their own `CodeRegion`s,
let index = CounterValueReference::from(id.index());
// Note, the conversion to LLVM `Counter` adjusts the index to be zero-based.
Some(Counter::counter_value_reference(index))
}
Operand::Expression(id) => { Operand::Expression(id) => {
self.expressions self.expressions
.get(id) .get(id)

10
compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
View File

@ -16,9 +16,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::DefId; use rustc_hir::def_id::DefId;
use rustc_llvm::RustString; use rustc_llvm::RustString;
use rustc_middle::bug; use rustc_middle::bug;
use rustc_middle::mir::coverage::{ use rustc_middle::mir::coverage::{CodeRegion, CounterId, CoverageKind, ExpressionId, Op, Operand};
CodeRegion, CounterValueReference, CoverageKind, ExpressionId, Op, Operand,
};
use rustc_middle::mir::Coverage; use rustc_middle::mir::Coverage;
use rustc_middle::ty; use rustc_middle::ty;
use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt}; use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt};
@ -33,7 +31,7 @@ mod ffi;
pub(crate) mod map_data; pub(crate) mod map_data;
pub mod mapgen; pub mod mapgen;
const UNUSED_FUNCTION_COUNTER_ID: CounterValueReference = CounterValueReference::START; const UNUSED_FUNCTION_COUNTER_ID: CounterId = CounterId::START;
const VAR_ALIGN_BYTES: usize = 8; const VAR_ALIGN_BYTES: usize = 8;
@ -125,7 +123,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
let fn_name = bx.get_pgo_func_name_var(instance); let fn_name = bx.get_pgo_func_name_var(instance);
let hash = bx.const_u64(function_source_hash); let hash = bx.const_u64(function_source_hash);
let num_counters = bx.const_u32(coverageinfo.num_counters); let num_counters = bx.const_u32(coverageinfo.num_counters);
let index = bx.const_u32(id.zero_based_index()); let index = bx.const_u32(id.as_u32());
debug!( debug!(
"codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})", "codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})",
fn_name, hash, num_counters, index, fn_name, hash, num_counters, index,
@ -178,7 +176,7 @@ impl<'tcx> Builder<'_, '_, 'tcx> {
fn add_coverage_counter( fn add_coverage_counter(
&mut self, &mut self,
instance: Instance<'tcx>, instance: Instance<'tcx>,
id: CounterValueReference, id: CounterId,
region: CodeRegion, region: CodeRegion,
) -> bool { ) -> bool {
if let Some(coverage_context) = self.coverage_context() { if let Some(coverage_context) = self.coverage_context() {

26
compiler/rustc_middle/src/mir/coverage.rs
View File

@ -6,22 +6,22 @@ use rustc_span::Symbol;
use std::fmt::{self, Debug, Formatter}; use std::fmt::{self, Debug, Formatter};
rustc_index::newtype_index! { rustc_index::newtype_index! {
/// ID of a coverage counter. Values ascend from 0.
///
/// Note that LLVM handles counter IDs as `uint32_t`, so there is no need
/// to use a larger representation on the Rust side.
#[derive(HashStable)] #[derive(HashStable)]
#[max = 0xFFFF_FFFF] #[max = 0xFFFF_FFFF]
#[debug_format = "CounterValueReference({})"] #[debug_format = "CounterId({})"]
pub struct CounterValueReference {} pub struct CounterId {}
} }
impl CounterValueReference { impl CounterId {
/// Counters start at 1 for historical reasons. pub const START: Self = Self::from_u32(0);
pub const START: Self = Self::from_u32(1);
/// Returns explicitly-requested zero-based version of the counter id, used #[inline(always)]
/// during codegen. LLVM expects zero-based indexes. pub fn next_id(self) -> Self {
pub fn zero_based_index(self) -> u32 { Self::from_u32(self.as_u32() + 1)
let one_based_index = self.as_u32();
debug_assert!(one_based_index > 0);
one_based_index - 1
} }
} }
@ -63,7 +63,7 @@ rustc_index::newtype_index! {
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub enum Operand { pub enum Operand {
Zero, Zero,
Counter(CounterValueReference), Counter(CounterId),
Expression(ExpressionId), Expression(ExpressionId),
} }
@ -83,7 +83,7 @@ pub enum CoverageKind {
function_source_hash: u64, function_source_hash: u64,
/// ID of this counter within its enclosing function. /// ID of this counter within its enclosing function.
/// Expressions in the same function can refer to it as an operand. /// Expressions in the same function can refer to it as an operand.
id: CounterValueReference, id: CounterId,
}, },
Expression { Expression {
/// ID of this coverage-counter expression within its enclosing function. /// ID of this coverage-counter expression within its enclosing function.

2
compiler/rustc_middle/src/ty/structural_impls.rs
View File

@ -470,7 +470,7 @@ TrivialTypeTraversalAndLiftImpls! {
::rustc_hir::Unsafety, ::rustc_hir::Unsafety,
::rustc_target::asm::InlineAsmRegOrRegClass, ::rustc_target::asm::InlineAsmRegOrRegClass,
::rustc_target::spec::abi::Abi, ::rustc_target::spec::abi::Abi,
crate::mir::coverage::CounterValueReference, crate::mir::coverage::CounterId,
crate::mir::coverage::ExpressionId, crate::mir::coverage::ExpressionId,
crate::mir::coverage::MappedExpressionIndex, crate::mir::coverage::MappedExpressionIndex,
crate::mir::Local, crate::mir::Local,

10
compiler/rustc_mir_transform/src/coverage/counters.rs
View File

@ -16,7 +16,7 @@ use rustc_middle::mir::coverage::*;
/// `Coverage` statements. /// `Coverage` statements.
pub(super) struct CoverageCounters { pub(super) struct CoverageCounters {
function_source_hash: u64, function_source_hash: u64,
next_counter_id: u32, next_counter_id: CounterId,
next_expression_id: ExpressionId, next_expression_id: ExpressionId,
pub debug_counters: DebugCounters, pub debug_counters: DebugCounters,
} }
@ -25,7 +25,7 @@ impl CoverageCounters {
pub fn new(function_source_hash: u64) -> Self { pub fn new(function_source_hash: u64) -> Self {
Self { Self {
function_source_hash, function_source_hash,
next_counter_id: CounterValueReference::START.as_u32(), next_counter_id: CounterId::START,
next_expression_id: ExpressionId::START, next_expression_id: ExpressionId::START,
debug_counters: DebugCounters::new(), debug_counters: DebugCounters::new(),
} }
@ -93,10 +93,10 @@ impl CoverageCounters {
} }
/// Counter IDs start from one and go up. /// Counter IDs start from one and go up.
fn next_counter(&mut self) -> CounterValueReference { fn next_counter(&mut self) -> CounterId {
let next = self.next_counter_id; let next = self.next_counter_id;
self.next_counter_id += 1; self.next_counter_id = next.next_id();
CounterValueReference::from(next) next
} }
/// Expression IDs start from 0 and go up. /// Expression IDs start from 0 and go up.

9
compiler/rustc_mir_transform/src/coverage/query.rs
View File

@ -43,11 +43,9 @@ struct CoverageVisitor {
} }
impl CoverageVisitor { impl CoverageVisitor {
/// Updates `num_counters` to the maximum encountered zero-based counter_id plus 1. Note the /// Updates `num_counters` to the maximum encountered counter ID plus 1.
/// final computed number of counters should be the number of all `CoverageKind::Counter`
/// statements in the MIR *plus one* for the implicit `ZERO` counter.
#[inline(always)] #[inline(always)]
fn update_num_counters(&mut self, counter_id: CounterValueReference) { fn update_num_counters(&mut self, counter_id: CounterId) {
let counter_id = counter_id.as_u32(); let counter_id = counter_id.as_u32();
self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1);
} }
@ -103,8 +101,7 @@ fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) ->
let mir_body = tcx.instance_mir(instance_def); let mir_body = tcx.instance_mir(instance_def);
let mut coverage_visitor = CoverageVisitor { let mut coverage_visitor = CoverageVisitor {
// num_counters always has at least the `ZERO` counter. info: CoverageInfo { num_counters: 0, num_expressions: 0 },
info: CoverageInfo { num_counters: 1, num_expressions: 0 },
add_missing_operands: false, add_missing_operands: false,
}; };

4
compiler/rustc_mir_transform/src/coverage/tests.rs
View File

@ -683,7 +683,7 @@ fn test_make_bcb_counters() {
let_bcb!(1); let_bcb!(1);
assert_eq!( assert_eq!(
1, // coincidentally, bcb1 has a `Counter` with id = 1 0, // bcb1 has a `Counter` with id = 0
match basic_coverage_blocks[bcb1].counter().expect("should have a counter") { match basic_coverage_blocks[bcb1].counter().expect("should have a counter") {
CoverageKind::Counter { id, .. } => id, CoverageKind::Counter { id, .. } => id,
_ => panic!("expected a Counter"), _ => panic!("expected a Counter"),
@ -693,7 +693,7 @@ fn test_make_bcb_counters() {
let_bcb!(2); let_bcb!(2);
assert_eq!( assert_eq!(
2, // coincidentally, bcb2 has a `Counter` with id = 2 1, // bcb2 has a `Counter` with id = 1
match basic_coverage_blocks[bcb2].counter().expect("should have a counter") { match basic_coverage_blocks[bcb2].counter().expect("should have a counter") {
CoverageKind::Counter { id, .. } => id, CoverageKind::Counter { id, .. } => id,
_ => panic!("expected a Counter"), _ => panic!("expected a Counter"),

2
tests/mir-opt/coverage_graphviz.bar.InstrumentCoverage.0.dot
View File

@ -2,5 +2,5 @@ digraph Cov_0_4 {
graph [fontname="Courier, monospace"]; graph [fontname="Courier, monospace"];
node [fontname="Courier, monospace"]; node [fontname="Courier, monospace"];
edge [fontname="Courier, monospace"]; edge [fontname="Courier, monospace"];
bcb0__Cov_0_4 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 18:1-20:2<br align="left"/> 19:5-19:9: @0[0]: Coverage::Counter(1) for $DIR/coverage_graphviz.rs:18:1 - 20:2<br align="left"/> 20:2-20:2: @0.Return: return</td></tr><tr><td align="left" balign="left">bb0: Return</td></tr></table>>]; bcb0__Cov_0_4 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 18:1-20:2<br align="left"/> 19:5-19:9: @0[0]: Coverage::Counter(0) for $DIR/coverage_graphviz.rs:18:1 - 20:2<br align="left"/> 20:2-20:2: @0.Return: return</td></tr><tr><td align="left" balign="left">bb0: Return</td></tr></table>>];
} }

2
tests/mir-opt/coverage_graphviz.main.InstrumentCoverage.0.dot
View File

@ -2,7 +2,7 @@ digraph Cov_0_3 {
graph [fontname="Courier, monospace"]; graph [fontname="Courier, monospace"];
node [fontname="Courier, monospace"]; node [fontname="Courier, monospace"];
edge [fontname="Courier, monospace"]; edge [fontname="Courier, monospace"];
bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br align="left"/> 13:10-13:10: @5[0]: Coverage::Counter(2) for $DIR/coverage_graphviz.rs:13:10 - 13:11</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>]; bcb3__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb3</td></tr><tr><td align="left" balign="left">Counter(bcb3) at 13:10-13:10<br align="left"/> 13:10-13:10: @5[0]: Coverage::Counter(1) for $DIR/coverage_graphviz.rs:13:10 - 13:11</td></tr><tr><td align="left" balign="left">bb5: Goto</td></tr></table>>];
bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb1:(bcb0 + bcb3) - bcb3) at 12:13-12:18<br align="left"/> 12:13-12:18: @4[0]: Coverage::Expression(2) = Expression(1) + Zero for $DIR/coverage_graphviz.rs:15:1 - 15:2<br align="left"/>Expression(bcb2:(bcb1:(bcb0 + bcb3) - bcb3) + 0) at 15:2-15:2<br align="left"/> 15:2-15:2: @4.Return: return</td></tr><tr><td align="left" balign="left">bb4: Return</td></tr></table>>]; bcb2__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb2</td></tr><tr><td align="left" balign="left">Expression(bcb1:(bcb0 + bcb3) - bcb3) at 12:13-12:18<br align="left"/> 12:13-12:18: @4[0]: Coverage::Expression(2) = Expression(1) + Zero for $DIR/coverage_graphviz.rs:15:1 - 15:2<br align="left"/>Expression(bcb2:(bcb1:(bcb0 + bcb3) - bcb3) + 0) at 15:2-15:2<br align="left"/> 15:2-15:2: @4.Return: return</td></tr><tr><td align="left" balign="left">bb4: Return</td></tr></table>>];
bcb1__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb1</td></tr><tr><td align="left" balign="left">Expression(bcb0 + bcb3) at 10:5-11:17<br align="left"/> 11:12-11:17: @2.Call: _2 = bar() -&gt; [return: bb3, unwind: bb6]</td></tr><tr><td align="left" balign="left">bb1: FalseUnwind<br align="left"/>bb2: Call</td></tr><tr><td align="left" balign="left">bb3: SwitchInt</td></tr></table>>]; bcb1__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb1</td></tr><tr><td align="left" balign="left">Expression(bcb0 + bcb3) at 10:5-11:17<br align="left"/> 11:12-11:17: @2.Call: _2 = bar() -&gt; [return: bb3, unwind: bb6]</td></tr><tr><td align="left" balign="left">bb1: FalseUnwind<br align="left"/>bb2: Call</td></tr><tr><td align="left" balign="left">bb3: SwitchInt</td></tr></table>>];
bcb0__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 9:1-9:11<br align="left"/> </td></tr><tr><td align="left" balign="left">bb0: Goto</td></tr></table>>]; bcb0__Cov_0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">bcb0</td></tr><tr><td align="left" balign="left"></td></tr><tr><td align="left" balign="left">Counter(bcb0) at 9:1-9:11<br align="left"/> </td></tr><tr><td align="left" balign="left">bb0: Goto</td></tr></table>>];

2
tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff
View File

@ -5,7 +5,7 @@
let mut _0: bool; let mut _0: bool;
bb0: { bb0: {
+ Coverage::Counter(1) for /the/src/instrument_coverage.rs:20:1 - 22:2; + Coverage::Counter(0) for /the/src/instrument_coverage.rs:20:1 - 22:2;
_0 = const true; _0 = const true;
return; return;
} }

8
tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff
View File

@ -8,12 +8,12 @@
let mut _3: !; let mut _3: !;
bb0: { bb0: {
+ Coverage::Counter(1) for /the/src/instrument_coverage.rs:11:1 - 11:11; + Coverage::Counter(0) for /the/src/instrument_coverage.rs:11:1 - 11:11;
goto -> bb1; goto -> bb1;
} }
bb1: { bb1: {
+ Coverage::Expression(0) = Counter(1) + Counter(2) for /the/src/instrument_coverage.rs:12:5 - 13:17; + Coverage::Expression(0) = Counter(0) + Counter(1) for /the/src/instrument_coverage.rs:12:5 - 13:17;
falseUnwind -> [real: bb2, unwind: bb6]; falseUnwind -> [real: bb2, unwind: bb6];
} }
@ -28,14 +28,14 @@
bb4: { bb4: {
+ Coverage::Expression(2) = Expression(1) + Zero for /the/src/instrument_coverage.rs:17:1 - 17:2; + Coverage::Expression(2) = Expression(1) + Zero for /the/src/instrument_coverage.rs:17:1 - 17:2;
+ Coverage::Expression(1) = Expression(0) - Counter(2) for /the/src/instrument_coverage.rs:14:13 - 14:18; + Coverage::Expression(1) = Expression(0) - Counter(1) for /the/src/instrument_coverage.rs:14:13 - 14:18;
_0 = const (); _0 = const ();
StorageDead(_2); StorageDead(_2);
return; return;
} }
bb5: { bb5: {
+ Coverage::Counter(2) for /the/src/instrument_coverage.rs:15:10 - 15:11; + Coverage::Counter(1) for /the/src/instrument_coverage.rs:15:10 - 15:11;
_1 = const (); _1 = const ();
StorageDead(_2); StorageDead(_2);
goto -> bb1; goto -> bb1;