Skip to content

Commit 97d3280

Browse files
committed
Auto merge of rust-lang#111673 - cjgillot:dominator-preprocess, r=cjgillot,tmiasko
Preprocess and cache dominator tree Preprocessing dominators has a very strong effect for rust-lang#111344. That pass checks that assignments dominate their uses repeatedly. Using the unprocessed dominator tree caused a quadratic runtime (number of bbs x depth of the dominator tree). This PR also caches the dominator tree and the pre-processed dominators in the MIR cfg cache. Rebase of rust-lang#107157 cc `@tmiasko`
2 parents b3cbf7c + 7c8f29f commit 97d3280

File tree

9 files changed

+107
-51
lines changed

9 files changed

+107
-51
lines changed

compiler/rustc_borrowck/src/invalidation.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ struct InvalidationGenerator<'cx, 'tcx> {
4646
all_facts: &'cx mut AllFacts,
4747
location_table: &'cx LocationTable,
4848
body: &'cx Body<'tcx>,
49-
dominators: Dominators<BasicBlock>,
49+
dominators: &'cx Dominators<BasicBlock>,
5050
borrow_set: &'cx BorrowSet<'tcx>,
5151
}
5252

compiler/rustc_borrowck/src/lib.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ use rustc_target::abi::FieldIdx;
4343

4444
use either::Either;
4545
use smallvec::SmallVec;
46-
use std::cell::OnceCell;
4746
use std::cell::RefCell;
4847
use std::collections::BTreeMap;
4948
use std::ops::Deref;
@@ -331,7 +330,6 @@ fn do_mir_borrowck<'tcx>(
331330
used_mut: Default::default(),
332331
used_mut_upvars: SmallVec::new(),
333332
borrow_set: Rc::clone(&borrow_set),
334-
dominators: Default::default(),
335333
upvars: Vec::new(),
336334
local_names: IndexVec::from_elem(None, &promoted_body.local_decls),
337335
region_names: RefCell::default(),
@@ -360,7 +358,6 @@ fn do_mir_borrowck<'tcx>(
360358
used_mut: Default::default(),
361359
used_mut_upvars: SmallVec::new(),
362360
borrow_set: Rc::clone(&borrow_set),
363-
dominators: Default::default(),
364361
upvars,
365362
local_names,
366363
region_names: RefCell::default(),
@@ -591,9 +588,6 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
591588
/// The set of borrows extracted from the MIR
592589
borrow_set: Rc<BorrowSet<'tcx>>,
593590

594-
/// Dominators for MIR
595-
dominators: OnceCell<Dominators<BasicBlock>>,
596-
597591
/// Information about upvars not necessarily preserved in types or MIR
598592
upvars: Vec<Upvar<'tcx>>,
599593

@@ -2269,7 +2263,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
22692263
}
22702264

22712265
fn dominators(&self) -> &Dominators<BasicBlock> {
2272-
self.dominators.get_or_init(|| self.body.basic_blocks.dominators())
2266+
// `BasicBlocks` computes dominators on-demand and caches them.
2267+
self.body.basic_blocks.dominators()
22732268
}
22742269
}
22752270

compiler/rustc_codegen_ssa/src/mir/analyze.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ impl DefLocation {
8484

8585
struct LocalAnalyzer<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
8686
fx: &'mir FunctionCx<'a, 'tcx, Bx>,
87-
dominators: Dominators<mir::BasicBlock>,
87+
dominators: &'mir Dominators<mir::BasicBlock>,
8888
locals: IndexVec<mir::Local, LocalKind>,
8989
}
9090

compiler/rustc_data_structures/src/graph/dominators/mod.rs

+85-10
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ rustc_index::newtype_index! {
2626
struct PreorderIndex {}
2727
}
2828

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

