Merge pull request #18 from Amanieu/blockparam

Rework the API for outgoing blockparams

src/cfg.rs

@@ -101,7 +101,7 @@ impl CFGInfo {
             }
             if require_no_branch_args {
                 let last = f.block_insns(block).last();
-                if f.branch_blockparam_arg_offset(block, last) > 0 {
+                if !f.inst_operands(last).is_empty() {
                     return Err(RegAllocError::DisallowedBranchArg(last));
                 }
             }

src/fuzzing/func.rs

@@ -13,7 +13,6 @@ use arbitrary::{Arbitrary, Unstructured};
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum InstOpcode {
-    Phi,
     Op,
     Ret,
     Branch,
@@ -40,14 +39,10 @@ impl InstData {
             is_safepoint: false,
         }
     }
-    pub fn branch(uses: &[usize]) -> InstData {
+    pub fn branch() -> InstData {
-        let mut operands = vec![];
-        for &u in uses {
-            operands.push(Operand::reg_use(VReg::new(u, RegClass::Int)));
-        }
         InstData {
             op: InstOpcode::Branch,
-            operands,
+            operands: vec![],
             clobbers: vec![],
             is_safepoint: false,
         }
@@ -68,7 +63,8 @@ pub struct Func {
     blocks: Vec<InstRange>,
     block_preds: Vec<Vec<Block>>,
     block_succs: Vec<Vec<Block>>,
-    block_params: Vec<Vec<VReg>>,
+    block_params_in: Vec<Vec<VReg>>,
+    block_params_out: Vec<Vec<Vec<VReg>>>,
     num_vregs: usize,
     reftype_vregs: Vec<VReg>,
 }
@@ -100,7 +96,7 @@ impl Function for Func {
     }
 
     fn block_params(&self, block: Block) -> &[VReg] {
-        &self.block_params[block.index()][..]
+        &self.block_params_in[block.index()][..]
     }
 
     fn is_ret(&self, insn: Inst) -> bool {
@@ -111,10 +107,8 @@ impl Function for Func {
         self.insts[insn.index()].op == InstOpcode::Branch
     }
 
-    fn branch_blockparam_arg_offset(&self, _: Block, _: Inst) -> usize {
+    fn branch_blockparams(&self, block: Block, _: Inst, succ: usize) -> &[VReg] {
-        // Branch blockparam args always start at zero for this
+        &self.block_params_out[block.index()][succ][..]
-        // Function implementation.
-        0
     }
 
     fn requires_refs_on_stack(&self, insn: Inst) -> bool {
@@ -164,7 +158,8 @@ impl FuncBuilder {
             f: Func {
                 block_preds: vec![],
                 block_succs: vec![],
-                block_params: vec![],
+                block_params_in: vec![],
+                block_params_out: vec![],
                 insts: vec![],
                 blocks: vec![],
                 num_vregs: 0,
@@ -181,7 +176,8 @@ impl FuncBuilder {
             .push(InstRange::forward(Inst::new(0), Inst::new(0)));
         self.f.block_preds.push(vec![]);
         self.f.block_succs.push(vec![]);
-        self.f.block_params.push(vec![]);
+        self.f.block_params_in.push(vec![]);
+        self.f.block_params_out.push(vec![]);
         self.insts_per_block.push(vec![]);
         b
     }
@@ -195,8 +191,12 @@ impl FuncBuilder {
         self.f.block_preds[to.index()].push(from);
     }
 
-    pub fn set_block_params(&mut self, block: Block, params: &[VReg]) {
+    pub fn set_block_params_in(&mut self, block: Block, params: &[VReg]) {
-        self.f.block_params[block.index()] = params.iter().cloned().collect();
+        self.f.block_params_in[block.index()] = params.iter().cloned().collect();
+    }
+
+    pub fn set_block_params_out(&mut self, block: Block, params: Vec<Vec<VReg>>) {
+        self.f.block_params_out[block.index()] = params;
     }
 
     fn compute_doms(&mut self) {
@@ -388,7 +388,7 @@ impl Func {
                 }
             }
             vregs_by_block_to_be_defined.last_mut().unwrap().reverse();
-            builder.set_block_params(Block::new(block), &block_params[block][..]);
+            builder.set_block_params_in(Block::new(block), &block_params[block][..]);
         }
 
         for block in 0..num_blocks {
@@ -510,9 +510,10 @@ impl Func {
             // Define the branch with blockparam args that must end
             // the block.
             if builder.f.block_succs[block].len() > 0 {
-                let mut args = vec![];
+                let mut params = vec![];
                 for &succ in &builder.f.block_succs[block] {
-                    for _ in 0..builder.f.block_params[succ.index()].len() {
+                    let mut args = vec![];
+                    for _ in 0..builder.f.block_params_in[succ.index()].len() {
                         let dom_block = choose_dominating_block(
                             &builder.idom[..],
                             Block::new(block),
@@ -524,10 +525,12 @@ impl Func {
                         } else {
                             u.choose(&avail[..])?
                         };
-                        args.push(vreg.vreg());
+                        args.push(*vreg);
                     }
+                    params.push(args);
                 }
-                builder.add_inst(Block::new(block), InstData::branch(&args[..]));
+                builder.set_block_params_out(Block::new(block), params);
+                builder.add_inst(Block::new(block), InstData::branch());
             } else {
                 builder.add_inst(Block::new(block), InstData::ret());
             }
@@ -552,15 +555,29 @@ impl std::fmt::Debug for Func {
                 .iter()
                 .map(|b| b.index())
                 .collect::<Vec<_>>();
-            let params = self.block_params[i]
+            let params_in = self.block_params_in[i]
                 .iter()
                 .map(|v| format!("v{}", v.vreg()))
                 .collect::<Vec<_>>()
                 .join(", ");
+            let params_out = self.block_params_out[i]
+                .iter()
+                .enumerate()
+                .map(|(succ_idx, vec)| {
+                    let succ = self.block_succs[i][succ_idx];
+                    let params = vec
+                        .iter()
+                        .map(|v| format!("v{}", v.vreg()))
+                        .collect::<Vec<_>>()
+                        .join(", ");
+                    format!("block{}({})", succ.index(), params)
+                })
+                .collect::<Vec<_>>()
+                .join(", ");
             write!(
                 f,
                 "  block{}({}): # succs:{:?} preds:{:?}\n",
-                i, params, succs, preds
+                i, params_in, succs, preds
             )?;
             for inst in blockrange.iter() {
                 if self.requires_refs_on_stack(inst) {
@@ -574,6 +591,9 @@ impl std::fmt::Debug for Func {
                     self.insts[inst.index()].operands,
                     self.insts[inst.index()].clobbers
                 )?;
+                if let InstOpcode::Branch = self.insts[inst.index()].op {
+                    write!(f, "    params: {}\n", params_out)?;
+                }
             }
         }
         write!(f, "}}\n")?;

src/ion/liveranges.rs

@@ -315,6 +315,7 @@ impl<'a, F: Function> Env<'a, F> {
         while !workqueue.is_empty() {
             let block = workqueue.pop_front().unwrap();
             workqueue_set.remove(&block);
+            let insns = self.func.block_insns(block);
 
             log::trace!("computing liveins for block{}", block.index());
 
@@ -323,7 +324,16 @@ impl<'a, F: Function> Env<'a, F> {
             let mut live = self.liveouts[block.index()].clone();
             log::trace!(" -> initial liveout set: {:?}", live);
 
-            for inst in self.func.block_insns(block).rev().iter() {
+            // Include outgoing blockparams in the initial live set.
+            if self.func.is_branch(insns.last()) {
+                for i in 0..self.func.block_succs(block).len() {
+                    for param in self.func.branch_blockparams(block, insns.last(), i) {
+                        live.set(param.vreg(), true);
+                    }
+                }
+            }
+
+            for inst in insns.rev().iter() {
                 if let Some((src, dst)) = self.func.is_move(inst) {
                     live.set(dst.vreg().vreg(), false);
                     live.set(src.vreg().vreg(), true);
@@ -399,12 +409,33 @@ impl<'a, F: Function> Env<'a, F> {
 
         for i in (0..self.func.num_blocks()).rev() {
             let block = Block::new(i);
+            let insns = self.func.block_insns(block);
 
             self.stats.livein_blocks += 1;
 
             // Init our local live-in set.
             let mut live = self.liveouts[block.index()].clone();
 
+            // If the last instruction is a branch (rather than
+            // return), create blockparam_out entries.
+            if self.func.is_branch(insns.last()) {
+                for (i, &succ) in self.func.block_succs(block).iter().enumerate() {
+                    let blockparams_in = self.func.block_params(succ);
+                    let blockparams_out = self.func.branch_blockparams(block, insns.last(), i);
+                    for (&blockparam_in, &blockparam_out) in
+                        blockparams_in.iter().zip(blockparams_out)
+                    {
+                        let blockparam_out = VRegIndex::new(blockparam_out.vreg());
+                        let blockparam_in = VRegIndex::new(blockparam_in.vreg());
+                        self.blockparam_outs
+                            .push((blockparam_out, block, succ, blockparam_in));
+
+                        // Include outgoing blockparams in the initial live set.
+                        live.set(blockparam_out.index(), true);
+                    }
+                }
+            }
+
             // Initially, registers are assumed live for the whole block.
             for vreg in live.iter() {
                 let range = CodeRange {
@@ -426,24 +457,6 @@ impl<'a, F: Function> Env<'a, F> {
                 self.vregs[param.vreg()].blockparam = block;
             }
 
-            let insns = self.func.block_insns(block);
-
-            // If the last instruction is a branch (rather than
-            // return), create blockparam_out entries.
-            if self.func.is_branch(insns.last()) {
-                let operands = self.func.inst_operands(insns.last());
-                let mut i = self.func.branch_blockparam_arg_offset(block, insns.last());
-                for &succ in self.func.block_succs(block) {
-                    for &blockparam in self.func.block_params(succ) {
-                        let from_vreg = VRegIndex::new(operands[i].vreg().vreg());
-                        let blockparam_vreg = VRegIndex::new(blockparam.vreg());
-                        self.blockparam_outs
-                            .push((from_vreg, block, succ, blockparam_vreg));
-                        i += 1;
-                    }
-                }
-            }
-
             // For each instruction, in reverse order, process
             // operands and clobbers.
             for inst in insns.rev().iter() {
@@ -892,13 +905,6 @@ impl<'a, F: Function> Env<'a, F> {
                             (OperandKind::Def, OperandPos::Early) => ProgPoint::before(inst),
                             (OperandKind::Def, OperandPos::Late) => ProgPoint::after(inst),
                             (OperandKind::Use, OperandPos::Late) => ProgPoint::after(inst),
-                            // If this is a branch, extend `pos` to
-                            // the end of the block. (Branch uses are
-                            // blockparams and need to be live at the
-                            // end of the block.)
-                            (OperandKind::Use, _) if self.func.is_branch(inst) => {
-                                self.cfginfo.block_exit[block.index()]
-                            }
                             // If there are any reused inputs in this
                             // instruction, and this is *not* the
                             // reused input, force `pos` to

src/lib.rs

@@ -860,21 +860,14 @@ pub trait Function {
     fn is_ret(&self, insn: Inst) -> bool;
 
     /// Determine whether an instruction is the end-of-block
-    /// branch. If so, its operands at the indices given by
+    /// branch.
-    /// `branch_blockparam_arg_offset()` below *must* be the block
-    /// parameters for each of its block's `block_succs` successor
-    /// blocks, in order.
     fn is_branch(&self, insn: Inst) -> bool;
 
     /// If `insn` is a branch at the end of `block`, returns the
-    /// operand index at which outgoing blockparam arguments are
+    /// outgoing blockparam arguments for the given successor. The
-    /// found. Starting at this index, blockparam arguments for each
+    /// number of arguments must match the number incoming blockparams
-    /// successor block's blockparams, in order, must be found.
+    /// for each respective successor block.
-    ///
+    fn branch_blockparams(&self, block: Block, insn: Inst, succ_idx: usize) -> &[VReg];
-    /// It is an error if `self.inst_operands(insn).len() -
-    /// self.branch_blockparam_arg_offset(insn)` is not exactly equal
-    /// to the sum of blockparam counts for all successor blocks.
-    fn branch_blockparam_arg_offset(&self, block: Block, insn: Inst) -> usize;
 
     /// Determine whether an instruction requires all reference-typed
     /// values to be placed onto the stack. For these instructions,

src/ssa.rs

@@ -71,15 +71,14 @@ pub fn validate_ssa<F: Function>(f: &F, cfginfo: &CFGInfo) -> Result<(), RegAllo
                     return Err(RegAllocError::BB(block));
                 }
                 if f.is_branch(insn) {
-                    let expected = f
+                    for (i, &succ) in f.block_succs(block).iter().enumerate() {
-                        .block_succs(block)
+                        let blockparams_in = f.block_params(succ);
-                        .iter()
+                        let blockparams_out = f.branch_blockparams(block, insn, i);
-                        .map(|&succ| f.block_params(succ).len())
+                        if blockparams_in.len() != blockparams_out.len() {
-                        .sum();
-                    if f.inst_operands(insn).len() != expected {
                             return Err(RegAllocError::Branch(insn));
                         }
                     }
+                }
             } else {
                 if f.is_branch(insn) || f.is_ret(insn) {
                     return Err(RegAllocError::BB(block));