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

Switch the BB CFG cache from postorder to RPO #112638

Merged
merged 8 commits into from
Jun 18, 2023
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
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 {
Comment on lines +121 to +122
Copy link
Member Author

Choose a reason for hiding this comment

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

@tmiasko This is where it could have been cool to avoid this existing allocation, as it's not easy to combine lazy RPO + as_mut_preserves_cfg. Probably not worth it just for this single call-site though, I don't remember seeing this elsewhere.

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