Skip to content

Move cmp_in_dominator_order out of graph dominator computation #132022

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

Merged
merged 1 commit into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 1 addition & 22 deletions compiler/rustc_data_structures/src/graph/dominators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
//! Thomas Lengauer and Robert Endre Tarjan.
//! <https://www.cs.princeton.edu/courses/archive/spr03/cs423/download/dominators.pdf>

use std::cmp::Ordering;

use rustc_index::{Idx, IndexSlice, IndexVec};

use super::ControlFlowGraph;
Expand Down Expand Up @@ -64,9 +62,6 @@ fn is_small_path_graph<G: ControlFlowGraph>(g: &G) -> bool {
}

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

// We allocate capacity for the full set of nodes, because most of the time
// most of the nodes *are* reachable.
let mut parent: IndexVec<PreorderIndex, PreorderIndex> =
Expand All @@ -83,12 +78,10 @@ fn dominators_impl<G: ControlFlowGraph>(graph: &G) -> Inner<G::Node> {
pre_order_to_real.push(graph.start_node());
parent.push(PreorderIndex::ZERO); // the parent of the root node is the root for now.
real_to_pre_order[graph.start_node()] = Some(PreorderIndex::ZERO);
let mut post_order_idx = 0;

// Traverse the graph, collecting a number of things:
//
// * Preorder mapping (to it, and back to the actual ordering)
// * Postorder mapping (used exclusively for `cmp_in_dominator_order` on the final product)
// * Parents for each vertex in the preorder tree
//
// These are all done here rather than through one of the 'standard'
Expand All @@ -104,8 +97,6 @@ fn dominators_impl<G: ControlFlowGraph>(graph: &G) -> Inner<G::Node> {
continue 'recurse;
}
}
post_order_rank[pre_order_to_real[frame.pre_order_idx]] = post_order_idx;
post_order_idx += 1;

stack.pop();
}
Expand Down Expand Up @@ -282,7 +273,7 @@ fn dominators_impl<G: ControlFlowGraph>(graph: &G) -> Inner<G::Node> {

let time = compute_access_time(start_node, &immediate_dominators);

Inner { post_order_rank, immediate_dominators, time }
Inner { immediate_dominators, time }
}

/// Evaluate the link-eval virtual forest, providing the currently minimum semi
Expand Down Expand Up @@ -348,7 +339,6 @@ fn compress(
/// Tracks the list of dominators for each node.
#[derive(Clone, Debug)]
struct Inner<N: Idx> {
post_order_rank: IndexVec<N, usize>,
// Even though we track only the immediate dominator of each node, it's
// possible to get its full list of dominators by looking up the dominator
// of each dominator.
Expand Down Expand Up @@ -379,17 +369,6 @@ impl<Node: Idx> Dominators<Node> {
}
}

/// 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 cmp_in_dominator_order(&self, lhs: Node, rhs: Node) -> Ordering {
match &self.kind {
Kind::Path => lhs.index().cmp(&rhs.index()),
Kind::General(g) => g.post_order_rank[rhs].cmp(&g.post_order_rank[lhs]),
}
}

/// Returns true if `a` dominates `b`.
///
/// # Panics
Expand Down
25 changes: 23 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ pub(crate) struct CoverageGraph {
pub(crate) successors: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
pub(crate) predecessors: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
dominators: Option<Dominators<BasicCoverageBlock>>,
/// Allows nodes to be compared in some total order such that _if_
/// `a` dominates `b`, then `a < b`. If neither node dominates the other,
/// their relative order is consistent but arbitrary.
dominator_order_rank: IndexVec<BasicCoverageBlock, u32>,
}

impl CoverageGraph {
Expand Down Expand Up @@ -54,10 +58,27 @@ impl CoverageGraph {
}
}

let mut this = Self { bcbs, bb_to_bcb, successors, predecessors, dominators: None };
let num_nodes = bcbs.len();
let mut this = Self {
bcbs,
bb_to_bcb,
successors,
predecessors,
dominators: None,
dominator_order_rank: IndexVec::from_elem_n(0, num_nodes),
};
assert_eq!(num_nodes, this.num_nodes());

this.dominators = Some(dominators::dominators(&this));

// The dominator rank of each node is just its index in a reverse-postorder traversal.
let reverse_post_order = graph::iterate::reverse_post_order(&this, this.start_node());
// The coverage graph is created by traversal, so all nodes are reachable.
assert_eq!(reverse_post_order.len(), this.num_nodes());
for (rank, bcb) in (0u32..).zip(reverse_post_order) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit : reverse_post_order.enumerate() ?
R=me either way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The node indices being ranked are 32-bit values, so using u32 for the rank (instead of usize) was intentional.

this.dominator_order_rank[bcb] = rank;
}

// The coverage graph's entry-point node (bcb0) always starts with bb0,
// which never has predecessors. Any other blocks merged into bcb0 can't
// have multiple (coverage-relevant) predecessors, so bcb0 always has
Expand Down Expand Up @@ -162,7 +183,7 @@ impl CoverageGraph {
a: BasicCoverageBlock,
b: BasicCoverageBlock,
) -> Ordering {
self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b)
self.dominator_order_rank[a].cmp(&self.dominator_order_rank[b])
}

/// Returns the source of this node's sole in-edge, if it has exactly one.
Expand Down
Loading