Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Preprocess dominator tree to answer queries in O(1) #107157

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/transform/validate.rs
Expand Up @@ -144,7 +144,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if self.unwind_edge_count <= 1 {
return;
}
let doms = self.body.basic_blocks.dominators();
let dom_tree = self.body.basic_blocks.dominator_tree();
let mut post_contract_node = FxHashMap::default();
// Reusing the allocation across invocations of the closure
let mut dom_path = vec![];
Expand All @@ -153,7 +153,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if let Some(root) = post_contract_node.get(&bb) {
break *root;
}
let parent = doms.immediate_dominator(bb);
let parent = dom_tree.immediate_dominator(bb);
dom_path.push(bb);
if !self.body.basic_blocks[parent].is_cleanup {
break bb;
Expand Down
128 changes: 107 additions & 21 deletions compiler/rustc_data_structures/src/graph/dominators/mod.rs
Expand Up @@ -25,7 +25,7 @@ rustc_index::newtype_index! {
struct PreorderIndex {}
}

pub fn dominators<G: ControlFlowGraph>(graph: G) -> Dominators<G::Node> {
pub fn dominator_tree<G: ControlFlowGraph>(graph: G) -> DominatorTree<G::Node> {
// compute the post order index (rank) for each node
let mut post_order_rank = IndexVec::from_elem_n(0, graph.num_nodes());

Expand Down Expand Up @@ -201,7 +201,7 @@ pub fn dominators<G: ControlFlowGraph>(graph: G) -> Dominators<G::Node> {
immediate_dominators[*node] = Some(pre_order_to_real[idom[idx]]);
}

Dominators { post_order_rank, immediate_dominators }
DominatorTree { post_order_rank, immediate_dominators }
}

/// Evaluate the link-eval virtual forest, providing the currently minimum semi
Expand Down Expand Up @@ -265,13 +265,14 @@ fn compress(
}

#[derive(Clone, Debug)]
pub struct Dominators<N: Idx> {
pub struct DominatorTree<N: Idx> {
post_order_rank: IndexVec<N, usize>,
// Note: immediate_dominators[root] is Some(root)!
immediate_dominators: IndexVec<N, Option<N>>,
}

impl<Node: Idx> Dominators<Node> {
pub fn is_reachable(&self, node: Node) -> bool {
impl<Node: Idx> DominatorTree<Node> {
fn is_reachable(&self, node: Node) -> bool {
self.immediate_dominators[node].is_some()
}

Expand All @@ -282,25 +283,12 @@ impl<Node: Idx> Dominators<Node> {

pub fn dominators(&self, node: Node) -> Iter<'_, Node> {
assert!(self.is_reachable(node), "node {node:?} is not reachable");
Iter { dominators: self, node: Some(node) }
}

pub fn is_dominated_by(&self, node: Node, dom: Node) -> bool {
// FIXME -- could be optimized by using post-order-rank
self.dominators(node).any(|n| n == dom)
}

/// Provide deterministic ordering of nodes such that, if any two nodes have a dominator
/// relationship, the dominator will always precede the dominated. (The relative ordering
/// of two unrelated nodes will also be consistent, but otherwise the order has no
/// meaning.) This method cannot be used to determine if either Node dominates the other.
pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs])
Iter { dom_tree: self, node: Some(node) }
}
}

pub struct Iter<'dom, Node: Idx> {
dominators: &'dom Dominators<Node>,
dom_tree: &'dom DominatorTree<Node>,
node: Option<Node>,
}

Expand All @@ -309,7 +297,7 @@ impl<'dom, Node: Idx> Iterator for Iter<'dom, Node> {

fn next(&mut self) -> Option<Self::Item> {
if let Some(node) = self.node {
let dom = self.dominators.immediate_dominator(node);
let dom = self.dom_tree.immediate_dominator(node);
if dom == node {
self.node = None; // reached the root
} else {
Expand All @@ -321,3 +309,101 @@ impl<'dom, Node: Idx> Iterator for Iter<'dom, Node> {
}
}
}

#[derive(Clone, Debug)]
pub struct Dominators<Node: Idx> {
time: IndexVec<Node, Time>,
post_order_rank: IndexVec<Node, usize>,
}

/// Describes the number of vertices discovered at the time when processing of a particular vertex
/// started and when it finished. Both values are zero for unreachable vertices.
#[derive(Copy, Clone, Default, Debug)]
struct Time {
start: u32,
finish: u32,
}

impl<Node: Idx> Dominators<Node> {
pub fn dummy() -> Self {
Self { time: Default::default(), post_order_rank: Default::default() }
}

/// Returns true if `a` dominates `b`.
///
/// # Panics
///
/// Panics if `b` is unreachable.
pub fn dominates(&self, a: Node, b: Node) -> bool {
let a = self.time[a];
let b = self.time[b];
assert!(b.start != 0, "node {b:?} is not reachable");
a.start <= b.start && b.finish <= a.finish
}

/// Provide deterministic ordering of nodes such that, if any two nodes have a dominator
/// relationship, the dominator will always precede the dominated. (The relative ordering
/// of two unrelated nodes will also be consistent, but otherwise the order has no
/// meaning.) This method cannot be used to determine if either Node dominates the other.
pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs])
}
}

