bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

coverage: Treat the "merged node flow graph" as a plain data struct

Browse source 
By removing all methods from this struct and treating it as a collection of
data fields, we make it easier for a future PR to store that data in a query
result, without having to move all of its methods into `rustc_middle`.
This commit is contained in:
Zalathar 2025-01-21 11:25:10 +11:00
parent 4b20a27ae0
commit 2bdc67a75e
3 changed files with 87 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

12
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> {

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

@ -8,7 +8,7 @@
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 crate::coverage::counters::iter_nodes::IterNodes; use crate::coverage::counters::iter_nodes::IterNodes;
@ -17,8 +17,8 @@ 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
@ -29,7 +29,7 @@ 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: IndexVec<Node, Node>, supernodes: IndexVec<Node, Node>,
@ -41,66 +41,59 @@ 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, take a snapshot of 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.snapshot(); let supernodes = supernodes.snapshot();
let succ_supernodes = successors.into_iter().map(|succ| supernodes[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[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_terms: builder.finish() }
}
} }
/// End result of allocating physical counters and counter expressions for the /// End result of allocating physical counters and counter expressions for the
@ -141,7 +134,9 @@ pub(crate) struct CounterTerm<Node> {
#[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.
@ -157,10 +152,12 @@ struct SpantreeBuilder<'a, Node: Idx> {
} }
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![],
@ -168,11 +165,15 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
} }
} }
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,
@ -183,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
@ -225,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[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.)
@ -279,18 +280,19 @@ 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, Vec<CounterTerm<Node>>> { fn finish(self) -> IndexVec<Node, Vec<CounterTerm<Node>>> {
let Self { graph, is_unvisited, span_edges, yank_buffer: _, counter_terms } = 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_terms.iter().all(|terms| !terms.is_empty()), counter_terms.iter().all(|terms| !terms.is_empty()),
"after visiting all nodes, every node should have at least one term", "after visiting all nodes, every node should have at least one term",
); );
counter_terms
self.counter_terms
} }
} }

12
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)