diff options
| author | Zalathar <Zalathar@users.noreply.github.com> | 2025-01-22 13:55:08 +1100 |
|---|---|---|
| committer | Zalathar <Zalathar@users.noreply.github.com> | 2025-02-06 21:44:31 +1100 |
| commit | 20d051ec870739c8f263e5f6f581ca24a5dd56fd (patch) | |
| tree | ec3355dad17588460d8ba1e0dd45160b97c13117 /compiler | |
| parent | ee7dc06cf181c073b1040669a40bc325d00f8c6d (diff) | |
| download | rust-20d051ec870739c8f263e5f6f581ca24a5dd56fd.tar.gz rust-20d051ec870739c8f263e5f6f581ca24a5dd56fd.zip | |
coverage: Defer part of counter-creation until codegen
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs | 13 | ||||
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs | 19 | ||||
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/lib.rs | 1 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/coverage.rs | 70 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/pretty.rs | 6 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/query/mod.rs | 13 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/counters.rs | 69 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs | 25 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/mappings.rs | 61 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/mod.rs | 70 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/query.rs | 94 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs | 3 |
12 files changed, 173 insertions, 271 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 38e7f4f21d4..1290c6efb09 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -54,7 +54,7 @@ pub(crate) fn prepare_covfun_record<'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 expressions = prepare_expressions(fn_cov_info, ids_info, is_used); + let expressions = prepare_expressions(ids_info, is_used); let mut covfun = CovfunRecord { mangled_function_name: tcx.symbol_name(instance).name, @@ -76,11 +76,7 @@ pub(crate) fn prepare_covfun_record<'tcx>( } /// Convert the function's coverage-counter expressions into a form suitable for FFI. -fn prepare_expressions( - fn_cov_info: &FunctionCoverageInfo, - ids_info: &CoverageIdsInfo, - is_used: bool, -) -> Vec<ffi::CounterExpression> { +fn prepare_expressions(ids_info: &CoverageIdsInfo, is_used: bool) -> Vec<ffi::CounterExpression> { // If any counters or expressions were removed by MIR opts, replace their // terms with zero. let counter_for_term = |term| { @@ -95,7 +91,7 @@ fn prepare_expressions( // producing the final coverage map, so there's no need to do the same // thing on the Rust side unless we're confident we can do much better. // (See `CounterExpressionsMinimizer` in `CoverageMappingWriter.cpp`.) - fn_cov_info + ids_info .expressions .iter() .map(move |&Expression { lhs, op, rhs }| ffi::CounterExpression { @@ -142,8 +138,7 @@ fn fill_region_tables<'tcx>( // If the mapping refers to counters/expressions that were removed by // MIR opts, replace those occurrences with zero. let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter { - let term = - fn_cov_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term"); + let term = ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term"); let term = if is_zero_term(term) { CovTerm::Zero } else { term }; ffi::Counter::from_term(term) }; diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 021108cd51c..e404db5e196 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -160,17 +160,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!( "marker statement {kind:?} should have been removed by CleanupPostBorrowck" ), - CoverageKind::CounterIncrement { id } => { - // The number of counters passed to `llvm.instrprof.increment` might - // 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. + CoverageKind::VirtualCounter { bcb } + if let Some(&id) = ids_info.phys_counter_for_node.get(&bcb) => + { 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 {}", - function_coverage_info.num_counters - ); let fn_name = bx.get_pgo_func_name_var(instance); let hash = bx.const_u64(function_coverage_info.function_source_hash); @@ -182,10 +175,8 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { ); bx.instrprof_increment(fn_name, hash, num_counters, index); } - CoverageKind::ExpressionUsed { id: _ } => { - // Expression-used statements are markers that are handled by - // `coverage_ids_info`, so there's nothing to codegen here. - } + // If a BCB doesn't have an associated physical counter, there's nothing to codegen. + CoverageKind::VirtualCounter { .. } => {} CoverageKind::CondBitmapUpdate { index, decision_depth } => { let cond_bitmap = coverage_cx .try_get_mcdc_condition_bitmap(&instance, decision_depth) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 14346795fda..f0d04b2b644 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -13,6 +13,7 @@ #![feature(extern_types)] #![feature(file_buffered)] #![feature(hash_raw_entry)] +#![feature(if_let_guard)] #![feature(impl_trait_in_assoc_type)] #![feature(iter_intersperse)] #![feature(let_chains)] diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 8123707dc73..31b9eeb715b 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -2,8 +2,9 @@ use std::fmt::{self, Debug, Formatter}; -use rustc_index::IndexVec; +use rustc_data_structures::fx::FxIndexMap; use rustc_index::bit_set::DenseBitSet; +use rustc_index::{Idx, IndexVec}; use rustc_macros::{HashStable, TyDecodable, TyEncodable}; use rustc_span::Span; @@ -103,23 +104,12 @@ pub enum CoverageKind { /// Should be erased before codegen (at some point after `InstrumentCoverage`). BlockMarker { id: BlockMarkerId }, - /// Marks the point in MIR control flow represented by a coverage counter. + /// Marks its enclosing basic block with the ID of the coverage graph node + /// that it was part of during the `InstrumentCoverage` MIR pass. /// - /// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR. - /// - /// If this statement does not survive MIR optimizations, any mappings that - /// refer to this counter can have those references simplified to zero. - CounterIncrement { id: CounterId }, - - /// Marks the point in MIR control-flow represented by a coverage expression. - /// - /// If this statement does not survive MIR optimizations, any mappings that - /// refer to this expression can have those references simplified to zero. - /// - /// (This is only inserted for expression IDs that are directly used by - /// mappings. Intermediate expressions with no direct mappings are - /// retained/zeroed based on whether they are transitively used.) - ExpressionUsed { id: ExpressionId }, + /// During codegen, this might be lowered to `llvm.instrprof.increment` or + /// to a no-op, depending on the outcome of counter-creation. + VirtualCounter { bcb: BasicCoverageBlock }, /// Marks the point in MIR control flow represented by a evaluated condition. /// @@ -138,8 +128,7 @@ impl Debug for CoverageKind { match self { SpanMarker => write!(fmt, "SpanMarker"), BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()), - CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()), - ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()), + VirtualCounter { bcb } => write!(fmt, "VirtualCounter({bcb:?})"), CondBitmapUpdate { index, decision_depth } => { write!(fmt, "CondBitmapUpdate(index={:?}, depth={:?})", index, decision_depth) } @@ -208,11 +197,12 @@ pub struct FunctionCoverageInfo { pub function_source_hash: u64, pub body_span: Span, - pub num_counters: usize, - pub expressions: IndexVec<ExpressionId, Expression>, + /// Used in conjunction with `priority_list` to create physical counters + /// and counter expressions, after MIR optimizations. + pub node_flow_data: NodeFlowData<BasicCoverageBlock>, + pub priority_list: Vec<BasicCoverageBlock>, pub mappings: Vec<Mapping>, - pub term_for_bcb: IndexVec<BasicCoverageBlock, Option<CovTerm>>, pub mcdc_bitmap_bits: usize, /// The depth of the deepest decision is used to know how many @@ -281,15 +271,18 @@ pub struct MCDCDecisionSpan { pub num_conditions: usize, } -/// 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. +/// Contains information needed during codegen, obtained by inspecting the +/// function's MIR after MIR optimizations. /// -/// Used by the `coverage_ids_info` query. +/// Returned by the `coverage_ids_info` query. #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)] pub struct CoverageIdsInfo { pub counters_seen: DenseBitSet<CounterId>, pub zero_expressions: DenseBitSet<ExpressionId>, + + pub phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, + pub term_for_bcb: IndexVec<BasicCoverageBlock, Option<CovTerm>>, + pub expressions: IndexVec<ExpressionId, Expression>, } impl CoverageIdsInfo { @@ -334,3 +327,28 @@ rustc_index::newtype_index! { const START_BCB = 0; } } + +/// Data representing a view of some underlying graph, in which each node's +/// successors have been merged into a single "supernode". +/// +/// The resulting supernodes have no obvious meaning on their own. +/// However, merging successor nodes means that a node's out-edges can all +/// be combined into a single out-edge, whose flow is the same as the flow +/// (execution count) of its corresponding node in the original graph. +/// +/// With all node flows now in the original graph now represented as edge flows +/// in the merged graph, it becomes possible to analyze the original node flows +/// using techniques for analyzing edge flows. +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable)] +pub struct NodeFlowData<Node: Idx> { + /// Maps each node to the supernode that contains it, indicated by some + /// arbitrary "root" node that is part of that supernode. + pub supernodes: IndexVec<Node, Node>, + /// For each node, stores the single supernode that all of its successors + /// have been merged into. + /// + /// (Note that each node in a supernode can potentially have a _different_ + /// successor supernode from its peers.) + pub succ_supernodes: IndexVec<Node, Node>, +} diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 3007b787496..11ebbbe807d 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -619,13 +619,9 @@ fn write_function_coverage_info( function_coverage_info: &coverage::FunctionCoverageInfo, w: &mut dyn io::Write, ) -> io::Result<()> { - let coverage::FunctionCoverageInfo { body_span, expressions, mappings, .. } = - function_coverage_info; + let coverage::FunctionCoverageInfo { body_span, mappings, .. } = function_coverage_info; writeln!(w, "{INDENT}coverage body span: {body_span:?}")?; - for (id, expression) in expressions.iter_enumerated() { - writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?; - } for coverage::Mapping { kind, span } in mappings { writeln!(w, "{INDENT}coverage {kind:?} => {span:?};")?; } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 6c442fc1047..228f94a17f8 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -615,9 +615,16 @@ rustc_queries! { feedable } - /// 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. + /// Scans through a function's MIR after MIR optimizations, to prepare the + /// information needed by codegen when `-Cinstrument-coverage` is active. + /// + /// This includes the details of where to insert `llvm.instrprof.increment` + /// intrinsics, and the expression tables to be embedded in the function's + /// coverage metadata. + /// + /// FIXME(Zalathar): This query's purpose has drifted a bit and should + /// probably be renamed, but that can wait until after the potential + /// follow-ups to #136053 have settled down. /// /// Returns `None` for functions that were not instrumented. query coverage_ids_info(key: ty::InstanceKind<'tcx>) -> Option<&'tcx mir::coverage::CoverageIdsInfo> { diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 50d5bf31829..fa4f80f827b 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -2,7 +2,6 @@ use std::cmp::Ordering; use either::Either; use itertools::Itertools; -use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_data_structures::graph::DirectedGraph; use rustc_index::IndexVec; @@ -11,31 +10,35 @@ use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, use crate::coverage::counters::balanced_flow::BalancedFlowGraph; use crate::coverage::counters::node_flow::{ - CounterTerm, NodeCounters, make_node_counters, node_flow_data_for_balanced_graph, + CounterTerm, NodeCounters, NodeFlowData, node_flow_data_for_balanced_graph, }; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; mod balanced_flow; -mod node_flow; +pub(crate) mod node_flow; mod union_find; -/// Ensures that each BCB node needing a counter has one, by creating physical -/// counters or counter expressions for nodes as required. -pub(super) fn make_bcb_counters( - graph: &CoverageGraph, - bcb_needs_counter: &DenseBitSet<BasicCoverageBlock>, -) -> CoverageCounters { +/// Struct containing the results of [`prepare_bcb_counters_data`]. +pub(crate) struct BcbCountersData { + pub(crate) node_flow_data: NodeFlowData<BasicCoverageBlock>, + pub(crate) priority_list: Vec<BasicCoverageBlock>, +} + +/// Analyzes the coverage graph to create intermediate data structures that +/// will later be used (during codegen) to create physical counters or counter +/// expressions for each BCB node that needs one. +pub(crate) fn prepare_bcb_counters_data(graph: &CoverageGraph) -> BcbCountersData { // Create the derived graphs that are necessary for subsequent steps. let balanced_graph = BalancedFlowGraph::for_graph(graph, |n| !graph[n].is_out_summable); 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. + // Also create a "priority list" of coverage graph nodes, to help determine + // which ones get physical counters or counter expressions. This needs to + // be done now, because the later parts of the counter-creation process + // won't have access to the original coverage graph. 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) + BcbCountersData { node_flow_data, priority_list } } /// Arranges the nodes in `balanced_graph` into a list, such that earlier nodes @@ -74,7 +77,7 @@ fn make_node_flow_priority_list( } // Converts node counters into a form suitable for embedding into MIR. -fn transcribe_counters( +pub(crate) fn transcribe_counters( old: &NodeCounters<BasicCoverageBlock>, bcb_needs_counter: &DenseBitSet<BasicCoverageBlock>, ) -> CoverageCounters { @@ -129,7 +132,7 @@ fn transcribe_counters( pub(super) struct CoverageCounters { /// List of places where a counter-increment statement should be injected /// into MIR, each with its corresponding counter ID. - phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, + pub(crate) phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, next_counter_id: CounterId, /// Coverage counters/expressions that are associated with individual BCBs. @@ -137,7 +140,7 @@ pub(super) struct CoverageCounters { /// Table of expression data, associating each expression ID with its /// corresponding operator (+ or -) and its LHS/RHS operands. - expressions: IndexVec<ExpressionId, Expression>, + pub(crate) expressions: IndexVec<ExpressionId, Expression>, /// Remember expressions that have already been created (or simplified), /// so that we don't create unnecessary duplicates. expressions_memo: FxHashMap<Expression, CovTerm>, @@ -188,12 +191,6 @@ impl CoverageCounters { self.make_expression(lhs, Op::Subtract, rhs_sum) } - pub(super) fn num_counters(&self) -> usize { - let num_counters = self.phys_counter_for_node.len(); - assert_eq!(num_counters, self.next_counter_id.as_usize()); - num_counters - } - fn set_node_counter(&mut self, bcb: BasicCoverageBlock, counter: CovTerm) -> CovTerm { let existing = self.node_counters[bcb].replace(counter); assert!( @@ -202,30 +199,4 @@ impl CoverageCounters { ); counter } - - /// Returns an iterator over all the nodes in the coverage graph that - /// should have a counter-increment statement injected into MIR, along with - /// each site's corresponding counter ID. - pub(super) fn counter_increment_sites( - &self, - ) -> impl Iterator<Item = (CounterId, BasicCoverageBlock)> + Captures<'_> { - self.phys_counter_for_node.iter().map(|(&site, &id)| (id, site)) - } - - /// Returns an iterator over the subset of BCB nodes that have been associated - /// with a counter *expression*, along with the ID of that expression. - pub(super) fn bcb_nodes_with_coverage_expressions( - &self, - ) -> impl Iterator<Item = (BasicCoverageBlock, ExpressionId)> + Captures<'_> { - self.node_counters.iter_enumerated().filter_map(|(bcb, &counter)| match counter { - // Yield the BCB along with its associated expression ID. - Some(CovTerm::Expression(id)) => Some((bcb, id)), - // This BCB is associated with a counter or nothing, so skip it. - Some(CovTerm::Counter { .. } | CovTerm::Zero) | None => None, - }) - } - - pub(super) fn into_expressions(self) -> IndexVec<ExpressionId, Expression> { - self.expressions - } } diff --git a/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs b/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs index 9d80b3af42d..91ed54b8b59 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs @@ -9,6 +9,7 @@ use rustc_data_structures::graph; use rustc_index::bit_set::DenseBitSet; use rustc_index::{Idx, IndexSlice, IndexVec}; +pub(crate) use rustc_middle::mir::coverage::NodeFlowData; use rustc_middle::mir::coverage::Op; use crate::coverage::counters::union_find::UnionFind; @@ -16,30 +17,6 @@ use crate::coverage::counters::union_find::UnionFind; #[cfg(test)] mod tests; -/// Data representing a view of some underlying graph, in which each node's -/// successors have been merged into a single "supernode". -/// -/// The resulting supernodes have no obvious meaning on their own. -/// However, merging successor nodes means that a node's out-edges can all -/// be combined into a single out-edge, whose flow is the same as the flow -/// (execution count) of its corresponding node in the original graph. -/// -/// With all node flows now in the original graph now represented as edge flows -/// in the merged graph, it becomes possible to analyze the original node flows -/// using techniques for analyzing edge flows. -#[derive(Debug)] -pub(crate) struct NodeFlowData<Node: Idx> { - /// Maps each node to the supernode that contains it, indicated by some - /// arbitrary "root" node that is part of that supernode. - supernodes: IndexVec<Node, Node>, - /// For each node, stores the single supernode that all of its successors - /// have been merged into. - /// - /// (Note that each node in a supernode can potentially have a _different_ - /// successor supernode from its peers.) - succ_supernodes: IndexVec<Node, Node>, -} - /// Creates a "merged" view of an underlying graph. /// /// The given graph is assumed to have [“balanced flow”](balanced-flow), diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index 8d0d92dc367..d83c0d40a7e 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -1,9 +1,7 @@ use std::collections::BTreeSet; use rustc_data_structures::fx::FxIndexMap; -use rustc_data_structures::graph::DirectedGraph; use rustc_index::IndexVec; -use rustc_index::bit_set::DenseBitSet; use rustc_middle::mir::coverage::{ BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageInfoHi, CoverageKind, }; @@ -63,10 +61,6 @@ const MCDC_MAX_BITMAP_SIZE: usize = i32::MAX as usize; #[derive(Default)] pub(super) struct ExtractedMappings { - /// Store our own copy of [`CoverageGraph::num_nodes`], so that we don't - /// need access to the whole graph when allocating per-BCB data. This is - /// only public so that other code can still use exhaustive destructuring. - pub(super) num_bcbs: usize, pub(super) code_mappings: Vec<CodeMapping>, pub(super) branch_pairs: Vec<BranchPair>, pub(super) mcdc_bitmap_bits: usize, @@ -118,7 +112,6 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( ); ExtractedMappings { - num_bcbs: graph.num_nodes(), code_mappings, branch_pairs, mcdc_bitmap_bits, @@ -127,60 +120,6 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( } } -impl ExtractedMappings { - pub(super) fn all_bcbs_with_counter_mappings(&self) -> DenseBitSet<BasicCoverageBlock> { - // Fully destructure self to make sure we don't miss any fields that have mappings. - let Self { - num_bcbs, - code_mappings, - branch_pairs, - mcdc_bitmap_bits: _, - mcdc_degraded_branches, - mcdc_mappings, - } = self; - - // Identify which BCBs have one or more mappings. - let mut bcbs_with_counter_mappings = DenseBitSet::new_empty(*num_bcbs); - let mut insert = |bcb| { - bcbs_with_counter_mappings.insert(bcb); - }; - - for &CodeMapping { span: _, bcb } in code_mappings { - insert(bcb); - } - for &BranchPair { true_bcb, false_bcb, .. } in branch_pairs { - insert(true_bcb); - insert(false_bcb); - } - for &MCDCBranch { true_bcb, false_bcb, .. } in mcdc_degraded_branches - .iter() - .chain(mcdc_mappings.iter().map(|(_, branches)| branches.into_iter()).flatten()) - { - insert(true_bcb); - insert(false_bcb); - } - - // MC/DC decisions refer to BCBs, but don't require those BCBs to have counters. - if bcbs_with_counter_mappings.is_empty() { - debug_assert!( - mcdc_mappings.is_empty(), - "A function with no counter mappings shouldn't have any decisions: {mcdc_mappings:?}", - ); - } - - bcbs_with_counter_mappings - } - - /// Returns the set of BCBs that have one or more `Code` mappings. - pub(super) fn bcbs_with_ordinary_code_mappings(&self) -> DenseBitSet<BasicCoverageBlock> { - let mut bcbs = DenseBitSet::new_empty(self.num_bcbs); - for &CodeMapping { span: _, bcb } in &self.code_mappings { - bcbs.insert(bcb); - } - bcbs - } -} - fn resolve_block_markers( coverage_info_hi: &CoverageInfoHi, mir_body: &mir::Body<'_>, diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index b1afcc84b6e..e195681bc92 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -21,7 +21,7 @@ use rustc_span::Span; use rustc_span::def_id::LocalDefId; use tracing::{debug, debug_span, trace}; -use crate::coverage::counters::CoverageCounters; +use crate::coverage::counters::BcbCountersData; use crate::coverage::graph::CoverageGraph; use crate::coverage::mappings::ExtractedMappings; @@ -82,29 +82,21 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: let extracted_mappings = mappings::extract_all_mapping_info_from_mir(tcx, mir_body, &hir_info, &graph); - //////////////////////////////////////////////////// - // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure - // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock` - // and all `Expression` dependencies (operands) are also generated, for any other - // `BasicCoverageBlock`s not already associated with a coverage span. - let bcbs_with_counter_mappings = extracted_mappings.all_bcbs_with_counter_mappings(); - if bcbs_with_counter_mappings.is_empty() { - // No relevant spans were found in MIR, so skip instrumenting this function. - return; - } - - let coverage_counters = counters::make_bcb_counters(&graph, &bcbs_with_counter_mappings); - let mappings = create_mappings(&extracted_mappings); if mappings.is_empty() { // No spans could be converted into valid mappings, so skip this function. debug!("no spans could be converted into valid mappings; skipping"); return; } - let term_for_bcb = coverage_counters.node_counters.clone(); - inject_coverage_statements(mir_body, &graph, &extracted_mappings, &coverage_counters); + // Use the coverage graph to prepare intermediate data that will eventually + // be used to assign physical counters and counter expressions to points in + // the control-flow graph + let BcbCountersData { node_flow_data, priority_list } = + counters::prepare_bcb_counters_data(&graph); + // Inject coverage statements into MIR. + inject_coverage_statements(mir_body, &graph); inject_mcdc_statements(mir_body, &graph, &extracted_mappings); let mcdc_num_condition_bitmaps = extracted_mappings @@ -118,11 +110,10 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: function_source_hash: hir_info.function_source_hash, body_span: hir_info.body_span, - num_counters: coverage_counters.num_counters(), - expressions: coverage_counters.into_expressions(), + node_flow_data, + priority_list, mappings, - term_for_bcb, mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits, mcdc_num_condition_bitmaps, @@ -137,7 +128,6 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: fn create_mappings(extracted_mappings: &ExtractedMappings) -> Vec<Mapping> { // Fully destructure the mappings struct to make sure we don't miss any kinds. let ExtractedMappings { - num_bcbs: _, code_mappings, branch_pairs, mcdc_bitmap_bits: _, @@ -215,41 +205,11 @@ fn create_mappings(extracted_mappings: &ExtractedMappings) -> Vec<Mapping> { mappings } -/// For each BCB node or BCB edge that has an associated coverage counter, -/// inject any necessary coverage statements into MIR. -fn inject_coverage_statements<'tcx>( - mir_body: &mut mir::Body<'tcx>, - graph: &CoverageGraph, - extracted_mappings: &ExtractedMappings, - coverage_counters: &CoverageCounters, -) { - // Inject counter-increment statements into MIR. - for (id, bcb) in coverage_counters.counter_increment_sites() { - let target_bb = graph[bcb].leader_bb(); - inject_statement(mir_body, CoverageKind::CounterIncrement { id }, target_bb); - } - - // For each counter expression that is directly associated with at least one - // span, we inject an "expression-used" statement, so that coverage codegen - // can check whether the injected statement survived MIR optimization. - // (BCB edges can't have spans, so we only need to process BCB nodes here.) - // - // We only do this for ordinary `Code` mappings, because branch and MC/DC - // mappings might have expressions that don't correspond to any single - // point in the control-flow graph. - // - // See the code in `rustc_codegen_llvm::coverageinfo::map_data` that deals - // with "expressions seen" and "zero terms". - let eligible_bcbs = extracted_mappings.bcbs_with_ordinary_code_mappings(); - for (bcb, expression_id) in coverage_counters - .bcb_nodes_with_coverage_expressions() - .filter(|&(bcb, _)| eligible_bcbs.contains(bcb)) - { - inject_statement( - mir_body, - CoverageKind::ExpressionUsed { id: expression_id }, - graph[bcb].leader_bb(), - ); +/// Inject any necessary coverage statements into MIR, so that they influence codegen. +fn inject_coverage_statements<'tcx>(mir_body: &mut mir::Body<'tcx>, graph: &CoverageGraph) { + for (bcb, data) in graph.iter_enumerated() { + let target_bb = data.leader_bb(); + inject_statement(mir_body, CoverageKind::VirtualCounter { bcb }, target_bb); } } diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index ef2724222ce..f362f4ccfa2 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -1,9 +1,10 @@ use rustc_data_structures::captures::Captures; +use rustc_index::IndexSlice; use rustc_index::bit_set::DenseBitSet; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::coverage::{ - CounterId, CovTerm, CoverageIdsInfo, CoverageKind, Expression, ExpressionId, - FunctionCoverageInfo, MappingKind, Op, + BasicCoverageBlock, CounterId, CovTerm, CoverageIdsInfo, CoverageKind, Expression, + ExpressionId, MappingKind, Op, }; use rustc_middle::mir::{Body, Statement, StatementKind}; use rustc_middle::ty::{self, TyCtxt}; @@ -12,6 +13,9 @@ use rustc_span::def_id::LocalDefId; use rustc_span::sym; use tracing::trace; +use crate::coverage::counters::node_flow::make_node_counters; +use crate::coverage::counters::{CoverageCounters, transcribe_counters}; + /// Registers query/hook implementations related to coverage. pub(crate) fn provide(providers: &mut Providers) { providers.hooks.is_eligible_for_coverage = is_eligible_for_coverage; @@ -89,41 +93,85 @@ fn coverage_ids_info<'tcx>( let mir_body = tcx.instance_mir(instance_def); let fn_cov_info = mir_body.function_coverage_info.as_deref()?; - let mut counters_seen = DenseBitSet::new_empty(fn_cov_info.num_counters); - let mut expressions_seen = DenseBitSet::new_filled(fn_cov_info.expressions.len()); + // Scan through the final MIR to see which BCBs survived MIR opts. + // Any BCB not in this set was optimized away. + let mut bcbs_seen = DenseBitSet::new_empty(fn_cov_info.priority_list.len()); + for kind in all_coverage_in_mir_body(mir_body) { + match *kind { + CoverageKind::VirtualCounter { bcb } => { + bcbs_seen.insert(bcb); + } + _ => {} + } + } + + // Determine the set of BCBs that are referred to by mappings, and therefore + // need a counter. Any node not in this set will only get a counter if it + // is part of the counter expression for a node that is in the set. + let mut bcb_needs_counter = + DenseBitSet::<BasicCoverageBlock>::new_empty(fn_cov_info.priority_list.len()); + for mapping in &fn_cov_info.mappings { + match mapping.kind { + MappingKind::Code { bcb } => { + bcb_needs_counter.insert(bcb); + } + MappingKind::Branch { true_bcb, false_bcb } => { + bcb_needs_counter.insert(true_bcb); + bcb_needs_counter.insert(false_bcb); + } + MappingKind::MCDCBranch { true_bcb, false_bcb, mcdc_params: _ } => { + bcb_needs_counter.insert(true_bcb); + bcb_needs_counter.insert(false_bcb); + } + MappingKind::MCDCDecision(_) => {} + } + } + + let node_counters = make_node_counters(&fn_cov_info.node_flow_data, &fn_cov_info.priority_list); + let coverage_counters = transcribe_counters(&node_counters, &bcb_needs_counter); + + let mut counters_seen = DenseBitSet::new_empty(coverage_counters.node_counters.len()); + let mut expressions_seen = DenseBitSet::new_filled(coverage_counters.expressions.len()); // For each expression ID that is directly used by one or more mappings, // mark it as not-yet-seen. This indicates that we expect to see a - // corresponding `ExpressionUsed` statement during MIR traversal. + // corresponding `VirtualCounter` statement during MIR traversal. for mapping in fn_cov_info.mappings.iter() { // Currently we only worry about ordinary code mappings. // For branch and MC/DC mappings, expressions might not correspond // to any particular point in the control-flow graph. - // (Keep this in sync with the injection of `ExpressionUsed` - // statements in the `InstrumentCoverage` MIR pass.) if let MappingKind::Code { bcb } = mapping.kind - && let Some(CovTerm::Expression(id)) = fn_cov_info.term_for_bcb[bcb] + && let Some(CovTerm::Expression(id)) = coverage_counters.node_counters[bcb] { expressions_seen.remove(id); } } - for kind in all_coverage_in_mir_body(mir_body) { - match *kind { - CoverageKind::CounterIncrement { id } => { - counters_seen.insert(id); - } - CoverageKind::ExpressionUsed { id } => { - expressions_seen.insert(id); - } - _ => {} + for bcb in bcbs_seen.iter() { + if let Some(&id) = coverage_counters.phys_counter_for_node.get(&bcb) { + counters_seen.insert(id); + } + if let Some(CovTerm::Expression(id)) = coverage_counters.node_counters[bcb] { + expressions_seen.insert(id); } } - let zero_expressions = - identify_zero_expressions(fn_cov_info, &counters_seen, &expressions_seen); + let zero_expressions = identify_zero_expressions( + &coverage_counters.expressions, + &counters_seen, + &expressions_seen, + ); - Some(CoverageIdsInfo { counters_seen, zero_expressions }) + let CoverageCounters { phys_counter_for_node, node_counters, expressions, .. } = + coverage_counters; + + Some(CoverageIdsInfo { + counters_seen, + zero_expressions, + phys_counter_for_node, + term_for_bcb: node_counters, + expressions, + }) } fn all_coverage_in_mir_body<'a, 'tcx>( @@ -150,7 +198,7 @@ fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { /// already being performed by the Rust-side expression renumbering, so that /// the resulting coverage mappings don't get worse. fn identify_zero_expressions( - fn_cov_info: &FunctionCoverageInfo, + expressions: &IndexSlice<ExpressionId, Expression>, counters_seen: &DenseBitSet<CounterId>, expressions_seen: &DenseBitSet<ExpressionId>, ) -> DenseBitSet<ExpressionId> { @@ -158,13 +206,13 @@ fn identify_zero_expressions( // have zero as both of their operands, and will therefore always have // a value of zero. Other expressions that refer to these as operands // can have those operands replaced with `CovTerm::Zero`. - let mut zero_expressions = DenseBitSet::new_empty(fn_cov_info.expressions.len()); + let mut zero_expressions = DenseBitSet::new_empty(expressions.len()); // Simplify a copy of each expression based on lower-numbered expressions, // and then update the set of always-zero expressions if necessary. // (By construction, expressions can only refer to other expressions // that have lower IDs, so one pass is sufficient.) - for (id, expression) in fn_cov_info.expressions.iter_enumerated() { + for (id, expression) in expressions.iter_enumerated() { if !expressions_seen.contains(id) { // If an expression was not seen, it must have been optimized away, // so any operand that refers to it can be replaced with zero. diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 26ce743be36..73b68d7155c 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -137,8 +137,7 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> { // These coverage statements should not exist prior to coverage instrumentation. StatementKind::Coverage( - CoverageKind::CounterIncrement { .. } - | CoverageKind::ExpressionUsed { .. } + CoverageKind::VirtualCounter { .. } | CoverageKind::CondBitmapUpdate { .. } | CoverageKind::TestVectorBitmapUpdate { .. }, ) => bug!( |
