From 0cb3a8019f213072d564cafe01673e81b5c2f2fa Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 30 Nov 2021 02:22:15 +0000 Subject: [PATCH 1/2] Rework the API for outgoing blockparams --- src/cfg.rs | 2 +- src/fuzzing/func.rs | 68 ++++++++++++++++++++++++++++--------------- src/ion/liveranges.rs | 17 ++++++----- src/lib.rs | 17 ++++------- src/ssa.rs | 13 ++++----- 5 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/cfg.rs b/src/cfg.rs index f2abc47d..7769556f 100644 --- a/src/cfg.rs +++ b/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)); } } diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index 38e7400f..e5174cb9 100644 --- a/src/fuzzing/func.rs +++ b/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 { - let mut operands = vec![]; - for &u in uses { - operands.push(Operand::reg_use(VReg::new(u, RegClass::Int))); - } + pub fn branch() -> InstData { InstData { op: InstOpcode::Branch, - operands, + operands: vec![], clobbers: vec![], is_safepoint: false, } @@ -68,7 +63,8 @@ pub struct Func { blocks: Vec, block_preds: Vec>, block_succs: Vec>, - block_params: Vec>, + block_params_in: Vec>, + block_params_out: Vec>>, num_vregs: usize, reftype_vregs: Vec, } @@ -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 { - // Branch blockparam args always start at zero for this - // Function implementation. - 0 + fn branch_blockparams(&self, block: Block, _: Inst, succ: usize) -> &[VReg] { + &self.block_params_out[block.index()][succ][..] } 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]) { - self.f.block_params[block.index()] = params.iter().cloned().collect(); + pub fn set_block_params_in(&mut self, block: Block, params: &[VReg]) { + self.f.block_params_in[block.index()] = params.iter().cloned().collect(); + } + + pub fn set_block_params_out(&mut self, block: Block, params: Vec>) { + 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::>(); - let params = self.block_params[i] + let params_in = self.block_params_in[i] .iter() .map(|v| format!("v{}", v.vreg())) .collect::>() .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::>() + .join(", "); + format!("block{}({})", succ.index(), params) + }) + .collect::>() + .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")?; diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 9c399eb5..1140c1ee 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -431,15 +431,16 @@ impl<'a, F: Function> Env<'a, F> { // 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()); + 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((from_vreg, block, succ, blockparam_vreg)); - i += 1; + .push((blockparam_out, block, succ, blockparam_in)); } } } diff --git a/src/lib.rs b/src/lib.rs index d91bf970..6cc43549 100644 --- a/src/lib.rs +++ b/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_blockparam_arg_offset()` below *must* be the block - /// parameters for each of its block's `block_succs` successor - /// blocks, in order. + /// branch. 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 - /// found. Starting at this index, blockparam arguments for each - /// successor block's blockparams, in order, must be found. - /// - /// 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; + /// outgoing blockparam arguments for the given successor. The + /// number of arguments must match the number incoming blockparams + /// for each respective successor block. + fn branch_blockparams(&self, block: Block, insn: Inst, succ_idx: usize) -> &[VReg]; /// Determine whether an instruction requires all reference-typed /// values to be placed onto the stack. For these instructions, diff --git a/src/ssa.rs b/src/ssa.rs index d8df647d..44ef6817 100644 --- a/src/ssa.rs +++ b/src/ssa.rs @@ -71,13 +71,12 @@ pub fn validate_ssa(f: &F, cfginfo: &CFGInfo) -> Result<(), RegAllo return Err(RegAllocError::BB(block)); } if f.is_branch(insn) { - let expected = f - .block_succs(block) - .iter() - .map(|&succ| f.block_params(succ).len()) - .sum(); - if f.inst_operands(insn).len() != expected { - return Err(RegAllocError::Branch(insn)); + for (i, &succ) in f.block_succs(block).iter().enumerate() { + let blockparams_in = f.block_params(succ); + let blockparams_out = f.branch_blockparams(block, insn, i); + if blockparams_in.len() != blockparams_out.len() { + return Err(RegAllocError::Branch(insn)); + } } } } else { From 6621a57cb7850bd49ff28d2f2387c9fc0df522bf Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 1 Dec 2021 01:43:02 +0000 Subject: [PATCH 2/2] Fix liveranges for branch parameters --- src/ion/liveranges.rs | 59 +++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 1140c1ee..4637318b 100644 --- a/src/ion/liveranges.rs +++ b/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,25 +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()) { - 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)); - } - } - } - // For each instruction, in reverse order, process // operands and clobbers. for inst in insns.rev().iter() { @@ -893,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