diff --git a/src/checker.rs b/src/checker.rs index 33035615..a1c97459 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -272,10 +272,8 @@ fn visit_all_vregs(f: &F, mut v: V) { v(op.vreg()); } if f.is_branch(inst) { - for succ_idx in 0..f.block_succs(block).len() { - for ¶m in f.branch_blockparams(block, inst, succ_idx) { - v(param); - } + for ¶m in f.branch_blockparams(block) { + v(param); } } } @@ -824,16 +822,22 @@ impl<'a, F: Function> Checker<'a, F> { let mut last_inst = None; for block in 0..self.f.num_blocks() { let block = Block::new(block); + let mut terminated = false; for inst_or_edit in out.block_insts_and_edits(self.f, block) { + assert!(!terminated); match inst_or_edit { InstOrEdit::Inst(inst) => { debug_assert!(last_inst.is_none() || inst > last_inst.unwrap()); last_inst = Some(inst); self.handle_inst(block, inst, &mut safepoint_slots, out); + if self.f.is_branch(inst) || self.f.is_ret(inst) { + terminated = true; + } } InstOrEdit::Edit(edit) => self.handle_edit(block, edit), } } + assert!(terminated); } } @@ -853,44 +857,39 @@ impl<'a, F: Function> Checker<'a, F> { self.bb_insts.get_mut(&block).unwrap().push(checkinst); } - // Skip normal checks if this is a branch: the blockparams do - // not exist in post-regalloc code, and the edge-moves have to - // be inserted before the branch rather than after. - if !self.f.is_branch(inst) { - let operands: Vec<_> = self.f.inst_operands(inst).iter().cloned().collect(); - let allocs: Vec<_> = out.inst_allocs(inst).iter().cloned().collect(); - let clobbers: Vec<_> = self.f.inst_clobbers(inst).into_iter().collect(); - let checkinst = CheckerInst::Op { - inst, - operands, - allocs, - clobbers, - }; - trace!("checker: adding inst {:?}", checkinst); - self.bb_insts.get_mut(&block).unwrap().push(checkinst); - } - // Instead, if this is a branch, emit a ParallelMove on each + let operands: Vec<_> = self.f.inst_operands(inst).iter().cloned().collect(); + let allocs: Vec<_> = out.inst_allocs(inst).iter().cloned().collect(); + let clobbers: Vec<_> = self.f.inst_clobbers(inst).into_iter().collect(); + let checkinst = CheckerInst::Op { + inst, + operands, + allocs, + clobbers, + }; + trace!("checker: adding inst {:?}", checkinst); + self.bb_insts.get_mut(&block).unwrap().push(checkinst); + + // If this is a branch, emit a ParallelMove on the // outgoing edge as necessary to handle blockparams. - else { - for (i, &succ) in self.f.block_succs(block).iter().enumerate() { - let args = self.f.branch_blockparams(block, inst, i); - let params = self.f.block_params(succ); - assert_eq!( - args.len(), - params.len(), - "block{} has succ block{}; gave {} args for {} params", - block.index(), - succ.index(), - args.len(), - params.len() - ); - if args.len() > 0 { - let moves = params.iter().cloned().zip(args.iter().cloned()).collect(); - self.edge_insts - .get_mut(&(block, succ)) - .unwrap() - .push(CheckerInst::ParallelMove { moves }); - } + if self.f.is_branch(inst) { + let succ = *self.f.block_succs(block).first().unwrap(); + let args = self.f.branch_blockparams(block); + let params = self.f.block_params(succ); + assert_eq!( + args.len(), + params.len(), + "block{} has succ block{}; gave {} args for {} params", + block.index(), + succ.index(), + args.len(), + params.len() + ); + if args.len() > 0 { + let moves = params.iter().cloned().zip(args.iter().cloned()).collect(); + self.edge_insts + .get_mut(&(block, succ)) + .unwrap() + .push(CheckerInst::ParallelMove { moves }); } } } diff --git a/src/fuzzing/func.rs b/src/fuzzing/func.rs index c6063e06..814b4745 100644 --- a/src/fuzzing/func.rs +++ b/src/fuzzing/func.rs @@ -55,7 +55,7 @@ pub struct Func { block_preds: Vec>, block_succs: Vec>, block_params_in: Vec>, - block_params_out: Vec>>, + block_params_out: Vec>, num_vregs: usize, reftype_vregs: Vec, debug_value_labels: Vec<(VReg, Inst, Inst, u32)>, @@ -99,8 +99,8 @@ impl Function for Func { self.insts[insn.index()].op == InstOpcode::Branch } - fn branch_blockparams(&self, block: Block, _: Inst, succ: usize) -> &[VReg] { - &self.block_params_out[block.index()][succ][..] + fn branch_blockparams(&self, block: Block) -> &[VReg] { + &self.block_params_out[block.index()][..] } fn requires_refs_on_stack(&self, insn: Inst) -> bool { @@ -194,7 +194,7 @@ impl FuncBuilder { self.f.block_params_in[block.index()] = params.iter().cloned().collect(); } - pub fn set_block_params_out(&mut self, block: Block, params: Vec>) { + pub fn set_block_params_out(&mut self, block: Block, params: Vec) { self.f.block_params_out[block.index()] = params; } @@ -383,7 +383,11 @@ impl Func { let mut vregs_to_be_defined = vec![]; let mut max_block_params = u.int_in_range(0..=core::cmp::min(3, vregs.len() / 3))?; for &vreg in &vregs { - if block > 0 && bool::arbitrary(u)? && max_block_params > 0 { + if block > 0 + && bool::arbitrary(u)? + && max_block_params > 0 + && builder.f.block_preds[block].len() > 1 + { block_params[block].push(vreg); max_block_params -= 1; } else { @@ -538,9 +542,9 @@ impl Func { // Define the branch with blockparam args that must end // the block. if builder.f.block_succs[block].len() > 0 { - let mut params = vec![]; - for &succ in &builder.f.block_succs[block] { - let mut args = vec![]; + // Only add blockparams if there is a single successor. + if let [succ] = builder.f.block_succs[block][..] { + let mut params = vec![]; for i in 0..builder.f.block_params_in[succ.index()].len() { let dom_block = choose_dominating_block( &builder.idom[..], @@ -565,12 +569,33 @@ impl Func { .copied() .collect(); let vreg = u.choose(&suitable_vregs)?; - args.push(*vreg); + params.push(*vreg); + } + builder.set_block_params_out(Block::new(block), params); + + // If the unique successor only has a single predecessor, + // we can have both blockparams and operands. Turn the last + // instruction into a branch. + let succ_preds = builder.f.block_preds[succ.index()].len() + + if succ == Block(0) { 1 } else { 0 }; + if succ_preds == 1 { + if let Some(last) = builder.insts_per_block[block].last_mut() { + last.op = InstOpcode::Branch; + } else { + builder.add_inst(Block::new(block), InstData::branch()); + } + } else { + builder.add_inst(Block::new(block), InstData::branch()); + } + } else { + // If a branch doesn't have blockparams then it is allowed + // have operands. Turn the last instruction into a branch. + if let Some(last) = builder.insts_per_block[block].last_mut() { + last.op = InstOpcode::Branch; + } else { + builder.add_inst(Block::new(block), InstData::branch()); } - params.push(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()); } @@ -604,16 +629,7 @@ impl core::fmt::Debug for Func { .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) - }) + .map(|v| format!("v{}", v.vreg())) .collect::>() .join(", "); write!( diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs index 09644a24..290b6631 100644 --- a/src/ion/liveranges.rs +++ b/src/ion/liveranges.rs @@ -307,11 +307,9 @@ impl<'a, F: Function> Env<'a, F> { // 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 ¶m in self.func.branch_blockparams(block, insns.last(), i) { - live.set(param.vreg(), true); - self.observe_vreg_class(param); - } + for ¶m in self.func.branch_blockparams(block) { + live.set(param.vreg(), true); + self.observe_vreg_class(param); } } @@ -399,24 +397,22 @@ 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()) { - 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(BlockparamOut { - to_vreg: blockparam_in, - to_block: succ, - from_block: block, - from_vreg: blockparam_out, - }); - - // Include outgoing blockparams in the initial live set. - live.set(blockparam_out.index(), true); - } + let succ = *self.func.block_succs(block).first().unwrap(); + let blockparams_in = self.func.block_params(succ); + let blockparams_out = self.func.branch_blockparams(block); + 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(BlockparamOut { + to_vreg: blockparam_in, + to_block: succ, + from_block: block, + from_vreg: blockparam_out, + }); + + // Include outgoing blockparams in the initial live set. + live.set(blockparam_out.index(), true); } } diff --git a/src/lib.rs b/src/lib.rs index 3ce07863..031ddc49 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -240,6 +240,11 @@ impl PRegSet { self.bits[0] |= other.bits[0]; self.bits[1] |= other.bits[1]; } + + /// Checks whether the `PRegSet` is empty. + fn is_empty(self) -> bool { + self == Self::empty() + } } impl IntoIterator for PRegSet { @@ -1042,11 +1047,11 @@ pub trait Function { /// branch. fn is_branch(&self, insn: Inst) -> bool; - /// If `insn` is a branch at the end of `block`, returns the - /// 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]; + /// If `block` ends with a branch instruction, returns the + /// outgoing blockparam arguments. Branch arguments are only + /// allowed on blocks with a single successor. The number of + /// arguments must match the number incoming blockparams. + fn branch_blockparams(&self, block: Block) -> &[VReg]; /// Determine whether an instruction requires all reference-typed /// values to be placed onto the stack. For these instructions, diff --git a/src/serialize.rs b/src/serialize.rs index a74a60ec..c63a9981 100644 --- a/src/serialize.rs +++ b/src/serialize.rs @@ -45,7 +45,7 @@ pub struct SerializableFunction { block_preds: Vec>, block_succs: Vec>, block_params_in: Vec>, - block_params_out: Vec>>, + block_params_out: Vec>, num_vregs: usize, reftype_vregs: Vec, debug_value_labels: Vec<(VReg, Inst, Inst, u32)>, @@ -106,10 +106,7 @@ impl SerializableFunction { block_params_out: (0..func.num_blocks()) .map(|i| { let block = Block::new(i); - let inst = func.block_insns(block).last(); - (0..func.block_succs(block).len()) - .map(|succ_idx| func.branch_blockparams(block, inst, succ_idx).to_vec()) - .collect() + func.branch_blockparams(block).to_vec() }) .collect(), num_vregs: func.num_vregs(), @@ -169,8 +166,8 @@ impl Function for SerializableFunction { self.insts[insn.index()].op == InstOpcode::Branch } - fn branch_blockparams(&self, block: Block, _: Inst, succ: usize) -> &[VReg] { - &self.block_params_out[block.index()][succ][..] + fn branch_blockparams(&self, block: Block) -> &[VReg] { + &self.block_params_out[block.index()][..] } fn requires_refs_on_stack(&self, insn: Inst) -> bool { @@ -258,16 +255,7 @@ impl fmt::Debug for SerializableFunction { .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) - }) + .map(|v| format!("v{}", v.vreg())) .collect::>() .join(", "); write!( diff --git a/src/ssa.rs b/src/ssa.rs index ac6263ef..cd5a21d7 100644 --- a/src/ssa.rs +++ b/src/ssa.rs @@ -88,8 +88,8 @@ pub fn validate_ssa(f: &F, cfginfo: &CFGInfo) -> Result<(), RegAllo } } - // Check that the length of branch args matches the sum of the - // number of blockparams in their succs, and that the end of every + // Check that the length of branch args matches the number of + // blockparams in their succs, and that the end of every // block ends in this branch or in a ret, and that there are no // other branches or rets in the middle of the block. for block in 0..f.num_blocks() { @@ -102,9 +102,26 @@ pub fn validate_ssa(f: &F, cfginfo: &CFGInfo) -> Result<(), RegAllo return Err(RegAllocError::BB(block)); } if f.is_branch(insn) { - for (i, &succ) in f.block_succs(block).iter().enumerate() { + let blockparams_out = f.branch_blockparams(block); + if !blockparams_out.is_empty() { + // Branches with blockparams can only have 1 successor. + let succ = match f.block_succs(block) { + &[succ] => succ, + _ => { + trace!( + "Block {:?} with outgoing blockparams \ + can only have one successor", + block + ); + return Err(RegAllocError::Branch(insn)); + } + }; + // ... and aren't allowed to have operands. + if !f.inst_operands(insn).is_empty() || !f.inst_clobbers(insn).is_empty() { + trace!("Branch {:?} with blockparams can't have operands", insn); + return Err(RegAllocError::Branch(insn)); + } let blockparams_in = f.block_params(succ); - let blockparams_out = f.branch_blockparams(block, insn, i); if blockparams_in.len() != blockparams_out.len() { trace!( "Mismatch on block params, found {} expected {}",