bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #135873 - Zalathar:be-prepared, r=oli-obk

Browse source 
coverage: Prepare for upcoming changes to counter creation

This is a collection of smaller changes to coverage instrumentation code that have been extracted from a larger PR that I'm still working on, in order to hopefully make review easier.

Each individual change should hopefully be mostly self-explanatory. One of the big goals of the upcoming PR will be to defer certain parts of counter-creation until codegen, via the query system, so that ends up being a recurring theme in these changes. Several of the changes are follow-ups to #135481.

There should be no observable change in compiler output.
This commit is contained in:
Matthias Krüger 2025-01-24 16:25:42 +01:00 • committed by GitHub
commit 135cd69b57
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 180 additions and 198 deletions
Download patch file Download diff file Expand all files Collapse all files

2
compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs
View file

@ -51,7 +51,7 @@ pub(crate) fn prepare_covfun_record<'tcx>(
is_used: bool, is_used: bool,
) -> Option<CovfunRecord<'tcx>> { ) -> Option<CovfunRecord<'tcx>> {
let fn_cov_info = tcx.instance_mir(instance.def).function_coverage_info.as_deref()?; 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); let expressions = prepare_expressions(fn_cov_info, ids_info, is_used);

8
compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
View file

@ -8,7 +8,6 @@ use rustc_codegen_ssa::traits::{
use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::ty::Instance; use rustc_middle::ty::Instance;
use rustc_middle::ty::layout::HasTyCtxt;
use tracing::{debug, instrument}; use tracing::{debug, instrument};
use crate::builder::Builder; use crate::builder::Builder;
@ -147,6 +146,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
debug!("function has a coverage statement but no coverage info"); debug!("function has a coverage statement but no coverage info");
return; 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. // Mark the instance as used in this CGU, for coverage purposes.
// This includes functions that were not partitioned into this CGU, // This includes functions that were not partitioned into this CGU,
@ -162,8 +165,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
// be smaller than the number originally inserted by the instrumentor, // be smaller than the number originally inserted by the instrumentor,
// if some high-numbered counters were removed by MIR optimizations. // if some high-numbered counters were removed by MIR optimizations.
// If so, LLVM's profiler runtime will use fewer physical counters. // If so, LLVM's profiler runtime will use fewer physical counters.
let num_counters = let num_counters = ids_info.num_counters_after_mir_opts();
bx.tcx().coverage_ids_info(instance.def).num_counters_after_mir_opts();
assert!( assert!(
num_counters as usize <= function_coverage_info.num_counters, num_counters as usize <= function_coverage_info.num_counters,
"num_counters disagreement: query says {num_counters} but function info only has {}", "num_counters disagreement: query says {num_counters} but function info only has {}",

28
compiler/rustc_middle/src/mir/coverage.rs
View file

@ -4,7 +4,7 @@ use std::fmt::{self, Debug, Formatter};
use rustc_index::IndexVec; use rustc_index::IndexVec;
use rustc_index::bit_set::DenseBitSet; use rustc_index::bit_set::DenseBitSet;
use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable}; use rustc_macros::{HashStable, TyDecodable, TyEncodable};
use rustc_span::Span; use rustc_span::Span;
rustc_index::newtype_index! { rustc_index::newtype_index! {
@ -72,7 +72,7 @@ impl ConditionId {
/// Enum that can hold a constant zero value, the ID of an physical coverage /// Enum that can hold a constant zero value, the ID of an physical coverage
/// counter, or the ID of a coverage-counter expression. /// counter, or the ID of a coverage-counter expression.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub enum CovTerm { pub enum CovTerm {
Zero, Zero,
Counter(CounterId), Counter(CounterId),
@ -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 { pub enum CoverageKind {
/// Marks a span that might otherwise not be represented in MIR, so that /// Marks a span that might otherwise not be represented in MIR, so that
/// coverage instrumentation can associate it with its enclosing block/BCB. /// coverage instrumentation can associate it with its enclosing block/BCB.
@ -151,7 +151,7 @@ impl Debug for CoverageKind {
} }
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)] #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
#[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable)]
pub enum Op { pub enum Op {
Subtract, Subtract,
Add, Add,
@ -168,7 +168,7 @@ impl Op {
} }
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct Expression { pub struct Expression {
pub lhs: CovTerm, pub lhs: CovTerm,
pub op: Op, pub op: Op,
@ -176,7 +176,7 @@ pub struct Expression {
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub enum MappingKind { pub enum MappingKind {
/// Associates a normal region of code with a counter/expression/zero. /// Associates a normal region of code with a counter/expression/zero.
Code(CovTerm), Code(CovTerm),
@ -208,7 +208,7 @@ impl MappingKind {
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct Mapping { pub struct Mapping {
pub kind: MappingKind, pub kind: MappingKind,
pub span: Span, pub span: Span,
@ -218,7 +218,7 @@ pub struct Mapping {
/// to be used in conjunction with the individual coverage statements injected /// to be used in conjunction with the individual coverage statements injected
/// into the function's basic blocks. /// into the function's basic blocks.
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct FunctionCoverageInfo { pub struct FunctionCoverageInfo {
pub function_source_hash: u64, pub function_source_hash: u64,
pub body_span: Span, pub body_span: Span,
@ -238,7 +238,7 @@ pub struct FunctionCoverageInfo {
/// ("Hi" indicates that this is "high-level" information collected at the /// ("Hi" indicates that this is "high-level" information collected at the
/// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.) /// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.)
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct CoverageInfoHi { pub struct CoverageInfoHi {
/// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was /// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was
/// injected into the MIR body. This makes it possible to allocate per-ID /// injected into the MIR body. This makes it possible to allocate per-ID
@ -252,7 +252,7 @@ pub struct CoverageInfoHi {
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct BranchSpan { pub struct BranchSpan {
pub span: Span, pub span: Span,
pub true_marker: BlockMarkerId, pub true_marker: BlockMarkerId,
@ -260,7 +260,7 @@ pub struct BranchSpan {
} }
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct ConditionInfo { pub struct ConditionInfo {
pub condition_id: ConditionId, pub condition_id: ConditionId,
pub true_next_id: Option<ConditionId>, pub true_next_id: Option<ConditionId>,
@ -268,7 +268,7 @@ pub struct ConditionInfo {
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct MCDCBranchSpan { pub struct MCDCBranchSpan {
pub span: Span, pub span: Span,
pub condition_info: ConditionInfo, pub condition_info: ConditionInfo,
@ -277,14 +277,14 @@ pub struct MCDCBranchSpan {
} }
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct DecisionInfo { pub struct DecisionInfo {
pub bitmap_idx: u32, pub bitmap_idx: u32,
pub num_conditions: u16, pub num_conditions: u16,
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable)]
pub struct MCDCDecisionSpan { pub struct MCDCDecisionSpan {
pub span: Span, pub span: Span,
pub end_markers: Vec<BlockMarkerId>, pub end_markers: Vec<BlockMarkerId>,

4
compiler/rustc_middle/src/mir/mod.rs
View file

@ -358,6 +358,8 @@ pub struct Body<'tcx> {
/// ///
/// Only present if coverage is enabled and this function is eligible. /// Only present if coverage is enabled and this function is eligible.
/// Boxed to limit space overhead in non-coverage builds. /// Boxed to limit space overhead in non-coverage builds.
#[type_foldable(identity)]
#[type_visitable(ignore)]
pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>, pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>,
/// Per-function coverage information added by the `InstrumentCoverage` /// Per-function coverage information added by the `InstrumentCoverage`
@ -366,6 +368,8 @@ pub struct Body<'tcx> {
/// ///
/// If `-Cinstrument-coverage` is not active, or if an individual function /// If `-Cinstrument-coverage` is not active, or if an individual function
/// is not eligible for coverage, then this should always be `None`. /// 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>>, pub function_coverage_info: Option<Box<coverage::FunctionCoverageInfo>>,
} }

9
compiler/rustc_middle/src/mir/syntax.rs
View file

@ -417,7 +417,14 @@ pub enum StatementKind<'tcx> {
/// ///
/// Interpreters and codegen backends that don't support coverage instrumentation /// Interpreters and codegen backends that don't support coverage instrumentation
/// can usually treat this as a no-op. /// 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. /// 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. /// This avoids adding a new block and a terminator for simple intrinsics.

4
compiler/rustc_middle/src/query/mod.rs
View file

@ -618,7 +618,9 @@ rustc_queries! {
/// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass /// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass
/// (for compiler option `-Cinstrument-coverage`), after MIR optimizations /// (for compiler option `-Cinstrument-coverage`), after MIR optimizations
/// have had a chance to potentially remove some of them. /// 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()) } desc { |tcx| "retrieving coverage IDs info from MIR for `{}`", tcx.def_path_str(key.def_id()) }
arena_cache arena_cache
} }

18
compiler/rustc_mir_transform/src/coverage/counters.rs
View file

@ -11,7 +11,9 @@ use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId,
use crate::coverage::counters::balanced_flow::BalancedFlowGraph; use crate::coverage::counters::balanced_flow::BalancedFlowGraph;
use crate::coverage::counters::iter_nodes::IterNodes; 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}; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
mod balanced_flow; mod balanced_flow;
@ -27,12 +29,12 @@ pub(super) fn make_bcb_counters(
) -> CoverageCounters { ) -> CoverageCounters {
// Create the derived graphs that are necessary for subsequent steps. // Create the derived graphs that are necessary for subsequent steps.
let balanced_graph = BalancedFlowGraph::for_graph(graph, |n| !graph[n].is_out_summable); 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 // Use those graphs to determine which nodes get physical counters, and how
// to compute the execution counts of other nodes from those counters. // to compute the execution counts of other nodes from those counters.
let nodes = make_node_counter_priority_list(graph, balanced_graph); let priority_list = make_node_flow_priority_list(graph, balanced_graph);
let node_counters = merged_graph.make_node_counters(&nodes); let node_counters = make_node_counters(&node_flow_data, &priority_list);
// Convert the counters into a form suitable for embedding into MIR. // Convert the counters into a form suitable for embedding into MIR.
transcribe_counters(&node_counters, bcb_needs_counter) transcribe_counters(&node_counters, bcb_needs_counter)
@ -40,7 +42,7 @@ pub(super) fn make_bcb_counters(
/// Arranges the nodes in `balanced_graph` into a list, such that earlier nodes /// 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. /// 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, graph: &CoverageGraph,
balanced_graph: BalancedFlowGraph<&CoverageGraph>, balanced_graph: BalancedFlowGraph<&CoverageGraph>,
) -> Vec<BasicCoverageBlock> { ) -> Vec<BasicCoverageBlock> {
@ -81,11 +83,11 @@ fn transcribe_counters(
let mut new = CoverageCounters::with_num_bcbs(bcb_needs_counter.domain_size()); let mut new = CoverageCounters::with_num_bcbs(bcb_needs_counter.domain_size());
for bcb in bcb_needs_counter.iter() { for bcb in bcb_needs_counter.iter() {
// Our counter-creation algorithm doesn't guarantee that a counter // Our counter-creation algorithm doesn't guarantee that a node's list
// expression starts or ends with a positive term, so partition the // of terms starts or ends with a positive term, so partition the
// counters into "positive" and "negative" lists for easier handling. // counters into "positive" and "negative" lists for easier handling.
let (mut pos, mut neg): (Vec<_>, Vec<_>) = 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::Add => Either::Left(node),
Op::Subtract => Either::Right(node), Op::Subtract => Either::Right(node),
}); });

178
compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs
View file

@ -8,18 +8,17 @@
use rustc_data_structures::graph; use rustc_data_structures::graph;
use rustc_index::bit_set::DenseBitSet; use rustc_index::bit_set::DenseBitSet;
use rustc_index::{Idx, IndexVec}; use rustc_index::{Idx, IndexSlice, IndexVec};
use rustc_middle::mir::coverage::Op; use rustc_middle::mir::coverage::Op;
use smallvec::SmallVec;
use crate::coverage::counters::iter_nodes::IterNodes; use crate::coverage::counters::iter_nodes::IterNodes;
use crate::coverage::counters::union_find::{FrozenUnionFind, UnionFind}; use crate::coverage::counters::union_find::UnionFind;
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
/// View of some underlying graph, in which each node's successors have been /// Data representing a view of some underlying graph, in which each node's
/// merged into a single "supernode". /// successors have been merged into a single "supernode".
/// ///
/// The resulting supernodes have no obvious meaning on their own. /// The resulting supernodes have no obvious meaning on their own.
/// However, merging successor nodes means that a node's out-edges can all /// However, merging successor nodes means that a node's out-edges can all
@ -30,10 +29,10 @@ mod tests;
/// in the merged graph, it becomes possible to analyze the original node flows /// in the merged graph, it becomes possible to analyze the original node flows
/// using techniques for analyzing edge flows. /// using techniques for analyzing edge flows.
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct MergedNodeFlowGraph<Node: Idx> { pub(crate) struct NodeFlowData<Node: Idx> {
/// Maps each node to the supernode that contains it, indicated by some /// Maps each node to the supernode that contains it, indicated by some
/// arbitrary "root" node that is part of that supernode. /// arbitrary "root" node that is part of that supernode.
supernodes: FrozenUnionFind<Node>, supernodes: IndexVec<Node, Node>,
/// For each node, stores the single supernode that all of its successors /// For each node, stores the single supernode that all of its successors
/// have been merged into. /// have been merged into.
/// ///
@ -42,84 +41,71 @@ pub(crate) struct MergedNodeFlowGraph<Node: Idx> {
succ_supernodes: IndexVec<Node, Node>, succ_supernodes: IndexVec<Node, Node>,
} }
impl<Node: Idx> MergedNodeFlowGraph<Node> { /// Creates a "merged" view of an underlying graph.
/// Creates a "merged" view of an underlying graph. ///
/// /// The given graph is assumed to have [“balanced flow”](balanced-flow),
/// The given graph is assumed to have [“balanced flow”](balanced-flow), /// though it does not necessarily have to be a `BalancedFlowGraph`.
/// though it does not necessarily have to be a `BalancedFlowGraph`. ///
/// /// [balanced-flow]: `crate::coverage::counters::balanced_flow::BalancedFlowGraph`.
/// [balanced-flow]: `crate::coverage::counters::balanced_flow::BalancedFlowGraph`. pub(crate) fn node_flow_data_for_balanced_graph<G>(graph: G) -> NodeFlowData<G::Node>
pub(crate) fn for_balanced_graph<G>(graph: G) -> Self where
where G: graph::Successors,
G: graph::DirectedGraph<Node = Node> + graph::Successors, {
{ let mut supernodes = UnionFind::<G::Node>::new(graph.num_nodes());
let mut supernodes = UnionFind::<G::Node>::new(graph.num_nodes());
// For each node, merge its successors into a single supernode, and // For each node, merge its successors into a single supernode, and
// arbitrarily choose one of those successors to represent all of them. // arbitrarily choose one of those successors to represent all of them.
let successors = graph let successors = graph
.iter_nodes() .iter_nodes()
.map(|node| { .map(|node| {
graph graph
.successors(node) .successors(node)
.reduce(|a, b| supernodes.unify(a, b)) .reduce(|a, b| supernodes.unify(a, b))
.expect("each node in a balanced graph must have at least one out-edge") .expect("each node in a balanced graph must have at least one out-edge")
}) })
.collect::<IndexVec<G::Node, G::Node>>(); .collect::<IndexVec<G::Node, G::Node>>();
// Now that unification is complete, freeze the supernode forest, // Now that unification is complete, take a snapshot of the supernode forest,
// and resolve each arbitrarily-chosen successor to its canonical root. // and resolve each arbitrarily-chosen successor to its canonical root.
// (This avoids having to explicitly resolve them later.) // (This avoids having to explicitly resolve them later.)
let supernodes = supernodes.freeze(); let supernodes = supernodes.snapshot();
let succ_supernodes = successors.into_iter().map(|succ| supernodes.find(succ)).collect(); let succ_supernodes = successors.into_iter().map(|succ| supernodes[succ]).collect();
Self { supernodes, succ_supernodes } NodeFlowData { supernodes, succ_supernodes }
}
/// Uses the graph information in `node_flow_data`, together with a given
/// permutation of all nodes in the graph, to create physical counters and
/// counter expressions for each node in the underlying graph.
///
/// The given list must contain exactly one copy of each node in the
/// underlying balanced-flow graph. The order of nodes is used as a hint to
/// influence counter allocation:
/// - Earlier nodes are more likely to receive counter expressions.
/// - Later nodes are more likely to receive physical counters.
pub(crate) fn make_node_counters<Node: Idx>(
node_flow_data: &NodeFlowData<Node>,
priority_list: &[Node],
) -> NodeCounters<Node> {
let mut builder = SpantreeBuilder::new(node_flow_data);
for &node in priority_list {
builder.visit_node(node);
} }
fn num_nodes(&self) -> usize { NodeCounters { counter_terms: builder.finish() }
self.succ_supernodes.len()
}
fn is_supernode(&self, node: Node) -> bool {
self.supernodes.find(node) == node
}
/// Using the information in this merged graph, together with a given
/// permutation of all nodes in the graph, to create physical counters and
/// counter expressions for each node in the underlying graph.
///
/// The given list must contain exactly one copy of each node in the
/// underlying balanced-flow graph. The order of nodes is used as a hint to
/// influence counter allocation:
/// - Earlier nodes are more likely to receive counter expressions.
/// - Later nodes are more likely to receive physical counters.
pub(crate) fn make_node_counters(&self, all_nodes_permutation: &[Node]) -> NodeCounters<Node> {
let mut builder = SpantreeBuilder::new(self);
for &node in all_nodes_permutation {
builder.visit_node(node);
}
NodeCounters { counter_exprs: builder.finish() }
}
} }
/// End result of allocating physical counters and counter expressions for the /// End result of allocating physical counters and counter expressions for the
/// nodes of a graph. /// nodes of a graph.
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct NodeCounters<Node: Idx> { pub(crate) struct NodeCounters<Node: Idx> {
counter_exprs: IndexVec<Node, CounterExprVec<Node>>,
}
impl<Node: Idx> NodeCounters<Node> {
/// For the given node, returns the finished list of terms that represent /// For the given node, returns the finished list of terms that represent
/// its physical counter or counter expression. Always non-empty. /// its physical counter or counter expression. Always non-empty.
/// ///
/// If a node was given a physical counter, its "expression" will contain /// If a node was given a physical counter, the term list will contain
/// that counter as its sole element. /// that counter as its sole element.
pub(crate) fn counter_expr(&self, this: Node) -> &[CounterTerm<Node>] { pub(crate) counter_terms: IndexVec<Node, Vec<CounterTerm<Node>>>,
self.counter_exprs[this].as_slice()
}
} }
#[derive(Debug)] #[derive(Debug)]
@ -146,12 +132,11 @@ pub(crate) struct CounterTerm<Node> {
pub(crate) node: Node, pub(crate) node: Node,
} }
/// Stores the list of counter terms that make up a node's counter expression.
type CounterExprVec<Node> = SmallVec<[CounterTerm<Node>; 2]>;
#[derive(Debug)] #[derive(Debug)]
struct SpantreeBuilder<'a, Node: Idx> { struct SpantreeBuilder<'a, Node: Idx> {
graph: &'a MergedNodeFlowGraph<Node>, supernodes: &'a IndexSlice<Node, Node>,
succ_supernodes: &'a IndexSlice<Node, Node>,
is_unvisited: DenseBitSet<Node>, is_unvisited: DenseBitSet<Node>,
/// Links supernodes to each other, gradually forming a spanning tree of /// Links supernodes to each other, gradually forming a spanning tree of
/// the merged-flow graph. /// the merged-flow graph.
@ -163,26 +148,32 @@ struct SpantreeBuilder<'a, Node: Idx> {
yank_buffer: Vec<Node>, yank_buffer: Vec<Node>,
/// An in-progress counter expression for each node. Each expression is /// An in-progress counter expression for each node. Each expression is
/// initially empty, and will be filled in as relevant nodes are visited. /// initially empty, and will be filled in as relevant nodes are visited.
counter_exprs: IndexVec<Node, CounterExprVec<Node>>, counter_terms: IndexVec<Node, Vec<CounterTerm<Node>>>,
} }
impl<'a, Node: Idx> SpantreeBuilder<'a, Node> { impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
fn new(graph: &'a MergedNodeFlowGraph<Node>) -> Self { fn new(node_flow_data: &'a NodeFlowData<Node>) -> Self {
let num_nodes = graph.num_nodes(); let NodeFlowData { supernodes, succ_supernodes } = node_flow_data;
let num_nodes = supernodes.len();
Self { Self {
graph, supernodes,
succ_supernodes,
is_unvisited: DenseBitSet::new_filled(num_nodes), is_unvisited: DenseBitSet::new_filled(num_nodes),
span_edges: IndexVec::from_fn_n(|_| None, num_nodes), span_edges: IndexVec::from_fn_n(|_| None, num_nodes),
yank_buffer: vec![], yank_buffer: vec![],
counter_exprs: IndexVec::from_fn_n(|_| SmallVec::new(), num_nodes), counter_terms: IndexVec::from_fn_n(|_| vec![], num_nodes),
} }
} }
fn is_supernode(&self, node: Node) -> bool {
self.supernodes[node] == node
}
/// Given a supernode, finds the supernode that is the "root" of its /// Given a supernode, finds the supernode that is the "root" of its
/// spantree component. Two nodes that have the same spantree root are /// spantree component. Two nodes that have the same spantree root are
/// connected in the spantree. /// connected in the spantree.
fn spantree_root(&self, this: Node) -> Node { fn spantree_root(&self, this: Node) -> Node {
debug_assert!(self.graph.is_supernode(this)); debug_assert!(self.is_supernode(this));
match self.span_edges[this] { match self.span_edges[this] {
None => this, None => this,
@ -193,7 +184,7 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
/// Rotates edges in the spantree so that `this` is the root of its /// Rotates edges in the spantree so that `this` is the root of its
/// spantree component. /// spantree component.
fn yank_to_spantree_root(&mut self, this: Node) { fn yank_to_spantree_root(&mut self, this: Node) {
debug_assert!(self.graph.is_supernode(this)); debug_assert!(self.is_supernode(this));
// The rotation is done iteratively, by first traversing from `this` to // The rotation is done iteratively, by first traversing from `this` to
// its root and storing the path in a buffer, and then traversing the // its root and storing the path in a buffer, and then traversing the
@ -235,12 +226,12 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
// Get the supernode containing `this`, and make it the root of its // Get the supernode containing `this`, and make it the root of its
// component of the spantree. // component of the spantree.
let this_supernode = self.graph.supernodes.find(this); let this_supernode = self.supernodes[this];
self.yank_to_spantree_root(this_supernode); self.yank_to_spantree_root(this_supernode);
// Get the supernode containing all of this's successors. // Get the supernode containing all of this's successors.
let succ_supernode = self.graph.succ_supernodes[this]; let succ_supernode = self.succ_supernodes[this];
debug_assert!(self.graph.is_supernode(succ_supernode)); debug_assert!(self.is_supernode(succ_supernode));
// If two supernodes are already connected in the spantree, they will // If two supernodes are already connected in the spantree, they will
// have the same spantree root. (Each supernode is connected to itself.) // have the same spantree root. (Each supernode is connected to itself.)
@ -268,8 +259,8 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
// `this_supernode`. // `this_supernode`.
// Instead of setting `this.measure = true` as in the original paper, // Instead of setting `this.measure = true` as in the original paper,
// we just add the node's ID to its own "expression". // we just add the node's ID to its own list of terms.
self.counter_exprs[this].push(CounterTerm { node: this, op: Op::Add }); self.counter_terms[this].push(CounterTerm { node: this, op: Op::Add });
// Walk the spantree from `this.successor` back to `this`. For each // Walk the spantree from `this.successor` back to `this`. For each
// spantree edge along the way, add this node's physical counter to // spantree edge along the way, add this node's physical counter to
@ -279,7 +270,7 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
let &SpantreeEdge { is_reversed, claiming_node, span_parent } = let &SpantreeEdge { is_reversed, claiming_node, span_parent } =
self.span_edges[curr].as_ref().unwrap(); self.span_edges[curr].as_ref().unwrap();
let op = if is_reversed { Op::Subtract } else { Op::Add }; let op = if is_reversed { Op::Subtract } else { Op::Add };
self.counter_exprs[claiming_node].push(CounterTerm { node: this, op }); self.counter_terms[claiming_node].push(CounterTerm { node: this, op });
curr = span_parent; curr = span_parent;
} }
@ -288,19 +279,20 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
/// Asserts that all nodes have been visited, and returns the computed /// Asserts that all nodes have been visited, and returns the computed
/// counter expressions (made up of physical counters) for each node. /// counter expressions (made up of physical counters) for each node.
fn finish(self) -> IndexVec<Node, CounterExprVec<Node>> { fn finish(self) -> IndexVec<Node, Vec<CounterTerm<Node>>> {
let Self { graph, is_unvisited, span_edges, yank_buffer: _, counter_exprs } = self; let Self { ref span_edges, ref is_unvisited, ref counter_terms, .. } = self;
assert!(is_unvisited.is_empty(), "some nodes were never visited: {is_unvisited:?}"); assert!(is_unvisited.is_empty(), "some nodes were never visited: {is_unvisited:?}");
debug_assert!( debug_assert!(
span_edges span_edges
.iter_enumerated() .iter_enumerated()
.all(|(node, span_edge)| { span_edge.is_some() <= graph.is_supernode(node) }), .all(|(node, span_edge)| { span_edge.is_some() <= self.is_supernode(node) }),
"only supernodes can have a span edge", "only supernodes can have a span edge",
); );
debug_assert!( debug_assert!(
counter_exprs.iter().all(|expr| !expr.is_empty()), counter_terms.iter().all(|terms| !terms.is_empty()),
"after visiting all nodes, every node should have a non-empty expression", "after visiting all nodes, every node should have at least one term",
); );
counter_exprs
self.counter_terms
} }
} }

20
compiler/rustc_mir_transform/src/coverage/counters/node_flow/tests.rs
View file

@ -4,10 +4,12 @@ use rustc_data_structures::graph::vec_graph::VecGraph;
use rustc_index::Idx; use rustc_index::Idx;
use rustc_middle::mir::coverage::Op; use rustc_middle::mir::coverage::Op;
use super::{CounterTerm, MergedNodeFlowGraph, NodeCounters}; use crate::coverage::counters::node_flow::{
CounterTerm, NodeCounters, NodeFlowData, make_node_counters, node_flow_data_for_balanced_graph,
};
fn merged_node_flow_graph<G: graph::Successors>(graph: G) -> MergedNodeFlowGraph<G::Node> { fn node_flow_data<G: graph::Successors>(graph: G) -> NodeFlowData<G::Node> {
MergedNodeFlowGraph::for_balanced_graph(graph) node_flow_data_for_balanced_graph(graph)
} }
fn make_graph<Node: Idx + Ord>(num_nodes: usize, edge_pairs: Vec<(Node, Node)>) -> VecGraph<Node> { fn make_graph<Node: Idx + Ord>(num_nodes: usize, edge_pairs: Vec<(Node, Node)>) -> VecGraph<Node> {
@ -30,8 +32,8 @@ fn example_driver() {
(4, 0), (4, 0),
]); ]);
let merged = merged_node_flow_graph(&graph); let node_flow_data = node_flow_data(&graph);
let counters = merged.make_node_counters(&[3, 1, 2, 0, 4]); let counters = make_node_counters(&node_flow_data, &[3, 1, 2, 0, 4]);
assert_eq!(format_counter_expressions(&counters), &[ assert_eq!(format_counter_expressions(&counters), &[
// (comment to force vertical formatting for clarity) // (comment to force vertical formatting for clarity)
@ -53,12 +55,12 @@ fn format_counter_expressions<Node: Idx>(counters: &NodeCounters<Node>) -> Vec<S
}; };
counters counters
.counter_exprs .counter_terms
.indices() .indices()
.map(|node| { .map(|node| {
let mut expr = counters.counter_expr(node).iter().collect::<Vec<_>>(); let mut terms = counters.counter_terms[node].iter().collect::<Vec<_>>();
expr.sort_by_key(|item| item.node.index()); terms.sort_by_key(|item| item.node.index());
format!("[{node:?}]: {}", expr.into_iter().map(format_item).join(" ")) format!("[{node:?}]: {}", terms.into_iter().map(format_item).join(" "))
}) })
.collect() .collect()
} }

28
compiler/rustc_mir_transform/src/coverage/counters/union_find.rs
View file

@ -88,29 +88,9 @@ impl<Key: Idx> UnionFind<Key> {
a a
} }
/// Creates a snapshot of this disjoint-set forest that can no longer be /// Takes a "snapshot" of the current state of this disjoint-set forest, in
/// mutated, but can be queried without mutation. /// the form of a vector that directly maps each key to its current root.
pub(crate) fn freeze(&mut self) -> FrozenUnionFind<Key> { pub(crate) fn snapshot(&mut self) -> IndexVec<Key, Key> {
// Just resolve each key to its actual root. self.table.indices().map(|key| self.find(key)).collect()
let roots = self.table.indices().map(|key| self.find(key)).collect();
FrozenUnionFind { roots }
}
}
/// Snapshot of a disjoint-set forest that can no longer be mutated, but can be
/// queried in O(1) time without mutation.
///
/// This is really just a wrapper around a direct mapping from keys to roots,
/// but with a [`Self::find`] method that resembles [`UnionFind::find`].
#[derive(Debug)]
pub(crate) struct FrozenUnionFind<Key: Idx> {
roots: IndexVec<Key, Key>,
}
impl<Key: Idx> FrozenUnionFind<Key> {
/// Returns the "root" key of the disjoint-set containing the given key.
/// If two keys have the same root, they belong to the same set.
pub(crate) fn find(&self, key: Key) -> Key {
self.roots[key]
} }
} }

32
compiler/rustc_mir_transform/src/coverage/mod.rs
View file

@ -180,7 +180,12 @@ fn create_mappings(
)); ));
for (decision, branches) in mcdc_mappings { for (decision, branches) in mcdc_mappings {
let num_conditions = branches.len() as u16; // FIXME(#134497): Previously it was possible for some of these branch
// conversions to fail, in which case the remaining branches in the
// decision would be degraded to plain `MappingKind::Branch`.
// The changes in #134497 made that failure impossible, because the
// fallible step was deferred to codegen. But the corresponding code
// in codegen wasn't updated to detect the need for a degrade step.
let conditions = branches let conditions = branches
.into_iter() .into_iter()
.map( .map(
@ -206,24 +211,13 @@ fn create_mappings(
) )
.collect::<Vec<_>>(); .collect::<Vec<_>>();
if conditions.len() == num_conditions as usize { // LLVM requires end index for counter mapping regions.
// LLVM requires end index for counter mapping regions. let kind = MappingKind::MCDCDecision(DecisionInfo {
let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32,
bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32, num_conditions: u16::try_from(conditions.len()).unwrap(),
num_conditions, });
}); let span = decision.span;
let span = decision.span; mappings.extend(std::iter::once(Mapping { kind, span }).chain(conditions.into_iter()));
mappings.extend(std::iter::once(Mapping { kind, span }).chain(conditions.into_iter()));
} else {
mappings.extend(conditions.into_iter().map(|mapping| {
let MappingKind::MCDCBranch { true_term, false_term, mcdc_params: _ } =
mapping.kind
else {
unreachable!("all mappings here are MCDCBranch as shown above");
};
Mapping { kind: MappingKind::Branch { true_term, false_term }, span: mapping.span }
}))
}
} }
mappings mappings

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

@ -87,15 +87,9 @@ fn coverage_attr_on(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
fn coverage_ids_info<'tcx>( fn coverage_ids_info<'tcx>(
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
instance_def: ty::InstanceKind<'tcx>, instance_def: ty::InstanceKind<'tcx>,
) -> CoverageIdsInfo { ) -> Option<CoverageIdsInfo> {
let mir_body = tcx.instance_mir(instance_def); let mir_body = tcx.instance_mir(instance_def);
let fn_cov_info = mir_body.function_coverage_info.as_deref()?;
let Some(fn_cov_info) = mir_body.function_coverage_info.as_deref() else {
return CoverageIdsInfo {
counters_seen: DenseBitSet::new_empty(0),
zero_expressions: DenseBitSet::new_empty(0),
};
};
let mut counters_seen = DenseBitSet::new_empty(fn_cov_info.num_counters); let mut counters_seen = DenseBitSet::new_empty(fn_cov_info.num_counters);
let mut expressions_seen = DenseBitSet::new_filled(fn_cov_info.expressions.len()); let mut expressions_seen = DenseBitSet::new_filled(fn_cov_info.expressions.len());
@ -129,7 +123,7 @@ fn coverage_ids_info<'tcx>(
let zero_expressions = let zero_expressions =
identify_zero_expressions(fn_cov_info, &counters_seen, &expressions_seen); identify_zero_expressions(fn_cov_info, &counters_seen, &expressions_seen);
CoverageIdsInfo { counters_seen, zero_expressions } Some(CoverageIdsInfo { counters_seen, zero_expressions })
} }
fn all_coverage_in_mir_body<'a, 'tcx>( fn all_coverage_in_mir_body<'a, 'tcx>(

4
tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff
View file

@ -4,8 +4,8 @@
fn bar() -> bool { fn bar() -> bool {
let mut _0: bool; let mut _0: bool;
+ coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0) + coverage body span: $DIR/instrument_coverage.rs:29:18: 31:2 (#0)
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1: 21:2 (#0); + coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:29:1: 31:2 (#0);
+ +
bb0: { bb0: {
+ Coverage::CounterIncrement(0); + Coverage::CounterIncrement(0);

12
tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff
View file

@ -7,13 +7,13 @@
let mut _2: bool; let mut _2: bool;
let mut _3: !; let mut _3: !;
+ coverage body span: $DIR/instrument_coverage.rs:10:11: 16:2 (#0) + coverage body span: $DIR/instrument_coverage.rs:14:11: 20:2 (#0)
+ coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Subtract, rhs: Counter(0) }; + coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Subtract, rhs: Counter(0) };
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1: 10:11 (#0); + coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:14:1: 14:11 (#0);
+ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:12:12: 12:17 (#0); + coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:16:12: 16:17 (#0);
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13: 13:18 (#0); + coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:17:13: 17:18 (#0);
+ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:14:10: 14:10 (#0); + coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:18:10: 18:10 (#0);
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:2: 16:2 (#0); + coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:20:2: 20:2 (#0);
+ +
bb0: { bb0: {
+ Coverage::CounterIncrement(0); + Coverage::CounterIncrement(0);

19
tests/mir-opt/coverage/instrument_coverage.rs
View file

@ -6,7 +6,11 @@
//@ compile-flags: -Cinstrument-coverage -Zno-profiler-runtime //@ compile-flags: -Cinstrument-coverage -Zno-profiler-runtime
// EMIT_MIR instrument_coverage.main.InstrumentCoverage.diff // EMIT_MIR instrument_coverage.main.InstrumentCoverage.diff
// EMIT_MIR instrument_coverage.bar.InstrumentCoverage.diff // CHECK-LABEL: fn main()
// CHECK: coverage body span:
// CHECK: coverage Code(Counter({{[0-9]+}})) =>
// CHECK: bb0:
// CHECK: Coverage::CounterIncrement
fn main() { fn main() {
loop { loop {
if bar() { if bar() {
@ -15,14 +19,13 @@ fn main() {
} }
} }
// EMIT_MIR instrument_coverage.bar.InstrumentCoverage.diff
// CHECK-LABEL: fn bar()
// CHECK: coverage body span:
// CHECK: coverage Code(Counter({{[0-9]+}})) =>
// CHECK: bb0:
// CHECK: Coverage::CounterIncrement
#[inline(never)] #[inline(never)]
fn bar() -> bool { fn bar() -> bool {
true true
} }
// CHECK: coverage ExpressionId({{[0-9]+}}) =>
// CHECK-DAG: coverage Code(Counter({{[0-9]+}})) =>
// CHECK-DAG: coverage Code(Expression({{[0-9]+}})) =>
// CHECK: bb0:
// CHECK-DAG: Coverage::ExpressionUsed({{[0-9]+}})
// CHECK-DAG: Coverage::CounterIncrement({{[0-9]+}})