pub fn dominators<G: Copy + ControlFlowGraph>(graph: G) -> Dominators<G::Node> {
let DominatorTree { mut immediate_dominators, post_order_rank } = dominator_tree(graph);

immediate_dominators[graph.start_node()] = None;

// Transpose the dominator tree edges, so that child nodes of vertex v are stored in
// node[edges[v].start..edges[y].end].
let mut edges: IndexVec<G::Node, std::ops::Range<u32>> =
IndexVec::from_elem_n(0..0, graph.num_nodes());
for &idom in immediate_dominators.iter() {
if let Some(idom) = idom {
edges[idom].end += 1;
}
}
let mut m = 0;
for e in edges.iter_mut() {
m += e.end;
e.start = m;
e.end = m;
}
let mut node = IndexVec::from_elem_n(Idx::new(0), m.try_into().unwrap());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you make the m a usize from the start, you can avoid the try_into().unwrap()

for (i, &idom) in immediate_dominators.iter_enumerated() {
if let Some(idom) = idom {
edges[idom].start -= 1;
node[edges[idom].start] = i;
}
}

// Perform a depth-first search of the dominator tree. Record the number of vertices discovered
// when vertex v is discovered first as time[v].start, and when its processing is finished as
// time[v].finish.
let mut time: IndexVec<G::Node, Time> =
IndexVec::from_elem_n(Time::default(), graph.num_nodes());
let mut stack = Vec::new();

let mut discovered = 1;
stack.push(graph.start_node());
time[graph.start_node()].start = discovered;

while let Some(&i) = stack.last() {
let e = &mut edges[i];
if e.start == e.end {
// Finish processing vertex i.
time[i].finish = discovered;
stack.pop();
} else {
let j = node[e.start];
e.start += 1;
// Start processing vertex j.
discovered += 1;
time[j].start = discovered;
stack.push(j);
}
}

Dominators { time, post_order_rank }
}
10 changes: 5 additions & 5 deletions compiler/rustc_data_structures/src/graph/dominators/tests.rs
Expand Up @@ -6,8 +6,8 @@ use super::super::tests::TestGraph;
fn diamond() {
let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (2, 3)]);

let dominators = dominators(&graph);
let immediate_dominators = &dominators.immediate_dominators;
let dom_tree = dominator_tree(&graph);
let immediate_dominators = &dom_tree.immediate_dominators;
assert_eq!(immediate_dominators[0], Some(0));
assert_eq!(immediate_dominators[1], Some(0));
assert_eq!(immediate_dominators[2], Some(0));
Expand All @@ -22,8 +22,8 @@ fn paper() {
&[(6, 5), (6, 4), (5, 1), (4, 2), (4, 3), (1, 2), (2, 3), (3, 2), (2, 1)],
);

