diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index a0e36dc074..2c5d82b4b6 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -98,7 +98,6 @@ pub struct CallIndInfo { pub struct JTSequenceInfo { pub targets: Vec, pub default_target: BranchTarget, - pub targets_for_term: Vec, // needed for MachTerminator. } fn count_zero_half_words(mut value: u64, num_half_words: u8) -> usize { @@ -1091,17 +1090,13 @@ impl MachInst for Inst { } } - fn is_term<'a>(&'a self) -> MachTerminator<'a> { + fn is_term(&self) -> MachTerminator { match self { &Inst::Ret { .. } | &Inst::EpiloguePlaceholder => MachTerminator::Ret, - &Inst::Jump { dest } => MachTerminator::Uncond(dest.as_label().unwrap()), - &Inst::CondBr { - taken, not_taken, .. - } => MachTerminator::Cond(taken.as_label().unwrap(), not_taken.as_label().unwrap()), - &Inst::IndirectBr { ref targets, .. } => MachTerminator::Indirect(&targets[..]), - &Inst::JTSequence { ref info, .. } => { - MachTerminator::Indirect(&info.targets_for_term[..]) - } + &Inst::Jump { .. } => MachTerminator::Uncond, + &Inst::CondBr { .. } => MachTerminator::Cond, + &Inst::IndirectBr { .. } => MachTerminator::Indirect, + &Inst::JTSequence { .. } => MachTerminator::Indirect, _ => MachTerminator::None, } } diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index f12ab7a095..fdb277c381 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -2739,7 +2739,6 @@ pub(crate) fn lower_branch>( .map(|bix| BranchTarget::Label(*bix)) .collect(); let default_target = BranchTarget::Label(targets[0]); - let targets_for_term: Vec = targets.to_vec(); ctx.emit(Inst::JTSequence { ridx, rtmp1, @@ -2747,7 +2746,6 @@ pub(crate) fn lower_branch>( info: Box::new(JTSequenceInfo { targets: jt_targets, default_target, - targets_for_term, }), }); } diff --git a/cranelift/codegen/src/isa/s390x/inst/mod.rs b/cranelift/codegen/src/isa/s390x/inst/mod.rs index c5f7f72f43..25071cd493 100644 --- a/cranelift/codegen/src/isa/s390x/inst/mod.rs +++ b/cranelift/codegen/src/isa/s390x/inst/mod.rs @@ -733,19 +733,17 @@ impl MachInst for Inst { } } - fn is_term<'a>(&'a self) -> MachTerminator<'a> { + fn is_term(&self) -> MachTerminator { match self { &Inst::Ret { .. } | &Inst::EpiloguePlaceholder => MachTerminator::Ret, - &Inst::Jump { dest } => MachTerminator::Uncond(dest), - &Inst::CondBr { - taken, not_taken, .. - } => MachTerminator::Cond(taken, not_taken), + &Inst::Jump { .. } => MachTerminator::Uncond, + &Inst::CondBr { .. } => MachTerminator::Cond, &Inst::OneWayCondBr { .. } => { // Explicitly invisible to CFG processing. MachTerminator::None } - &Inst::IndirectBr { ref targets, .. } => MachTerminator::Indirect(&targets[..]), - &Inst::JTSequence { ref targets, .. } => MachTerminator::Indirect(&targets[..]), + &Inst::IndirectBr { .. } => MachTerminator::Indirect, + &Inst::JTSequence { .. } => MachTerminator::Indirect, _ => MachTerminator::None, } } diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index 1dd8d18ad6..4b25da45be 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/cranelift/codegen/src/isa/x64/inst/mod.rs @@ -2119,18 +2119,13 @@ impl MachInst for Inst { } } - fn is_term<'a>(&'a self) -> MachTerminator<'a> { + fn is_term(&self) -> MachTerminator { match self { // Interesting cases. &Self::Ret { .. } | &Self::EpiloguePlaceholder => MachTerminator::Ret, - &Self::JmpKnown { dst } => MachTerminator::Uncond(dst), - &Self::JmpCond { - taken, not_taken, .. - } => MachTerminator::Cond(taken, not_taken), - &Self::JmpTableSeq { - ref targets_for_term, - .. - } => MachTerminator::Indirect(&targets_for_term[..]), + &Self::JmpKnown { .. } => MachTerminator::Uncond, + &Self::JmpCond { .. } => MachTerminator::Cond, + &Self::JmpTableSeq { .. } => MachTerminator::Indirect, // All other cases are boring. _ => MachTerminator::None, } diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs index 6cd7bfa5e8..44c30872cc 100644 --- a/cranelift/codegen/src/machinst/lower.rs +++ b/cranelift/codegen/src/machinst/lower.rs @@ -745,6 +745,9 @@ impl<'func, I: VCodeInst> Lower<'func, I> { fn lower_clif_branches>( &mut self, backend: &B, + // Lowered block index: + bindex: BlockIndex, + // Original CLIF block: block: Block, branches: &SmallVec<[Inst; 2]>, targets: &SmallVec<[MachLabel; 2]>, @@ -762,12 +765,15 @@ impl<'func, I: VCodeInst> Lower<'func, I> { let loc = self.srcloc(branches[0]); self.finish_ir_inst(loc); // Add block param outputs for current block. - self.lower_branch_blockparam_args(block); + self.lower_branch_blockparam_args(bindex); Ok(()) } - fn lower_branch_blockparam_args(&mut self, block: Block) { - visit_block_succs(self.f, block, |inst, _succ| { + fn lower_branch_blockparam_args(&mut self, block: BlockIndex) { + for succ_idx in 0..self.vcode.block_order().succ_indices(block).len() { + // Avoid immutable borrow by explicitly indexing. + let (inst, succ) = self.vcode.block_order().succ_indices(block)[succ_idx]; + // Get branch args and convert to Regs. let branch_args = self.f.dfg.inst_variable_args(inst); let mut branch_arg_vregs: SmallVec<[Reg; 16]> = smallvec![]; for &arg in branch_args { @@ -778,8 +784,8 @@ impl<'func, I: VCodeInst> Lower<'func, I> { branch_arg_vregs.push(vreg.into()); } } - self.vcode.add_branch_args_for_succ(&branch_arg_vregs[..]); - }); + self.vcode.add_succ(succ, &branch_arg_vregs[..]); + } self.finish_ir_inst(SourceLoc::default()); } @@ -849,7 +855,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { if let Some(bb) = lb.orig_block() { self.collect_branches_and_targets(bindex, bb, &mut branches, &mut targets); if branches.len() > 0 { - self.lower_clif_branches(backend, bb, &branches, &targets)?; + self.lower_clif_branches(backend, bindex, bb, &branches, &targets)?; self.finish_ir_inst(self.srcloc(branches[0])); } } else { @@ -875,7 +881,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> { .add_block_param(vreg, self.vcode.get_vreg_type(vreg)); } } - self.vcode.add_branch_args_for_succ(&branch_arg_vregs[..]); + self.vcode.add_succ(succ, &branch_arg_vregs[..]); self.emit(I::gen_jump(MachLabel::from_block(succ))); self.finish_ir_inst(SourceLoc::default()); diff --git a/cranelift/codegen/src/machinst/mod.rs b/cranelift/codegen/src/machinst/mod.rs index 7d1581d4f8..6d0d145349 100644 --- a/cranelift/codegen/src/machinst/mod.rs +++ b/cranelift/codegen/src/machinst/mod.rs @@ -94,7 +94,7 @@ pub trait MachInst: Clone + Debug { /// Is this a terminator (branch or ret)? If so, return its type /// (ret/uncond/cond) and target if applicable. - fn is_term<'a>(&'a self) -> MachTerminator<'a>; + fn is_term(&self) -> MachTerminator; /// Returns true if the instruction is an epilogue placeholder. fn is_epilogue_placeholder(&self) -> bool; @@ -221,18 +221,22 @@ pub trait MachInstLabelUse: Clone + Copy + Debug + Eq { /// Describes a block terminator (not call) in the vcode, when its branches /// have not yet been finalized (so a branch may have two targets). +/// +/// Actual targets are not included: the single-source-of-truth for +/// those is the VCode itself, which holds, for each block, successors +/// and outgoing branch args per successor. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum MachTerminator<'a> { +pub enum MachTerminator { /// Not a terminator. None, /// A return instruction. Ret, /// An unconditional branch to another block. - Uncond(MachLabel), + Uncond, /// A conditional branch to one of two other blocks. - Cond(MachLabel, MachLabel), + Cond, /// An indirect branch with known possible targets. - Indirect(&'a [MachLabel]), + Indirect, } /// A trait describing the ability to encode a MachInst into binary machine code. diff --git a/cranelift/codegen/src/machinst/vcode.rs b/cranelift/codegen/src/machinst/vcode.rs index fde128088c..c5afee7044 100644 --- a/cranelift/codegen/src/machinst/vcode.rs +++ b/cranelift/codegen/src/machinst/vcode.rs @@ -371,7 +371,7 @@ impl VCodeBuilder { self.vcode.block_params.push(param.into()); } - pub fn add_branch_args_for_succ(&mut self, args: &[Reg]) { + fn add_branch_args_for_succ(&mut self, args: &[Reg]) { let start = self.vcode.branch_block_args.len(); self.vcode .branch_block_args @@ -385,34 +385,16 @@ impl VCodeBuilder { /// Push an instruction for the current BB and current IR inst /// within the BB. pub fn push(&mut self, insn: I) { - match insn.is_term() { - MachTerminator::None | MachTerminator::Ret => {} - MachTerminator::Uncond(target) => { - self.vcode - .block_succs_preds - .push(BlockIndex::new(target.get() as usize)); - } - MachTerminator::Cond(true_branch, false_branch) => { - self.vcode - .block_succs_preds - .push(BlockIndex::new(true_branch.get() as usize)); - self.vcode - .block_succs_preds - .push(BlockIndex::new(false_branch.get() as usize)); - } - MachTerminator::Indirect(targets) => { - for target in targets { - self.vcode - .block_succs_preds - .push(BlockIndex::new(target.get() as usize)); - } - } - } - self.vcode.insts.push(insn); self.vcode.srclocs.push(self.cur_srcloc); } + /// Add a successor block with branch args. + pub fn add_succ(&mut self, block: BlockIndex, args: &[Reg]) { + self.vcode.block_succs_preds.push(block); + self.add_branch_args_for_succ(args); + } + /// Set the current source location. pub fn set_srcloc(&mut self, srcloc: SourceLoc) { self.cur_srcloc = srcloc; @@ -1191,9 +1173,7 @@ impl RegallocFunction for VCode { fn is_branch(&self, insn: InsnIndex) -> bool { match self.insts[insn.index()].is_term() { - MachTerminator::Cond(..) - | MachTerminator::Uncond(..) - | MachTerminator::Indirect(..) => true, + MachTerminator::Cond | MachTerminator::Uncond | MachTerminator::Indirect => true, _ => false, } }