From 510f833da263467bea4cecca285ae6acd4fe96fa Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Thu, 1 Dec 2022 18:57:00 -0800 Subject: [PATCH] No RPO pass; put RPO functionality in CFGInfo. --- src/backend/localify.rs | 4 +- src/backend/mod.rs | 12 ++- src/backend/stackify.rs | 24 +++--- src/cfg/mod.rs | 77 +++++------------- src/entity.rs | 6 +- src/frontend.rs | 11 +-- src/ir/func.rs | 2 +- src/passes.rs | 1 - src/passes/dom_pass.rs | 2 +- src/passes/rpo.rs | 174 ---------------------------------------- 10 files changed, 50 insertions(+), 263 deletions(-) delete mode 100644 src/passes/rpo.rs diff --git a/src/backend/localify.rs b/src/backend/localify.rs index f7585fc..fa742ea 100644 --- a/src/backend/localify.rs +++ b/src/backend/localify.rs @@ -59,7 +59,7 @@ impl<'a> Context<'a> { let mut live: HashMap = HashMap::default(); let mut block_starts: HashMap = HashMap::default(); - for &block in &self.cfg.postorder { + for &block in self.cfg.rpo.values().rev() { block_starts.insert(block, point); self.body.blocks[block].terminator.visit_uses(|u| { @@ -86,7 +86,7 @@ impl<'a> Context<'a> { // // Note that we do this *after* inserting our own start // above, so we handle self-loops properly. - for &pred in self.cfg.preds(block) { + for &pred in &self.body.blocks[block].preds { if let Some(&start) = block_starts.get(&pred) { for live_start in live.values_mut() { *live_start = std::cmp::min(*live_start, start); diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 0936cfc..db5b7e4 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -3,7 +3,6 @@ use crate::cfg::CFGInfo; use crate::entity::EntityRef; use crate::ir::{ExportKind, FuncDecl, FunctionBody, ImportKind, Module, Type, Value, ValueDef}; -use crate::passes::rpo::RPO; use crate::Operator; use anyhow::Result; use rayon::prelude::*; @@ -18,7 +17,6 @@ use localify::Localifier; pub struct WasmFuncBackend<'a> { body: &'a FunctionBody, - rpo: RPO, trees: Trees, ctrl: Vec>, locals: Localifier, @@ -34,17 +32,15 @@ impl<'a> WasmFuncBackend<'a> { pub fn new(body: &'a FunctionBody) -> Result> { log::debug!("Backend compiling:\n{}\n", body.display_verbose("| ")); let cfg = CFGInfo::new(body); - let rpo = RPO::compute(body); - log::debug!("RPO:\n{:?}\n", rpo); + log::debug!("CFG:\n{:?}\n", cfg); let trees = Trees::compute(body); log::debug!("Trees:\n{:?}\n", trees); - let ctrl = StackifyContext::new(body, &cfg, &rpo)?.compute(); + let ctrl = StackifyContext::new(body, &cfg)?.compute(); log::debug!("Ctrl:\n{:?}\n", ctrl); let locals = Localifier::compute(body, &cfg, &trees); log::debug!("Locals:\n{:?}\n", locals); Ok(WasmFuncBackend { body, - rpo, trees, ctrl, locals, @@ -148,7 +144,9 @@ impl<'a> WasmFuncBackend<'a> { if self.trees.owner.contains_key(&inst) { continue; } - self.lower_inst(inst, /* root = */ true, func); + if let &ValueDef::Operator(..) = &self.body.values[inst] { + self.lower_inst(inst, /* root = */ true, func); + } } } WasmBlock::BlockParams { from, to } => { diff --git a/src/backend/stackify.rs b/src/backend/stackify.rs index 90bd6ee..aec02ea 100644 --- a/src/backend/stackify.rs +++ b/src/backend/stackify.rs @@ -12,7 +12,6 @@ use crate::cfg::CFGInfo; use crate::entity::EntityRef; use crate::ir::{Block, BlockTarget, FunctionBody, Terminator, Type, Value}; -use crate::passes::rpo::RPO; use std::collections::HashSet; use std::convert::TryFrom; @@ -75,7 +74,6 @@ impl WasmLabel { pub struct Context<'a, 'b> { body: &'a FunctionBody, cfg: &'b CFGInfo, - rpo: &'b RPO, merge_nodes: HashSet, loop_headers: HashSet, ctrl_stack: Vec, @@ -99,13 +97,11 @@ impl CtrlEntry { } impl<'a, 'b> Context<'a, 'b> { - pub fn new(body: &'a FunctionBody, cfg: &'b CFGInfo, rpo: &'b RPO) -> anyhow::Result { - let (merge_nodes, loop_headers) = - Self::compute_merge_nodes_and_loop_headers(body, cfg, rpo)?; + pub fn new(body: &'a FunctionBody, cfg: &'b CFGInfo) -> anyhow::Result { + let (merge_nodes, loop_headers) = Self::compute_merge_nodes_and_loop_headers(body, cfg)?; Ok(Self { body, cfg, - rpo, merge_nodes, loop_headers, ctrl_stack: vec![], @@ -121,15 +117,16 @@ impl<'a, 'b> Context<'a, 'b> { fn compute_merge_nodes_and_loop_headers( body: &FunctionBody, cfg: &CFGInfo, - rpo: &RPO, ) -> anyhow::Result<(HashSet, HashSet)> { let mut loop_headers = HashSet::new(); let mut branched_once = HashSet::new(); let mut merge_nodes = HashSet::new(); - for (block_rpo, &block) in rpo.order.entries() { - for &succ in cfg.succs(block) { - let succ_rpo = rpo.rev[succ].unwrap(); + for (block_rpo, &block) in cfg.rpo.entries() { + for &succ in &body.blocks[block].succs { + log::trace!("block {} rpo {} has succ {}", block, block_rpo, succ); + let succ_rpo = cfg.rpo_pos[succ].unwrap(); + log::trace!(" -> succ rpo {}", succ_rpo); if succ_rpo <= block_rpo { if !cfg.dominates(succ, block) { anyhow::bail!("Irreducible control flow: edge from {} to {}", block, succ); @@ -147,7 +144,7 @@ impl<'a, 'b> Context<'a, 'b> { // Make any `select` target a "merge node" too, so it gets its // own block. - for &block in rpo.order.values() { + for &block in cfg.rpo.values() { if let &Terminator::Select { ref targets, ref default, @@ -171,7 +168,8 @@ impl<'a, 'b> Context<'a, 'b> { .filter(|child| self.merge_nodes.contains(&child)) .collect::>(); // Sort merge nodes so highest RPO number comes first. - merge_node_children.sort_unstable_by_key(|&block| std::cmp::Reverse(self.rpo.rev[block])); + merge_node_children + .sort_unstable_by_key(|&block| std::cmp::Reverse(self.cfg.rpo_pos[block])); let is_loop_header = self.loop_headers.contains(&block); @@ -216,7 +214,7 @@ impl<'a, 'b> Context<'a, 'b> { // the target is either a merge block, or is a backward branch // (by RPO number). if self.merge_nodes.contains(&target.block) - || self.rpo.rev[target.block] <= self.rpo.rev[source] + || self.cfg.rpo_pos[target.block] <= self.cfg.rpo_pos[source] { let index = self.resolve_target(target.block); self.do_blockparam_transfer( diff --git a/src/cfg/mod.rs b/src/cfg/mod.rs index fdf624b..cef0a21 100644 --- a/src/cfg/mod.rs +++ b/src/cfg/mod.rs @@ -3,27 +3,25 @@ // Borrowed from regalloc2's cfg.rs, which is also Apache-2.0 with // LLVM exception. -use crate::entity::{EntityRef, PerEntity}; +use crate::declare_entity; +use crate::entity::{EntityRef, EntityVec, PerEntity}; use crate::ir::{Block, FunctionBody, Terminator, Value, ValueDef}; -use smallvec::SmallVec; pub mod domtree; pub mod postorder; +declare_entity!(RPOIndex, "rpo"); + #[derive(Clone, Debug)] pub struct CFGInfo { /// Entry block. pub entry: Block, - /// Predecessors for each block. - pub block_preds: PerEntity>, - /// Successors for each block. - pub block_succs: PerEntity>, /// Blocks that end in return. pub return_blocks: Vec, - /// Postorder traversal of blocks. - pub postorder: Vec, - /// Position of each block in postorder, if reachable. - pub postorder_pos: PerEntity>, + /// Reverse-postorder traversal of blocks. + pub rpo: EntityVec, + /// Position of each block in RPO, if reachable. + pub rpo_pos: PerEntity>, /// Domtree parents, indexed by block. pub domtree: PerEntity, /// Domtree children. @@ -58,15 +56,6 @@ impl<'a> Iterator for DomtreeChildIter<'a> { impl CFGInfo { pub fn new(f: &FunctionBody) -> CFGInfo { - let mut block_preds: PerEntity> = PerEntity::default(); - let mut block_succs: PerEntity> = PerEntity::default(); - for (block, block_def) in f.blocks.entries() { - block_def.terminator.visit_successors(|succ| { - block_preds[succ].push(block); - block_succs[block].push(succ); - }); - } - let mut return_blocks = vec![]; for (block_id, block) in f.blocks.entries() { if let Terminator::Return { .. } = &block.terminator { @@ -74,14 +63,10 @@ impl CFGInfo { } } - let postorder = postorder::calculate(f.entry, |block| &block_succs[block]); + let postorder = postorder::calculate(f.entry, |block| &f.blocks[block].succs[..]); - let mut postorder_pos = PerEntity::default(); - for (i, block) in postorder.iter().enumerate() { - postorder_pos[*block] = Some(i); - } - - let domtree = domtree::calculate(|block| &&block_preds[block], &postorder[..], f.entry); + let domtree = + domtree::calculate(|block| &f.blocks[block].preds[..], &postorder[..], f.entry); let mut domtree_children: PerEntity = PerEntity::default(); for block in f.blocks.iter().rev() { @@ -108,13 +93,19 @@ impl CFGInfo { def_block[value] = def_block[underlying_value]; } + let mut rpo = postorder; + rpo.reverse(); + let rpo = EntityVec::from(rpo); + let mut rpo_pos = PerEntity::default(); + for (rpo, &block) in rpo.entries() { + rpo_pos[block] = Some(rpo); + } + CFGInfo { entry: f.entry, - block_preds, - block_succs, return_blocks, - postorder, - postorder_pos, + rpo, + rpo_pos, domtree, domtree_children, def_block, @@ -131,30 +122,4 @@ impl CFGInfo { block: self.domtree_children[block].child, } } - - pub fn succs(&self, block: Block) -> &[Block] { - &self.block_succs[block] - } - - pub fn preds(&self, block: Block) -> &[Block] { - &self.block_preds[block] - } - - pub fn pred_count_with_entry(&self, block: Block) -> usize { - let is_entry = block == self.entry; - self.preds(block).len() + if is_entry { 1 } else { 0 } - } - - pub fn succ_count_with_return(&self, block: Block) -> usize { - let is_return = self.return_blocks.binary_search(&block).is_ok(); - self.succs(block).len() + if is_return { 1 } else { 0 } - } - - pub fn rpo(&self) -> Vec { - self.postorder.iter().cloned().rev().collect() - } - - pub fn rpo_pos(&self, block: Block) -> Option { - self.postorder_pos[block].map(|fwd_pos| self.postorder.len() - 1 - fwd_pos) - } } diff --git a/src/entity.rs b/src/entity.rs index 8838bca..fb03fca 100644 --- a/src/entity.rs +++ b/src/entity.rs @@ -103,15 +103,15 @@ impl EntityVec { (0..self.0.len()).map(|index| Idx::new(index)) } - pub fn values(&self) -> impl Iterator { + pub fn values(&self) -> impl DoubleEndedIterator { self.0.iter() } - pub fn values_mut(&mut self) -> impl Iterator { + pub fn values_mut(&mut self) -> impl DoubleEndedIterator { self.0.iter_mut() } - pub fn entries(&self) -> impl Iterator { + pub fn entries(&self) -> impl DoubleEndedIterator { self.0 .iter() .enumerate() diff --git a/src/frontend.rs b/src/frontend.rs index 69b54d3..d16464b 100644 --- a/src/frontend.rs +++ b/src/frontend.rs @@ -1397,7 +1397,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { args, }; self.body - .end_block(self.cur_block, Terminator::Br { target }); + .set_terminator(self.cur_block, Terminator::Br { target }); } } @@ -1420,7 +1420,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { if self.reachable { let if_true_args = if_true_args.to_vec(); let if_false_args = if_false_args.to_vec(); - self.body.end_block( + self.body.set_terminator( self.cur_block, Terminator::CondBr { cond, @@ -1468,7 +1468,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { args: default_args, }; - self.body.end_block( + self.body.set_terminator( self.cur_block, Terminator::Select { value: index, @@ -1489,7 +1489,7 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { if self.reachable { let values = values.to_vec(); self.body - .end_block(self.cur_block, Terminator::Return { values }); + .set_terminator(self.cur_block, Terminator::Return { values }); self.reachable = false; } } @@ -1501,7 +1501,8 @@ impl<'a, 'b> FunctionBodyBuilder<'a, 'b> { self.reachable ); if self.reachable { - self.body.end_block(self.cur_block, Terminator::Unreachable); + self.body + .set_terminator(self.cur_block, Terminator::Unreachable); self.reachable = false; } } diff --git a/src/ir/func.rs b/src/ir/func.rs index 611abf1..0dd0a6a 100644 --- a/src/ir/func.rs +++ b/src/ir/func.rs @@ -169,7 +169,7 @@ impl FunctionBody { self.value_blocks[value] = block; } - pub fn end_block(&mut self, block: Block, terminator: Terminator) { + pub fn set_terminator(&mut self, block: Block, terminator: Terminator) { log::trace!("block {} terminator {:?}", block, terminator); terminator.visit_successors(|succ| { self.add_edge(block, succ); diff --git a/src/passes.rs b/src/passes.rs index ae8e380..0f55dcf 100644 --- a/src/passes.rs +++ b/src/passes.rs @@ -3,4 +3,3 @@ pub mod basic_opt; pub mod dom_pass; pub mod resolve_aliases; -pub mod rpo; diff --git a/src/passes/dom_pass.rs b/src/passes/dom_pass.rs index 75fb03a..e4a6b01 100644 --- a/src/passes/dom_pass.rs +++ b/src/passes/dom_pass.rs @@ -1,4 +1,4 @@ -//! Domtree-based pass. +//! Simple framework for a domtree-based pass. use crate::cfg::CFGInfo; use crate::ir::{Block, FunctionBody}; diff --git a/src/passes/rpo.rs b/src/passes/rpo.rs deleted file mode 100644 index a412853..0000000 --- a/src/passes/rpo.rs +++ /dev/null @@ -1,174 +0,0 @@ -//! Reorder-into-RPO pass. -//! -//! The RPO sort order we choose is quite special: we want loop bodies -//! to be placed contiguously, without blocks that do not belong to -//! the loop in the middle. -//! -//! Consider the following CFG: -//! -//! ```plain -//! 1 -//! | -//! 2 <-. -//! / | | -//! | 3 --' -//! | | -//! `> 4 -//! | -//! 5 -//! ``` -//! -//! A normal RPO sort may produce 1, 2, 4, 5, 3 or 1, 2, 3, 4, 5 -//! depending on which child order it chooses from block 2. (If it -//! visits 3 first, it will emit it first in postorder hence it comes -//! last.) -//! -//! One way of ensuring we get the right order would be to compute the -//! loop nest and make note of loops when choosing children to visit, -//! but we really would rather not do that, since we may not otherwise -//! need it. -//! -//! Instead, we keep a "pending" list: as we have nodes on the stack -//! during postorder traversal, we keep a list of other children that -//! we will visit once we get back to a given level. If another node -//! is pending, and is a successor we are considering, we visit it -//! *first* in postorder, so it is last in RPO. This is a way to -//! ensure that (e.g.) block 4 above is visited first when considering -//! successors of block 2. - -use crate::declare_entity; -use crate::entity::{EntityRef, EntityVec, PerEntity}; -use crate::ir::{Block, FunctionBody}; -use std::collections::{HashMap, HashSet}; - -declare_entity!(RPOIndex, "rpo"); - -impl RPOIndex { - pub fn prev(self) -> RPOIndex { - RPOIndex::from(self.0.checked_sub(1).unwrap()) - } -} - -#[derive(Clone, Debug, Default)] -pub struct RPO { - pub order: EntityVec, - pub rev: PerEntity>, -} - -impl RPO { - pub fn compute(body: &FunctionBody) -> RPO { - let mut postorder = vec![]; - let mut visited = HashSet::new(); - let mut pending = vec![]; - let mut pending_idx = HashMap::new(); - visited.insert(body.entry); - Self::visit( - body, - body.entry, - &mut visited, - &mut pending, - &mut pending_idx, - &mut postorder, - ); - postorder.reverse(); - let order = EntityVec::from(postorder); - - let mut rev = PerEntity::default(); - for (rpo_index, &block) in order.entries() { - rev[block] = Some(rpo_index); - } - - RPO { order, rev } - } - - fn visit( - body: &FunctionBody, - block: Block, - visited: &mut HashSet, - pending: &mut Vec, - pending_idx: &mut HashMap, - postorder: &mut Vec, - ) { - // `pending` is a Vec, not a Set; we prioritize based on - // position (first in pending go first in postorder -> last in - // RPO). A case with nested loops to show why this matters: - // - // TODO example - - let pending_top = pending.len(); - pending.extend(body.blocks[block].succs.iter().copied()); - - // Sort new entries in `pending` by index at which they appear - // earlier. Those that don't appear in `pending` at all should - // be visited last (to appear in RPO first), so we want `None` - // values to sort first here (hence the "unwrap or MAX" - // idiom). Then those that appear earlier in `pending` should - // be visited earlier here to appear later in RPO, so they - // sort later. - pending[pending_top..] - .sort_by_key(|entry| pending_idx.get(entry).copied().unwrap_or(usize::MAX)); - - // Above we placed items in order they are to be visited; - // below we pop off the end, so we reverse here. - pending[pending_top..].reverse(); - - // Now update indices in `pending_idx`: insert entries for - // those seqs not yet present. - for i in pending_top..pending.len() { - pending_idx.entry(pending[i]).or_insert(i); - } - - for _ in 0..(pending.len() - pending_top) { - let succ = pending.pop().unwrap(); - if pending_idx.get(&succ) == Some(&pending.len()) { - pending_idx.remove(&succ); - } - - if visited.insert(succ) { - Self::visit(body, succ, visited, pending, pending_idx, postorder); - } - } - postorder.push(block); - } - - fn map_block(&self, block: Block) -> Option { - Some(Block::new(self.rev[block]?.index())) - } -} - -pub fn run(body: &mut FunctionBody) { - let rpo = RPO::compute(body); - // Remap entry block. - body.entry = rpo - .map_block(body.entry) - .expect("Entry block must be in RPO sequence"); - // Reorder blocks. - let mut block_data = std::mem::take(&mut body.blocks).into_vec(); - let mut new_block_data = vec![]; - for block in rpo.order.values().copied() { - new_block_data.push(std::mem::take(&mut block_data[block.index()])); - } - body.blocks = EntityVec::from(new_block_data); - // Rewrite references in each terminator, pred and succ list. - for block in body.blocks.values_mut() { - block.terminator.update_targets(|target| { - target.block = rpo - .map_block(target.block) - .expect("Target of reachable block must be reachable"); - }); - block.preds.retain_mut(|pred| { - if let Some(new_pred) = rpo.map_block(*pred) { - *pred = new_pred; - true - } else { - // Some preds may be unreachable, so are not in RPO. - false - } - }); - for succ in &mut block.succs { - *succ = rpo - .map_block(*succ) - .expect("Succ of reachable block must be reachable"); - } - } -}