let dominators = dominators(&graph);
let immediate_dominators = &dominators.immediate_dominators;
let dom_tree = dominator_tree(&graph);
let immediate_dominators = &dom_tree.immediate_dominators;
assert_eq!(immediate_dominators[0], None); // <-- note that 0 is not in graph
assert_eq!(immediate_dominators[1], Some(6));
assert_eq!(immediate_dominators[2], Some(6));
Expand All @@ -41,5 +41,5 @@ fn paper_slt() {
&[(1, 2), (1, 3), (2, 3), (2, 7), (3, 4), (3, 6), (4, 5), (5, 4), (6, 7), (7, 8), (8, 5)],
);

dominators(&graph);
dominator_tree(&graph);
}
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/mir/basic_blocks.rs
Expand Up @@ -5,7 +5,9 @@ use crate::mir::traversal::PostorderCache;
use crate::mir::{BasicBlock, BasicBlockData, Successors, START_BLOCK};

use rustc_data_structures::graph;
use rustc_data_structures::graph::dominators::{dominators, Dominators};
use rustc_data_structures::graph::dominators::{
dominator_tree, dominators, DominatorTree, Dominators,
};
use rustc_index::vec::IndexVec;

#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable, TypeFoldable, TypeVisitable)]
Expand Down Expand Up @@ -35,6 +37,10 @@ impl<'tcx> BasicBlocks<'tcx> {
self.is_cyclic.is_cyclic(self)
}

pub fn dominator_tree(&self) -> DominatorTree<BasicBlock> {
dominator_tree(&self)
}

