Skip to content

Commit

Permalink
Auto merge of #112638 - lqd:rpo, r=cjgillot
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed Jun 18, 2023
2 parents 76fb0e3 + 08a9f25 commit 677710e
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 46 deletions.
7 changes: 2 additions & 5 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

use rustc_hir as hir;
use rustc_middle::mir;
use rustc_middle::mir::traversal::ReversePostorderIter;
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::subst::InternalSubsts;
Expand Down Expand Up @@ -53,9 +52,8 @@ impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> {
return;
}

let mut rpo = traversal::reverse_postorder(body);
let ccx = ConstCx::new(tcx, body);
let (mut temps, all_candidates) = collect_temps_and_candidates(&ccx, &mut rpo);
let (mut temps, all_candidates) = collect_temps_and_candidates(&ccx);

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

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

pub fn collect_temps_and_candidates<'tcx>(
ccx: &ConstCx<'_, 'tcx>,
rpo: &mut ReversePostorderIter<'_, 'tcx>,
) -> (IndexVec<Local, TempState>, Vec<Candidate>) {
let mut collector = Collector {
temps: IndexVec::from_elem(TempState::Undefined, &ccx.body.local_decls),
candidates: vec![],
ccx,
};
for (bb, data) in rpo {
for (bb, data) in traversal::reverse_postorder(ccx.body) {
collector.visit_basic_block_data(bb, data);
}
(collector.temps, collector.candidates)
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_middle/src/mir/basic_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct Cache {
predecessors: OnceCell<Predecessors>,
switch_sources: OnceCell<SwitchSources>,
is_cyclic: OnceCell<bool>,
postorder: OnceCell<Vec<BasicBlock>>,
reverse_postorder: OnceCell<Vec<BasicBlock>>,
dominators: OnceCell<Dominators<BasicBlock>>,
}

Expand Down Expand Up @@ -62,11 +62,14 @@ impl<'tcx> BasicBlocks<'tcx> {
})
}

/// Returns basic blocks in a postorder.
/// Returns basic blocks in a reverse postorder.
#[inline]
pub fn postorder(&self) -> &[BasicBlock] {
self.cache.postorder.get_or_init(|| {
Postorder::new(&self.basic_blocks, START_BLOCK).map(|(bb, _)| bb).collect()
pub fn reverse_postorder(&self) -> &[BasicBlock] {
self.cache.reverse_postorder.get_or_init(|| {
let mut rpo: Vec<_> =
Postorder::new(&self.basic_blocks, START_BLOCK).map(|(bb, _)| bb).collect();
rpo.reverse();
rpo
})
}

Expand Down
51 changes: 18 additions & 33 deletions compiler/rustc_middle/src/mir/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,6 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
}
}

pub fn postorder<'a, 'tcx>(body: &'a Body<'tcx>) -> Postorder<'a, 'tcx> {
Postorder::new(&body.basic_blocks, START_BLOCK)
}

impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> {
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);

Expand Down Expand Up @@ -219,6 +215,17 @@ impl<'a, 'tcx> Iterator for Postorder<'a, 'tcx> {
}
}

/// Creates an iterator over the `Body`'s basic blocks, that:
/// - returns basic blocks in a postorder,
/// - traverses the `BasicBlocks` CFG cache's reverse postorder backwards, and does not cache the
/// postorder itself.
pub fn postorder<'a, 'tcx>(
body: &'a Body<'tcx>,
) -> impl Iterator<Item = (BasicBlock, &'a BasicBlockData<'tcx>)> + ExactSizeIterator + DoubleEndedIterator
{
reverse_postorder(body).rev()
}

/// Reverse postorder traversal of a graph
///
/// Reverse postorder is the reverse order of a postorder traversal.
Expand Down Expand Up @@ -295,34 +302,12 @@ pub fn reachable_as_bitset(body: &Body<'_>) -> BitSet<BasicBlock> {
iter.visited
}

#[derive(Clone)]
pub struct ReversePostorderIter<'a, 'tcx> {
/// Creates an iterator over the `Body`'s basic blocks, that:
/// - returns basic blocks in a reverse postorder,
/// - makes use of the `BasicBlocks` CFG cache's reverse postorder.
pub fn reverse_postorder<'a, 'tcx>(
body: &'a Body<'tcx>,
blocks: &'a [BasicBlock],
idx: usize,
}

impl<'a, 'tcx> Iterator for ReversePostorderIter<'a, 'tcx> {
type Item = (BasicBlock, &'a BasicBlockData<'tcx>);

fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> {
if self.idx == 0 {
return None;
}
self.idx -= 1;

self.blocks.get(self.idx).map(|&bb| (bb, &self.body[bb]))
}

fn size_hint(&self) -> (usize, Option<usize>) {
(self.idx, Some(self.idx))
}
}

impl<'a, 'tcx> ExactSizeIterator for ReversePostorderIter<'a, 'tcx> {}

pub fn reverse_postorder<'a, 'tcx>(body: &'a Body<'tcx>) -> ReversePostorderIter<'a, 'tcx> {
let blocks = body.basic_blocks.postorder();
let len = blocks.len();
ReversePostorderIter { body, blocks, idx: len }
) -> impl Iterator<Item = (BasicBlock, &'a BasicBlockData<'tcx>)> + ExactSizeIterator + DoubleEndedIterator
{
body.basic_blocks.reverse_postorder().iter().map(|&bb| (bb, &body.basic_blocks[bb]))
}
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ impl<'tcx> MirPass<'tcx> for ConstProp {

// Traverse the body in reverse post-order, to ensure that `FullConstProp` locals are
// assigned before being read.
let postorder = body.basic_blocks.postorder().to_vec();
for bb in postorder.into_iter().rev() {
let rpo = body.basic_blocks.reverse_postorder().to_vec();
for bb in rpo {
let data = &mut body.basic_blocks.as_mut_preserves_cfg()[bb];
optimization_finder.visit_basic_block_data(bb, data);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/prettify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl<'tcx> MirPass<'tcx> for ReorderBasicBlocks {

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let rpo: IndexVec<BasicBlock, BasicBlock> =
body.basic_blocks.postorder().iter().copied().rev().collect();
body.basic_blocks.reverse_postorder().iter().copied().collect();
if rpo.iter().is_sorted() {
return;
}
Expand Down

0 comments on commit 677710e

Please sign in to comment.