Skip to content

coverage: Prepare for upcoming changes to counter creation #135873

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 7 commits into from
Jan 24, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn prepare_covfun_record<'tcx>(
is_used: bool,
) -> Option<CovfunRecord<'tcx>> {
let fn_cov_info = tcx.instance_mir(instance.def).function_coverage_info.as_deref()?;
let ids_info = tcx.coverage_ids_info(instance.def);
let ids_info = tcx.coverage_ids_info(instance.def)?;

let expressions = prepare_expressions(fn_cov_info, ids_info, is_used);

Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use rustc_codegen_ssa::traits::{
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;
Expand Down Expand Up @@ -147,6 +146,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
debug!("function has a coverage statement but no coverage info");
return;
};
let Some(ids_info) = bx.tcx.coverage_ids_info(instance.def) else {
debug!("function has a coverage statement but no IDs info");
return;
};

// Mark the instance as used in this CGU, for coverage purposes.
// This includes functions that were not partitioned into this CGU,
Expand All @@ -162,8 +165,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
// be smaller than the number originally inserted by the instrumentor,
// if some high-numbered counters were removed by MIR optimizations.
// If so, LLVM's profiler runtime will use fewer physical counters.
let num_counters =
bx.tcx().coverage_ids_info(instance.def).num_counters_after_mir_opts();
let num_counters = ids_info.num_counters_after_mir_opts();
assert!(
num_counters as usize <= function_coverage_info.num_counters,
"num_counters disagreement: query says {num_counters} but function info only has {}",
Expand Down
28 changes: 14 additions & 14 deletions compiler/rustc_middle/src/mir/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::fmt::{self, Debug, Formatter};

use rustc_index::IndexVec;
use rustc_index::bit_set::DenseBitSet;
use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable};
use rustc_macros::{HashStable, TyDecodable, TyEncodable};
use rustc_span::Span;

