Skip to content

Commit 677710e

Browse files
committed
Auto merge of #112638 - lqd:rpo, r=cjgillot
Switch the BB CFG cache from postorder to RPO The `BasicBlocks` CFG cache is interesting: - it stores a postorder, but `traversal::postorder` doesn't use it - `traversal::reverse_postorder` does traverse the postorder cache backwards - we do more RPO traversals than postorder traversals (around 20x on the perf.rlo benchmarks IIRC) but it's not cached - a couple places here and there were manually reversing the non-cached postorder traversal This PR switches the order of the cache, and makes a bit more use of it. This is a tiny win locally, but it's also for consistency and aesthetics. r? `@ghost`
2 parents 76fb0e3 + 08a9f25 commit 677710e

File tree

5 files changed

+31
-46
lines changed

5 files changed

+31
-46
lines changed

compiler/rustc_const_eval/src/transform/promote_consts.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
1515
use rustc_hir as hir;
1616
use rustc_middle::mir;
17-
use rustc_middle::mir::traversal::ReversePostorderIter;
1817
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
1918
use rustc_middle::mir::*;
2019
use rustc_middle::ty::subst::InternalSubsts;
@@ -53,9 +52,8 @@ impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> {
5352
return;
5453
}
5554

56-
let mut rpo = traversal::reverse_postorder(body);
5755
let ccx = ConstCx::new(tcx, body);
58-
let (mut temps, all_candidates) = collect_temps_and_candidates(&ccx, &mut rpo);
56+
let (mut temps, all_candidates) = collect_temps_and_candidates(&ccx);
5957

6058
let promotable_candidates = validate_candidates(&ccx, &mut temps, &all_candidates);
6159

@@ -166,14 +164,13 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
166164

167165
pub fn collect_temps_and_candidates<'tcx>(
168166
ccx: &ConstCx<'_, 'tcx>,
169-
rpo: &mut ReversePostorderIter<'_, 'tcx>,
170167
) -> (IndexVec<Local, TempState>, Vec<Candidate>) {
171168
let mut collector = Collector {
172169
temps: IndexVec::from_elem(TempState::Undefined, &ccx.body.local_decls),
173170
candidates: vec![],
174171
ccx,
175172
};
176-
for (bb, data) in rpo {
173+
for (bb, data) in traversal::reverse_postorder(ccx.body) {
177174
collector.visit_basic_block_data(bb, data);
178175
}
179176
(collector.temps, collector.candidates)

compiler/rustc_middle/src/mir/basic_blocks.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ struct Cache {
2626
predecessors: OnceCell<Predecessors>,
2727
switch_sources: OnceCell<SwitchSources>,
2828
is_cyclic: OnceCell<bool>,
29-
postorder: OnceCell<Vec<BasicBlock>>,
29+
reverse_postorder: OnceCell<Vec<BasicBlock>>,
3030
dominators: OnceCell<Dominators<BasicBlock>>,
3131
}
3232

@@ -62,11 +62,14 @@ impl<'tcx> BasicBlocks<'tcx> {
6262
})
6363
}
6464

65-
/// Returns basic blocks in a postorder.
65+
/// Returns basic blocks in a reverse postorder.
6666
#[inline]
67-
pub fn postorder(&self) -> &[BasicBlock] {
68-
self.cache.postorder.get_or_init(|| {
69-
Postorder::new(&self.basic_blocks, START_BLOCK).map(|(bb, _)| bb).collect()
67+
pub fn reverse_postorder(&self) -> &[BasicBlock] {
68+
self.cache.reverse_postorder.get_or_init(|| {
69+
let mut rpo: Vec<_> =
70+
Postorder::new(&self.basic_blocks, START_BLOCK).map(|(bb, _)| bb).collect();
71+
rpo.reverse();
72+
rpo
7073
})
7174
}
7275

compiler/rustc_middle/src/mir/traversal.rs

+18-33
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,6 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
188188
}
189189
}
190190

191-
pub fn postorder<'a, 'tcx>(body: &'a Body<'tcx>) -> Postorder<'a, 'tcx> {
192-
Postorder::new(&body.basic_blocks, START_BLOCK)
193-
}
194-
195191
impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> {
196192
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);
197193

@@ -219,6 +215,17 @@ impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> {
219215
}
220216
}
221217