@@ -244,7 +244,10 @@ pub fn dominators<G: ControlFlowGraph>(graph: G) -> Dominators<G::Node> {
244244

245245
let start_node = graph.start_node();
246246
immediate_dominators[start_node] = None;
247-
Dominators { start_node, post_order_rank, immediate_dominators }
247+
248+
let time = compute_access_time(start_node, &immediate_dominators);
249+
250+
Dominators { start_node, post_order_rank, immediate_dominators, time }
248251
}
249252

250253
/// Evaluate the link-eval virtual forest, providing the currently minimum semi
@@ -316,6 +319,7 @@ pub struct Dominators<N: Idx> {
316319
// possible to get its full list of dominators by looking up the dominator
317320
// of each dominator. (See the `impl Iterator for Iter` definition).
318321
immediate_dominators: IndexVec<N, Option<N>>,
322+
time: IndexVec<N, Time>,
319323
}
320324

321325
impl<Node: Idx> Dominators<Node> {
@@ -333,12 +337,7 @@ impl<Node: Idx> Dominators<Node> {
333337
/// See the `impl Iterator for Iter` definition to understand how this works.
334338
pub fn dominators(&self, node: Node) -> Iter<'_, Node> {
335339
assert!(self.is_reachable(node), "node {node:?} is not reachable");
336-
Iter { dominators: self, node: Some(node) }
337-
}
338-
339-
pub fn dominates(&self, dom: Node, node: Node) -> bool {
340-
// FIXME -- could be optimized by using post-order-rank
341-
self.dominators(node).any(|n| n == dom)
340+
Iter { dom_tree: self, node: Some(node) }
342341
}
343342

344343
/// Provide deterministic ordering of nodes such that, if any two nodes have a dominator
@@ -348,10 +347,22 @@ impl<Node: Idx> Dominators<Node> {
348347
pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
349348
self.post_order_rank[rhs].partial_cmp(&self.post_order_rank[lhs])
350349
}
350+
351+
/// Returns true if `a` dominates `b`.
352+
///
353+
/// # Panics
354+
///
355+
/// Panics if `b` is unreachable.
356+
pub fn dominates(&self, a: Node, b: Node) -> bool {
357+
let a = self.time[a];
358+
let b = self.time[b];
359+
assert!(b.start != 0, "node {b:?} is not reachable");
360+
a.start <= b.start && b.finish <= a.finish
361+
}
351362
}
352363

353364
pub struct Iter<'dom, Node: Idx> {
354-
dominators: &'dom Dominators<Node>,
365+
dom_tree: &'dom Dominators<Node>,
355366
node: Option<Node>,
356367
}
357368

@@ -360,10 +371,74 @@ impl<'dom, Node: Idx> Iterator for Iter<'dom, Node> {
360371

361372
fn next(&mut self) -> Option<Self::Item> {
362373
if let Some(node) = self.node {
363-
self.node = self.dominators.immediate_dominator(node);
374+
self.node = self.dom_tree.immediate_dominator(node);
364375
Some(node)
365376
} else {
366377
None
367378
}
368379
}
369380
}
381+
382+
/// Describes the number of vertices discovered at the time when processing of a particular vertex
383+
/// started and when it finished. Both values are zero for unreachable vertices.
384+
#[derive(Copy, Clone, Default, Debug)]
385+
struct Time {
386+
start: u32,
387+
finish: u32,
388+
}
389+
390+
fn compute_access_time<N: Idx>(
391+
start_node: N,
392+
immediate_dominators: &IndexSlice<N, Option<N>>,
393+
) -> IndexVec<N, Time> {
394+
// Transpose the dominator tree edges, so that child nodes of vertex v are stored in
395+
// node[edges[v].start..edges[v].end].
396+
let mut edges: IndexVec<N, std::ops::Range<u32>> =
397+
IndexVec::from_elem(0..0, immediate_dominators);
398+
for &idom in immediate_dominators.iter() {
399+
if let Some(idom) = idom {
400+
edges[idom].end += 1;
401+
}
402+
}
403+
let mut m = 0;
404+
for e in edges.iter_mut() {
405+
m += e.end;
406+
e.start = m;
407+
e.end = m;
408+
}
409+
let mut node = IndexVec::from_elem_n(Idx::new(0), m.try_into().unwrap());
410+
for (i, &idom) in immediate_dominators.iter_enumerated() {
411+
if let Some(idom) = idom {
412+
edges[idom].start -= 1;
413+
node[edges[idom].start] = i;
414+
}
415+
}
416+
417+
// Perform a depth-first search of the dominator tree. Record the number of vertices discovered
418+
// when vertex v is discovered first as time[v].start, and when its processing is finished as
419+
// time[v].finish.
420+
let mut time: IndexVec<N, Time> = IndexVec::from_elem(Time::default(), immediate_dominators);
421+
let mut stack = Vec::new();
422+
423+
let mut discovered = 1;
424+
stack.push(start_node);
425+
time[start_node].start = discovered;
426+
427+
while let Some(&i) = stack.last() {
428+
let e = &mut edges[i];
429+
if e.start == e.end {
430+
// Finish processing vertex i.
431+
time[i].finish = discovered;
432+
stack.pop();
433+
} else {
434+
let j = node[e.start];
435+
e.start += 1;
436+
// Start processing vertex j.
437+
discovered += 1;
438+
time[j].start = discovered;
439+
stack.push(j);
440+
}
441+
}
442+
443+
time
444+
}

