bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

coverage: Simplify how counter terms are stored

Browse source 
Using `SmallVec` here was fine when it was a module-private detail, but if we
want to pass these terms across query boundaries then it's not worth the extra
hassle.

Replacing a method call with direct field access is slightly simpler.

Using the name `counter_terms` is more consistent with other code that tries to
maintain a distinction between (physical) "counters" and "expressions".
This commit is contained in:
Zalathar 2025-01-21 20:34:52 +11:00
parent ff48331588
commit 52c1bfa7bb
3 changed files with 20 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

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

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

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

@ -10,7 +10,6 @@ use rustc_data_structures::graph;
use rustc_index::bit_set::DenseBitSet;
use rustc_index::{Idx, IndexVec};
use rustc_middle::mir::coverage::Op;
use smallvec::SmallVec;
use crate::coverage::counters::iter_nodes::IterNodes;
use crate::coverage::counters::union_find::{FrozenUnionFind, UnionFind};
@ -100,7 +99,7 @@ impl<Node: Idx> MergedNodeFlowGraph<Node> {
builder.visit_node(node);
}
NodeCounters { counter_exprs: builder.finish() }
NodeCounters { counter_terms: builder.finish() }
}
}
@ -108,18 +107,12 @@ impl<Node: Idx> MergedNodeFlowGraph<Node> {
/// nodes of a graph.
#[derive(Debug)]
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
/// 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.
pub(crate) fn counter_expr(&self, this: Node) -> &[CounterTerm<Node>] {
self.counter_exprs[this].as_slice()
}
pub(crate) counter_terms: IndexVec<Node, Vec<CounterTerm<Node>>>,
}
#[derive(Debug)]
@ -146,9 +139,6 @@ pub(crate) struct CounterTerm<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)]
struct SpantreeBuilder<'a, Node: Idx> {
graph: &'a MergedNodeFlowGraph<Node>,
@ -163,7 +153,7 @@ struct SpantreeBuilder<'a, Node: Idx> {
yank_buffer: Vec<Node>,
/// An in-progress counter expression for each node. Each expression is
/// 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> {
@ -174,7 +164,7 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
is_unvisited: DenseBitSet::new_filled(num_nodes),
span_edges: IndexVec::from_fn_n(|_| None, num_nodes),
yank_buffer: vec![],
counter_exprs: IndexVec::from_fn_n(|_| SmallVec::new(), num_nodes),
counter_terms: IndexVec::from_fn_n(|_| vec![], num_nodes),
}
}
@ -268,8 +258,8 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
// `this_supernode`.
// Instead of setting `this.measure = true` as in the original paper,
// we just add the node's ID to its own "expression".
self.counter_exprs[this].push(CounterTerm { node: this, op: Op::Add });
// we just add the node's ID to its own list of terms.
self.counter_terms[this].push(CounterTerm { node: this, op: Op::Add });
// Walk the spantree from `this.successor` back to `this`. For each
// spantree edge along the way, add this node's physical counter to
@ -279,7 +269,7 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
let &SpantreeEdge { is_reversed, claiming_node, span_parent } =
self.span_edges[curr].as_ref().unwrap();
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;
}
@ -288,8 +278,8 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
/// Asserts that all nodes have been visited, and returns the computed
/// counter expressions (made up of physical counters) for each node.
fn finish(self) -> IndexVec<Node, CounterExprVec<Node>> {
let Self { graph, is_unvisited, span_edges, yank_buffer: _, counter_exprs } = self;
fn finish(self) -> IndexVec<Node, Vec<CounterTerm<Node>>> {
let Self { graph, is_unvisited, span_edges, yank_buffer: _, counter_terms } = self;
assert!(is_unvisited.is_empty(), "some nodes were never visited: {is_unvisited:?}");
debug_assert!(
span_edges
@ -298,9 +288,9 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
"only supernodes can have a span edge",
);
debug_assert!(
counter_exprs.iter().all(|expr| !expr.is_empty()),
"after visiting all nodes, every node should have a non-empty expression",
counter_terms.iter().all(|terms| !terms.is_empty()),
"after visiting all nodes, every node should have at least one term",
);
counter_exprs
counter_terms
}
}

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

@ -53,12 +53,12 @@ fn format_counter_expressions<Node: Idx>(counters: &NodeCounters<Node>) -> Vec<S
};
counters
.counter_exprs
.counter_terms
.indices()
.map(|node| {
let mut expr = counters.counter_expr(node).iter().collect::<Vec<_>>();
expr.sort_by_key(|item| item.node.index());
format!("[{node:?}]: {}", expr.into_iter().map(format_item).join(" "))
let mut terms = counters.counter_terms[node].iter().collect::<Vec<_>>();
terms.sort_by_key(|item| item.node.index());
format!("[{node:?}]: {}", terms.into_iter().map(format_item).join(" "))
})
.collect()
}