#[inline]
pub fn dominators(&self) -> Dominators<BasicBlock> {
dominators(&self)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Expand Up @@ -3049,7 +3049,7 @@ impl Location {
if self.block == other.block {
self.statement_index <= other.statement_index
} else {
dominators.is_dominated_by(other.block, self.block)
dominators.dominates(self.block, other.block)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Expand Up @@ -520,7 +520,7 @@ impl<'a> BcbCounters<'a> {
let mut found_loop_exit = false;
for &branch in branches.iter() {
if backedge_from_bcbs.iter().any(|&backedge_from_bcb| {
self.bcb_is_dominated_by(backedge_from_bcb, branch.target_bcb)
self.bcb_dominates(branch.target_bcb, backedge_from_bcb)
}) {
if let Some(reloop_branch) = some_reloop_branch {
if reloop_branch.counter(&self.basic_coverage_blocks).is_none() {
Expand Down Expand Up @@ -603,8 +603,8 @@ impl<'a> BcbCounters<'a> {
}

#[inline]
fn bcb_is_dominated_by(&self, node: BasicCoverageBlock, dom: BasicCoverageBlock) -> bool {
self.basic_coverage_blocks.is_dominated_by(node, dom)
fn bcb_dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool {
self.basic_coverage_blocks.dominates(dom, node)
}

#[inline]
Expand Down
30 changes: 5 additions & 25 deletions compiler/rustc_mir_transform/src/coverage/graph.rs
Expand Up @@ -209,8 +209,8 @@ impl CoverageGraph {
}

#[inline(always)]
pub fn is_dominated_by(&self, node: BasicCoverageBlock, dom: BasicCoverageBlock) -> bool {
self.dominators.as_ref().unwrap().is_dominated_by(node, dom)
pub fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool {
self.dominators.as_ref().unwrap().dominates(dom, node)
}

#[inline(always)]
Expand Down Expand Up @@ -312,7 +312,7 @@ rustc_index::newtype_index! {
/// to the BCB's primary counter or expression).
///
/// The BCB CFG is critical to simplifying the coverage analysis by ensuring graph path-based
/// queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch (control flow)
/// queries (`dominates()`, `predecessors`, `successors`, etc.) have branch (control flow)
/// significance.
#[derive(Debug, Clone)]
pub(super) struct BasicCoverageBlockData {
Expand Down Expand Up @@ -594,7 +594,7 @@ impl TraverseCoverageGraphWithLoops {
// branching block would have given an `Expression` (or vice versa).
let (some_successor_to_add, some_loop_header) =
if let Some((_, loop_header)) = context.loop_backedges {
if basic_coverage_blocks.is_dominated_by(successor, loop_header) {
if basic_coverage_blocks.dominates(loop_header, successor) {
(Some(successor), Some(loop_header))
} else {
(None, None)
Expand Down Expand Up @@ -652,29 +652,9 @@ pub(super) fn find_loop_backedges(
let mut backedges = IndexVec::from_elem_n(Vec::<BasicCoverageBlock>::new(), num_bcbs);

// Identify loops by their backedges.
//
// The computational complexity is bounded by: n(s) x d where `n` is the number of
// `BasicCoverageBlock` nodes (the simplified/reduced representation of the CFG derived from the
// MIR); `s` is the average number of successors per node (which is most likely less than 2, and
// independent of the size of the function, so it can be treated as a constant);
// and `d` is the average number of dominators per node.
//
// The average number of dominators depends on the size and complexity of the function, and
// nodes near the start of the function's control flow graph typically have less dominators
// than nodes near the end of the CFG. Without doing a detailed mathematical analysis, I
// think the resulting complexity has the characteristics of O(n log n).
//
// The overall complexity appears to be comparable to many other MIR transform algorithms, and I
// don't expect that this function is creating a performance hot spot, but if this becomes an
// issue, there may be ways to optimize the `is_dominated_by` algorithm (as indicated by an
// existing `FIXME` comment in that code), or possibly ways to optimize it's usage here, perhaps
// by keeping track of results for visited `BasicCoverageBlock`s if they can be used to short
// circuit downstream `is_dominated_by` checks.
//
// For now, that kind of optimization seems unnecessarily complicated.
for (bcb, _) in basic_coverage_blocks.iter_enumerated() {
for &successor in &basic_coverage_blocks.successors[bcb] {
if basic_coverage_blocks.is_dominated_by(bcb, successor) {
if basic_coverage_blocks.dominates(successor, bcb) {
let loop_header = successor;
let backedge_from_bcb = bcb;
debug!(
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Expand Up @@ -63,7 +63,7 @@ impl CoverageStatement {
/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that
/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches
/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock`
/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`.
/// `dominates()` the `BasicBlock`s in this `CoverageSpan`.
#[derive(Debug, Clone)]
pub(super) struct CoverageSpan {
pub span: Span,
Expand Down Expand Up @@ -705,12 +705,12 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
fn hold_pending_dups_unless_dominated(&mut self) {
// Equal coverage spans are ordered by dominators before dominated (if any), so it should be
// impossible for `curr` to dominate any previous `CoverageSpan`.
debug_assert!(!self.span_bcb_is_dominated_by(self.prev(), self.curr()));
debug_assert!(!self.span_bcb_dominates(self.curr(), self.prev()));

let initial_pending_count = self.pending_dups.len();
if initial_pending_count > 0 {
let mut pending_dups = self.pending_dups.split_off(0);
pending_dups.retain(|dup| !self.span_bcb_is_dominated_by(self.curr(), dup));
pending_dups.retain(|dup| !self.span_bcb_dominates(dup, self.curr()));
self.pending_dups.append(&mut pending_dups);
if self.pending_dups.len() < initial_pending_count {
debug!(
Expand All @@ -721,7 +721,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
}
}

if self.span_bcb_is_dominated_by(self.curr(), self.prev()) {
if self.span_bcb_dominates(self.prev(), self.curr()) {
debug!(
" different bcbs but SAME spans, and prev dominates curr. Discard prev={:?}",
self.prev()
Expand Down Expand Up @@ -787,8 +787,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
}
}

fn span_bcb_is_dominated_by(&self, covspan: &CoverageSpan, dom_covspan: &CoverageSpan) -> bool {
self.basic_coverage_blocks.is_dominated_by(covspan.bcb, dom_covspan.bcb)
fn span_bcb_dominates(&self, dom_covspan: &CoverageSpan, covspan: &CoverageSpan) -> bool {
self.basic_coverage_blocks.dominates(dom_covspan.bcb, covspan.bcb)
}
}

Expand Down