compiler/rustc_middle/src/mir/basic_blocks.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ struct Cache {
2727
switch_sources: OnceCell<SwitchSources>,
2828
is_cyclic: OnceCell<bool>,
2929
postorder: OnceCell<Vec<BasicBlock>>,
30+
dominators: OnceCell<Dominators<BasicBlock>>,
3031
}
3132

3233
impl<'tcx> BasicBlocks<'tcx> {
@@ -41,8 +42,8 @@ impl<'tcx> BasicBlocks<'tcx> {
4142
*self.cache.is_cyclic.get_or_init(|| graph::is_cyclic(self))
4243
}
4344

44-
pub fn dominators(&self) -> Dominators<BasicBlock> {
45-
dominators(&self)
45+
pub fn dominators(&self) -> &Dominators<BasicBlock> {
46+
self.cache.dominators.get_or_init(|| dominators(self))
4647
}
4748

4849
/// Returns predecessors for each basic block.

compiler/rustc_mir_transform/src/coverage/graph.rs

+7-22
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use rustc_index::{IndexSlice, IndexVec};
99
use rustc_middle::mir::coverage::*;
1010
use rustc_middle::mir::{self, BasicBlock, BasicBlockData, Terminator, TerminatorKind};
1111

12+
use std::cmp::Ordering;
1213
use std::ops::{Index, IndexMut};
1314

1415
const ID_SEPARATOR: &str = ",";
@@ -212,8 +213,12 @@ impl CoverageGraph {
212213
}
213214

214215
#[inline(always)]
215-
pub fn dominators(&self) -> &Dominators<BasicCoverageBlock> {
216-
self.dominators.as_ref().unwrap()
216+
pub fn rank_partial_cmp(
217+
&self,
218+
a: BasicCoverageBlock,
219+
b: BasicCoverageBlock,
220+
) -> Option<Ordering> {
221+
self.dominators.as_ref().unwrap().rank_partial_cmp(a, b)
217222
}
218223
}
219224

@@ -650,26 +655,6 @@ pub(super) fn find_loop_backedges(
650655
let mut backedges = IndexVec::from_elem_n(Vec::<BasicCoverageBlock>::new(), num_bcbs);
651656

652657
// Identify loops by their backedges.
653-
//
654-
// The computational complexity is bounded by: n(s) x d where `n` is the number of
655-
// `BasicCoverageBlock` nodes (the simplified/reduced representation of the CFG derived from the
656-
// MIR); `s` is the average number of successors per node (which is most likely less than 2, and
657-
// independent of the size of the function, so it can be treated as a constant);
658-
// and `d` is the average number of dominators per node.
659-
//
660-
// The average number of dominators depends on the size and complexity of the function, and
661-
// nodes near the start of the function's control flow graph typically have less dominators
662-
// than nodes near the end of the CFG. Without doing a detailed mathematical analysis, I
663-
// think the resulting complexity has the characteristics of O(n log n).
664-
//
665-
// The overall complexity appears to be comparable to many other MIR transform algorithms, and I
666-
// don't expect that this function is creating a performance hot spot, but if this becomes an
667-
// issue, there may be ways to optimize the `dominates` algorithm (as indicated by an
668-
// existing `FIXME` comment in that code), or possibly ways to optimize it's usage here, perhaps
669-
// by keeping track of results for visited `BasicCoverageBlock`s if they can be used to short
670-
// circuit downstream `dominates` checks.
671-
//
672-
// For now, that kind of optimization seems unnecessarily complicated.
673658
for (bcb, _) in basic_coverage_blocks.iter_enumerated() {
674659
for &successor in &basic_coverage_blocks.successors[bcb] {
675660
if basic_coverage_blocks.dominates(successor, bcb) {

compiler/rustc_mir_transform/src/coverage/spans.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
344344
// before the dominated equal spans). When later comparing two spans in
345345
// order, the first will either dominate the second, or they will have no
346346
// dominator relationship.
347-
self.basic_coverage_blocks.dominators().rank_partial_cmp(a.bcb, b.bcb)
347+
self.basic_coverage_blocks.rank_partial_cmp(a.bcb, b.bcb)
348348
}
349349
} else {
350350
// Sort hi() in reverse order so shorter spans are attempted after longer spans.

compiler/rustc_mir_transform/src/ctfe_limit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ fn has_back_edge(
4747
return false;
4848
}
4949
// Check if any of the dominators of the node are also the node's successor.
50-
doms.dominators(node).any(|dom| node_data.terminator().successors().any(|succ| succ == dom))
50+
node_data.terminator().successors().any(|succ| doms.dominates(succ, node))
5151
}
5252

5353
fn insert_counter(basic_block_data: &mut BasicBlockData<'_>) {

compiler/rustc_mir_transform/src/ssa.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ pub struct SsaLocals {
3131
/// We often encounter MIR bodies with 1 or 2 basic blocks. In those cases, it's unnecessary to
3232
/// actually compute dominators, we can just compare block indices because bb0 is always the first
3333
/// block, and in any body all other blocks are always dominated by bb0.
34-
struct SmallDominators {
35-
inner: Option<Dominators<BasicBlock>>,
34+
struct SmallDominators<'a> {
35+
inner: Option<&'a Dominators<BasicBlock>>,
3636
}
3737

38-
impl SmallDominators {
38+
impl SmallDominators<'_> {
3939
fn dominates(&self, first: Location, second: Location) -> bool {
4040
if first.block == second.block {
4141
first.statement_index <= second.statement_index
@@ -198,14 +198,14 @@ enum LocationExtended {
198198
Arg,
199199
}
200200

201-
struct SsaVisitor {
202-
dominators: SmallDominators,
201+
struct SsaVisitor<'a> {
202+
dominators: SmallDominators<'a>,
203203
assignments: IndexVec<Local, Set1<LocationExtended>>,
204204
assignment_order: Vec<Local>,
205205
direct_uses: IndexVec<Local, u32>,
206206
}
207207

208-
impl<'tcx> Visitor<'tcx> for SsaVisitor {
208+
impl<'tcx> Visitor<'tcx> for SsaVisitor<'_> {
209209
fn visit_local(&mut self, local: Local, ctxt: PlaceContext, loc: Location) {
210210
match ctxt {
211211
PlaceContext::MutatingUse(MutatingUseContext::Projection)

0 commit comments

Comments
 (0)