bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #115586 - Zalathar:query, r=cjgillot

Browse source 
coverage: Simplify the `coverageinfo` query

The `coverageinfo` query walks through a `mir::Body`'s statements to find the total number of coverage counter IDs and coverage expression IDs that have been used, as this information is needed by coverage codegen.

This PR makes 3 nice simplifications to that query:
- Extract a common iterator over coverage statements, shared by both coverage-related queries
- Simplify the query's visitor from two passes to just one pass
- Explicitly track the highest seen IDs in the visitor, and only convert to a count right at the end

I also updated some related comments. Some had been invalidated by these changes, while others had already been invalidated by previous coverage changes.
This commit is contained in:
bors 2023-09-08 02:24:55 +00:00
commit 69ec43001a
1 changed files with 51 additions and 72 deletions
Download patch file Download diff file Expand all files Collapse all files

123
compiler/rustc_mir_transform/src/coverage/query.rs
View file

@ -1,5 +1,6 @@
use super::*; use super::*;
use rustc_data_structures::captures::Captures;
use rustc_middle::mir::coverage::*; use rustc_middle::mir::coverage::*;
use rustc_middle::mir::{self, Body, Coverage, CoverageInfo}; use rustc_middle::mir::{self, Body, Coverage, CoverageInfo};
use rustc_middle::query::Providers; use rustc_middle::query::Providers;
@ -12,15 +13,10 @@ pub(crate) fn provide(providers: &mut Providers) {
providers.covered_code_regions = |tcx, def_id| covered_code_regions(tcx, def_id); providers.covered_code_regions = |tcx, def_id| covered_code_regions(tcx, def_id);
} }
/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in /// Coverage codegen needs to know the total number of counter IDs and expression IDs that have
/// other words, the number of counter value references injected into the MIR (plus 1 for the /// been used by a function's coverage mappings. These totals are used to create vectors to hold
/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected /// the relevant counter and expression data, and the maximum counter ID (+ 1) is also needed by
/// counters have a counter ID from `1..num_counters-1`. /// the `llvm.instrprof.increment` intrinsic.
///
/// `num_expressions` is the number of counter expressions added to the MIR body.
///
/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend
/// code generate, to lookup counters and expressions by simple u32 indexes.
/// ///
/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code /// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code
/// including injected counters. (It is OK if some counters are optimized out, but those counters /// including injected counters. (It is OK if some counters are optimized out, but those counters
@ -28,71 +24,51 @@ pub(crate) fn provide(providers: &mut Providers) {
/// calls may not work; but computing the number of counters or expressions by adding `1` to the /// calls may not work; but computing the number of counters or expressions by adding `1` to the
/// highest ID (for a given instrumented function) is valid. /// highest ID (for a given instrumented function) is valid.
/// ///
/// This visitor runs twice, first with `add_missing_operands` set to `false`, to find the maximum /// It's possible for a coverage expression to remain in MIR while one or both of its operands
/// counter ID and maximum expression ID based on their enum variant `id` fields; then, as a /// have been optimized away. To avoid problems in codegen, we include those operands' IDs when
/// safeguard, with `add_missing_operands` set to `true`, to find any other counter or expression /// determining the maximum counter/expression ID, even if the underlying counter/expression is
/// IDs referenced by expression operands, if not already seen. /// no longer present.
///
/// Ideally, each operand ID in a MIR `CoverageKind::Expression` will have a separate MIR `Coverage`
/// statement for the `Counter` or `Expression` with the referenced ID. but since current or future
/// MIR optimizations can theoretically optimize out segments of a MIR, it may not be possible to
/// guarantee this, so the second pass ensures the `CoverageInfo` counts include all referenced IDs.
struct CoverageVisitor { struct CoverageVisitor {
info: CoverageInfo, max_counter_id: CounterId,
add_missing_operands: bool, max_expression_id: ExpressionId,
} }
impl CoverageVisitor { impl CoverageVisitor {
/// Updates `num_counters` to the maximum encountered counter ID plus 1. /// Updates `max_counter_id` to the maximum encountered counter ID.
#[inline(always)] #[inline(always)]
fn update_num_counters(&mut self, counter_id: CounterId) { fn update_max_counter_id(&mut self, counter_id: CounterId) {
let counter_id = counter_id.as_u32(); self.max_counter_id = self.max_counter_id.max(counter_id);
self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1);
} }
/// Updates `num_expressions` to the maximum encountered expression ID plus 1. /// Updates `max_expression_id` to the maximum encountered expression ID.
#[inline(always)] #[inline(always)]
fn update_num_expressions(&mut self, expression_id: ExpressionId) { fn update_max_expression_id(&mut self, expression_id: ExpressionId) {
let expression_id = expression_id.as_u32(); self.max_expression_id = self.max_expression_id.max(expression_id);
self.info.num_expressions = std::cmp::max(self.info.num_expressions, expression_id + 1);
} }
fn update_from_expression_operand(&mut self, operand: Operand) { fn update_from_expression_operand(&mut self, operand: Operand) {
match operand { match operand {
Operand::Counter(id) => self.update_num_counters(id), Operand::Counter(id) => self.update_max_counter_id(id),
Operand::Expression(id) => self.update_num_expressions(id), Operand::Expression(id) => self.update_max_expression_id(id),
Operand::Zero => {} Operand::Zero => {}
} }
} }
fn visit_body(&mut self, body: &Body<'_>) { fn visit_body(&mut self, body: &Body<'_>) {
for bb_data in body.basic_blocks.iter() { for coverage in all_coverage_in_mir_body(body) {
for statement in bb_data.statements.iter() { self.visit_coverage(coverage);
if let StatementKind::Coverage(box ref coverage) = statement.kind {
if is_inlined(body, statement) {
continue;
}
self.visit_coverage(coverage);
}
}
} }
} }
fn visit_coverage(&mut self, coverage: &Coverage) { fn visit_coverage(&mut self, coverage: &Coverage) {
if self.add_missing_operands { match coverage.kind {
match coverage.kind { CoverageKind::Counter { id, .. } => self.update_max_counter_id(id),
CoverageKind::Expression { lhs, rhs, .. } => { CoverageKind::Expression { id, lhs, rhs, .. } => {
self.update_from_expression_operand(lhs); self.update_max_expression_id(id);
self.update_from_expression_operand(rhs); self.update_from_expression_operand(lhs);
} self.update_from_expression_operand(rhs);
_ => {}
}
} else {
match coverage.kind {
CoverageKind::Counter { id, .. } => self.update_num_counters(id),
CoverageKind::Expression { id, .. } => self.update_num_expressions(id),
_ => {}
} }
CoverageKind::Unreachable => {}
} }
} }
} }
@ -101,37 +77,40 @@ 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 {
info: CoverageInfo { num_counters: 0, num_expressions: 0 }, max_counter_id: CounterId::START,
add_missing_operands: false, max_expression_id: ExpressionId::START,
}; };
coverage_visitor.visit_body(mir_body); coverage_visitor.visit_body(mir_body);
coverage_visitor.add_missing_operands = true; // Add 1 to the highest IDs to get the total number of IDs.
coverage_visitor.visit_body(mir_body); CoverageInfo {
num_counters: (coverage_visitor.max_counter_id + 1).as_u32(),
coverage_visitor.info num_expressions: (coverage_visitor.max_expression_id + 1).as_u32(),
}
} }
fn covered_code_regions(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<&CodeRegion> { fn covered_code_regions(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<&CodeRegion> {
let body = mir_body(tcx, def_id); let body = mir_body(tcx, def_id);
body.basic_blocks all_coverage_in_mir_body(body)
.iter() // Not all coverage statements have an attached code region.
.flat_map(|data| { .filter_map(|coverage| coverage.code_region.as_ref())
data.statements.iter().filter_map(|statement| match statement.kind {
StatementKind::Coverage(box ref coverage) => {
if is_inlined(body, statement) {
None
} else {
coverage.code_region.as_ref() // may be None
}
}
_ => None,
})
})
.collect() .collect()
} }
fn all_coverage_in_mir_body<'a, 'tcx>(
body: &'a Body<'tcx>,
) -> impl Iterator<Item = &'a Coverage> + Captures<'tcx> {
body.basic_blocks.iter().flat_map(|bb_data| &bb_data.statements).filter_map(|statement| {
match statement.kind {
StatementKind::Coverage(box ref coverage) if !is_inlined(body, statement) => {
Some(coverage)
}
_ => None,
}
})
}
fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool {
let scope_data = &body.source_scopes[statement.source_info.scope]; let scope_data = &body.source_scopes[statement.source_info.scope];
scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some()