No RPO pass; put RPO functionality in CFGInfo.

src/backend/localify.rs

@@ -59,7 +59,7 @@ impl<'a> Context<'a> {
 
         let mut live: HashMap<Value, usize> = HashMap::default();
         let mut block_starts: HashMap<Block, usize> = 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);

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<WasmBlock<'a>>,
     locals: Localifier,
@@ -34,17 +32,15 @@ impl<'a> WasmFuncBackend<'a> {
     pub fn new(body: &'a FunctionBody) -> Result<WasmFuncBackend<'a>> {
         log::debug!("Backend compiling:\n{}\n", body.display_verbose("| "));
         let cfg = CFGInfo::new(body);
-        let rpo = RPO::compute(body);
+        log::debug!("CFG:\n{:?}\n", cfg);
-        log::debug!("RPO:\n{:?}\n", rpo);
         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 } => {

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<Block>,
     loop_headers: HashSet<Block>,
     ctrl_stack: Vec<CtrlEntry>,
@@ -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<Self> {
+    pub fn new(body: &'a FunctionBody, cfg: &'b CFGInfo) -> anyhow::Result<Self> {
-        let (merge_nodes, loop_headers) =
+        let (merge_nodes, loop_headers) = Self::compute_merge_nodes_and_loop_headers(body, cfg)?;
-            Self::compute_merge_nodes_and_loop_headers(body, cfg, rpo)?;
         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<Block>, HashSet<Block>)> {
         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 (block_rpo, &block) in cfg.rpo.entries() {
-            for &succ in cfg.succs(block) {
+            for &succ in &body.blocks[block].succs {
-                let succ_rpo = rpo.rev[succ].unwrap();
+                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::<Vec<_>>();
         // 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(

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<Block, SmallVec<[Block; 4]>>,
-    /// Successors for each block.
-    pub block_succs: PerEntity<Block, SmallVec<[Block; 4]>>,
     /// Blocks that end in return.
     pub return_blocks: Vec<Block>,
-    /// Postorder traversal of blocks.
+    /// Reverse-postorder traversal of blocks.
-    pub postorder: Vec<Block>,
+    pub rpo: EntityVec<RPOIndex, Block>,
-    /// Position of each block in postorder, if reachable.
+    /// Position of each block in RPO, if reachable.
-    pub postorder_pos: PerEntity<Block, Option<usize>>,
+    pub rpo_pos: PerEntity<Block, Option<RPOIndex>>,
     /// Domtree parents, indexed by block.
     pub domtree: PerEntity<Block, Block>,
     /// 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<Block, SmallVec<[Block; 4]>> = PerEntity::default();
-        let mut block_succs: PerEntity<Block, SmallVec<[Block; 4]>> = 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();
+        let domtree =
-        for (i, block) in postorder.iter().enumerate() {
+            domtree::calculate(|block| &f.blocks[block].preds[..], &postorder[..], f.entry);
-            postorder_pos[*block] = Some(i);
-        }
-
-        let domtree = domtree::calculate(|block| &&block_preds[block], &postorder[..], f.entry);
 
         let mut domtree_children: PerEntity<Block, DomtreeChildren> = 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,
+            rpo,
-            postorder_pos,
+            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<Block> {
-        self.postorder.iter().cloned().rev().collect()
-    }
-
-    pub fn rpo_pos(&self, block: Block) -> Option<usize> {
-        self.postorder_pos[block].map(|fwd_pos| self.postorder.len() - 1 - fwd_pos)
-    }
 }

src/entity.rs

@@ -103,15 +103,15 @@ impl<Idx: EntityRef, T: Clone + Debug> EntityVec<Idx, T> {
         (0..self.0.len()).map(|index| Idx::new(index))
     }
 
-    pub fn values(&self) -> impl Iterator<Item = &T> {
+    pub fn values(&self) -> impl DoubleEndedIterator<Item = &T> {
         self.0.iter()
     }
 
-    pub fn values_mut(&mut self) -> impl Iterator<Item = &mut T> {
+    pub fn values_mut(&mut self) -> impl DoubleEndedIterator<Item = &mut T> {
         self.0.iter_mut()
     }
 
-    pub fn entries(&self) -> impl Iterator<Item = (Idx, &T)> {
+    pub fn entries(&self) -> impl DoubleEndedIterator<Item = (Idx, &T)> {
         self.0
             .iter()
             .enumerate()

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;
         }
     }

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);

src/passes.rs

@@ -3,4 +3,3 @@
 pub mod basic_opt;
 pub mod dom_pass;
 pub mod resolve_aliases;
-pub mod rpo;

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};

src/passes/rpo.rs

@@ -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<RPOIndex, Block>,
-    pub rev: PerEntity<Block, Option<RPOIndex>>,
-}
-
-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<Block>,
-        pending: &mut Vec<Block>,
-        pending_idx: &mut HashMap<Block, usize>,
-        postorder: &mut Vec<Block>,
-    ) {
-        // `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<Block> {
-        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");
-        }
-    }
-}