rustc_index::newtype_index! {
Expand Down Expand Up @@ -72,7 +72,7 @@ impl ConditionId {
/// Enum that can hold a constant zero value, the ID of an physical coverage
/// counter, or the ID of a coverage-counter expression.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub enum CovTerm {
Zero,
Counter(CounterId),
Expand All @@ -89,7 +89,7 @@ impl Debug for CovTerm {
}
}

#[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]
pub enum CoverageKind {
/// Marks a span that might otherwise not be represented in MIR, so that
/// coverage instrumentation can associate it with its enclosing block/BCB.
Expand Down Expand Up @@ -151,7 +151,7 @@ impl Debug for CoverageKind {
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
#[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable)]
pub enum Op {
Subtract,
Add,
Expand All @@ -168,15 +168,15 @@ impl Op {
}

#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct Expression {
pub lhs: CovTerm,
pub op: Op,
pub rhs: CovTerm,
}

#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub enum MappingKind {
/// Associates a normal region of code with a counter/expression/zero.
Code(CovTerm),
Expand Down Expand Up @@ -208,7 +208,7 @@ impl MappingKind {
}

#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct Mapping {
pub kind: MappingKind,
pub span: Span,
Expand All @@ -218,7 +218,7 @@ pub struct Mapping {
/// to be used in conjunction with the individual coverage statements injected
/// into the function's basic blocks.
#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct FunctionCoverageInfo {
pub function_source_hash: u64,
pub body_span: Span,
Expand All @@ -238,7 +238,7 @@ pub struct FunctionCoverageInfo {
/// ("Hi" indicates that this is "high-level" information collected at the
/// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.)
#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct CoverageInfoHi {
/// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was
/// injected into the MIR body. This makes it possible to allocate per-ID
Expand All @@ -252,23 +252,23 @@ pub struct CoverageInfoHi {
}

#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct BranchSpan {
pub span: Span,
pub true_marker: BlockMarkerId,
pub false_marker: BlockMarkerId,
}

#[derive(Copy, Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct ConditionInfo {
pub condition_id: ConditionId,
pub true_next_id: Option<ConditionId>,
pub false_next_id: Option<ConditionId>,
}

#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct MCDCBranchSpan {
pub span: Span,
pub condition_info: ConditionInfo,
Expand All @@ -277,14 +277,14 @@ pub struct MCDCBranchSpan {
}

#[derive(Copy, Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct DecisionInfo {
pub bitmap_idx: u32,
pub num_conditions: u16,
}

#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct MCDCDecisionSpan {
pub span: Span,
pub end_markers: Vec<BlockMarkerId>,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,8 @@ pub struct Body<'tcx> {
///
/// Only present if coverage is enabled and this function is eligible.
/// Boxed to limit space overhead in non-coverage builds.
#[type_foldable(identity)]
#[type_visitable(ignore)]
pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>,

/// Per-function coverage information added by the `InstrumentCoverage`
Expand All @@ -366,6 +368,8 @@ pub struct Body<'tcx> {
///
/// If `-Cinstrument-coverage` is not active, or if an individual function
/// is not eligible for coverage, then this should always be `None`.
#[type_foldable(identity)]
#[type_visitable(ignore)]
pub function_coverage_info: Option<Box<coverage::FunctionCoverageInfo>>,
}

Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,14 @@ pub enum StatementKind<'tcx> {
///
/// Interpreters and codegen backends that don't support coverage instrumentation
/// can usually treat this as a no-op.
Coverage(CoverageKind),
Coverage(
// Coverage statements are unlikely to ever contain type information in
// the foreseeable future, so excluding them from TypeFoldable/TypeVisitable
// avoids some unhelpful derive boilerplate.
#[type_foldable(identity)]
#[type_visitable(ignore)]
CoverageKind,
),

/// Denotes a call to an intrinsic that does not require an unwind path and always returns.
/// This avoids adding a new block and a terminator for simple intrinsics.
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,9 @@ rustc_queries! {
/// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass
/// (for compiler option `-Cinstrument-coverage`), after MIR optimizations
/// have had a chance to potentially remove some of them.
query coverage_ids_info(key: ty::InstanceKind<'tcx>) -> &'tcx mir::coverage::CoverageIdsInfo {
///
/// Returns `None` for functions that were not instrumented.
query coverage_ids_info(key: ty::InstanceKind<'tcx>) -> Option<&'tcx mir::coverage::CoverageIdsInfo> {
desc { |tcx| "retrieving coverage IDs info from MIR for `{}`", tcx.def_path_str(key.def_id()) }
arena_cache
}
Expand Down
18 changes: 10 additions & 8 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId,

use crate::coverage::counters::balanced_flow::BalancedFlowGraph;
use crate::coverage::counters::iter_nodes::IterNodes;
use crate::coverage::counters::node_flow::{CounterTerm, MergedNodeFlowGraph, NodeCounters};
use crate::coverage::counters::node_flow::{
CounterTerm, NodeCounters, make_node_counters, node_flow_data_for_balanced_graph,
};
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};

mod balanced_flow;
Expand All @@ -27,20 +29,20 @@ pub(super) fn make_bcb_counters(
) -> CoverageCounters {
// Create the derived graphs that are necessary for subsequent steps.
let balanced_graph = BalancedFlowGraph::for_graph(graph, |n| !graph[n].is_out_summable);
let merged_graph = MergedNodeFlowGraph::for_balanced_graph(&balanced_graph);
let node_flow_data = node_flow_data_for_balanced_graph(&balanced_graph);

// Use those graphs to determine which nodes get physical counters, and how
// to compute the execution counts of other nodes from those counters.
let nodes = make_node_counter_priority_list(graph, balanced_graph);
let node_counters = merged_graph.make_node_counters(&nodes);
let priority_list = make_node_flow_priority_list(graph, balanced_graph);
let node_counters = make_node_counters(&node_flow_data, &priority_list);

// Convert the counters into a form suitable for embedding into MIR.
transcribe_counters(&node_counters, bcb_needs_counter)
}

/// Arranges the nodes in `balanced_graph` into a list, such that earlier nodes
/// take priority in being given a counter expression instead of a physical counter.
fn make_node_counter_priority_list(
fn make_node_flow_priority_list(
graph: &CoverageGraph,
balanced_graph: BalancedFlowGraph<&CoverageGraph>,
) -> Vec<BasicCoverageBlock> {
Expand Down Expand Up @@ -81,11 +83,11 @@ fn transcribe_counters(
let mut new = CoverageCounters::with_num_bcbs(bcb_needs_counter.domain_size());

for bcb in bcb_needs_counter.iter() {
// Our counter-creation algorithm doesn't guarantee that a counter
// expression starts or ends with a positive term, so partition the
// Our counter-creation algorithm doesn't guarantee that a node's list
// of terms starts or ends with a positive term, so partition the
// counters into "positive" and "negative" lists for easier handling.
let (mut pos, mut neg): (Vec<_>, Vec<_>) =
old.counter_expr(bcb).iter().partition_map(|&CounterTerm { node, op }| match op {
old.counter_terms[bcb].iter().partition_map(|&CounterTerm { node, op }| match op {
Op::Add => Either::Left(node),
Op::Subtract => Either::Right(node),
});
Expand Down
Loading
Loading