218+
/// Creates an iterator over the `Body`'s basic blocks, that:
219+
/// - returns basic blocks in a postorder,
220+
/// - traverses the `BasicBlocks` CFG cache's reverse postorder backwards, and does not cache the
221+
/// postorder itself.
222+
pub fn postorder<'a, 'tcx>(
223+
body: &'a Body<'tcx>,
224+
) -> impl Iterator<Item = (BasicBlock, &'a BasicBlockData<'tcx>)> + ExactSizeIterator + DoubleEndedIterator
225+
{
226+
reverse_postorder(body).rev()
227+
}
228+
222229
/// Reverse postorder traversal of a graph
223230
///
224231
/// Reverse postorder is the reverse order of a postorder traversal.
@@ -295,34 +302,12 @@ pub fn reachable_as_bitset(body: &Body<'_>) -> BitSet<BasicBlock> {
295302
iter.visited
296303
}
297304

298-
#[derive(Clone)]
299-
pub struct ReversePostorderIter<'a, 'tcx> {
305+
/// Creates an iterator over the `Body`'s basic blocks, that:
306+
/// - returns basic blocks in a reverse postorder,
307+
/// - makes use of the `BasicBlocks` CFG cache's reverse postorder.
308+
pub fn reverse_postorder<'a, 'tcx>(
300309
body: &'a Body<'tcx>,
301-
blocks: &'a [BasicBlock],
302-
idx: usize,
303-
}
304-
305-
impl<'a, 'tcx> Iterator for ReversePostorderIter<'a, 'tcx> {
306-
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);
307-
308-
fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> {
309-
if self.idx == 0 {
310-
return None;
311-
}
312-
self.idx -= 1;
313-
314-
self.blocks.get(self.idx).map(|&bb| (bb, &self.body[bb]))
315-
}
316-
317-
fn size_hint(&self) -> (usize, Option<usize>) {
318-
(self.idx, Some(self.idx))
319-
}
320-
}
321-
322-
impl<'a, 'tcx> ExactSizeIterator for ReversePostorderIter<'a, 'tcx> {}
323-
324-
pub fn reverse_postorder<'a, 'tcx>(body: &'a Body<'tcx>) -> ReversePostorderIter<'a, 'tcx> {
325-
let blocks = body.basic_blocks.postorder();
326-
let len = blocks.len();
327-
ReversePostorderIter { body, blocks, idx: len }
310+
) -> impl Iterator<Item = (BasicBlock, &'a BasicBlockData<'tcx>)> + ExactSizeIterator + DoubleEndedIterator
311+
{
312+
body.basic_blocks.reverse_postorder().iter().map(|&bb| (bb, &body.basic_blocks[bb]))
328313
}

compiler/rustc_mir_transform/src/const_prop.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ impl<'tcx> MirPass<'tcx> for ConstProp {
118118

119119
// Traverse the body in reverse post-order, to ensure that `FullConstProp` locals are
120120
// assigned before being read.
121-
let postorder = body.basic_blocks.postorder().to_vec();
122-
for bb in postorder.into_iter().rev() {
121+
let rpo = body.basic_blocks.reverse_postorder().to_vec();
122+
for bb in rpo {
123123
let data = &mut body.basic_blocks.as_mut_preserves_cfg()[bb];
124124
optimization_finder.visit_basic_block_data(bb, data);
125125
}

compiler/rustc_mir_transform/src/prettify.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ impl<'tcx> MirPass<'tcx> for ReorderBasicBlocks {
2424

2525
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
2626
let rpo: IndexVec<BasicBlock, BasicBlock> =
27-
body.basic_blocks.postorder().iter().copied().rev().collect();
27+
body.basic_blocks.reverse_postorder().iter().copied().collect();
2828
if rpo.iter().is_sorted() {
2929
return;
3030
}

0 commit comments

Comments
 (0)