From d6fb1bfa31f0acc868af7d80a132eae268101770 Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Tue, 2 Apr 2024 23:41:24 -0700 Subject: [PATCH] cranelift: Simplify spill/reload emission The `gen_spill` and `gen_reload` methods on `Callee` are used to emit appropriate moves between registers and the stack, as directed by the register allocator. These moves always apply to a single register at a time, even if that register was originally part of a group of registers. For example, when an I128 is represented using two 64-bit registers, either of those registers may be spilled independently. As a result, the `load_spillslot`/`store_spillslot` helpers were more general than necessary, which in turn required extra complexity in the `gen_load_stack_multi`/`gen_store_stack_multi` helpers. None of these helpers were used in any other context, so all that complexity was unnecessary. Inlining all four helpers and then simplifying eliminates a lot of code without changing the output of the compiler. These helpers were also the only uses of `StackAMode::offset`, so I've deleted that. While I was there, I also deleted `StackAMode::get_type`, which was introduced in #8151 and became unused again in #8246. --- cranelift/codegen/src/machinst/abi.rs | 105 ++++---------------------- 1 file changed, 15 insertions(+), 90 deletions(-) diff --git a/cranelift/codegen/src/machinst/abi.rs b/cranelift/codegen/src/machinst/abi.rs index 0f4c45ce8065..d86f1cf31b86 100644 --- a/cranelift/codegen/src/machinst/abi.rs +++ b/cranelift/codegen/src/machinst/abi.rs @@ -292,25 +292,6 @@ pub enum StackAMode { SPOffset(i64, ir::Type), } -impl StackAMode { - /// Offset by an addend. - pub fn offset(self, addend: i64) -> Self { - match self { - StackAMode::ArgOffset(off, ty) => StackAMode::ArgOffset(off + addend, ty), - StackAMode::NominalSPOffset(off, ty) => StackAMode::NominalSPOffset(off + addend, ty), - StackAMode::SPOffset(off, ty) => StackAMode::SPOffset(off + addend, ty), - } - } - - pub fn get_type(&self) -> ir::Type { - match self { - &StackAMode::ArgOffset(_, ty) => ty, - &StackAMode::NominalSPOffset(_, ty) => ty, - &StackAMode::SPOffset(_, ty) => ty, - } - } -} - /// Trait implemented by machine-specific backend to represent ISA flags. pub trait IsaFlags: Clone { /// Get a flag indicating whether forward-edge CFI is enabled. @@ -1346,38 +1327,6 @@ fn generate_gv( } } -fn gen_load_stack_multi( - from: StackAMode, - dst: ValueRegs>, - ty: Type, -) -> SmallInstVec { - let mut ret = smallvec![]; - let (_, tys) = M::I::rc_for_type(ty).unwrap(); - let mut offset = 0; - // N.B.: registers are given in the `ValueRegs` in target endian order. - for (&dst, &ty) in dst.regs().iter().zip(tys.iter()) { - ret.push(M::gen_load_stack(from.offset(offset), dst, ty)); - offset += ty.bytes() as i64; - } - ret -} - -fn gen_store_stack_multi( - from: StackAMode, - src: ValueRegs, - ty: Type, -) -> SmallInstVec { - let mut ret = smallvec![]; - let (_, tys) = M::I::rc_for_type(ty).unwrap(); - let mut offset = 0; - // N.B.: registers are given in the `ValueRegs` in target endian order. - for (&src, &ty) in src.regs().iter().zip(tys.iter()) { - ret.push(M::gen_store_stack(from.offset(offset), src, ty)); - offset += ty.bytes() as i64; - } - ret -} - /// If the signature needs to be legalized, then return the struct-return /// parameter that should be prepended to its returns. Otherwise, return `None`. fn missing_struct_return(sig: &ir::Signature) -> Option { @@ -1758,32 +1707,6 @@ impl Callee { ) } - /// Load from a spillslot. - pub fn load_spillslot( - &self, - slot: SpillSlot, - ty: Type, - into_regs: ValueRegs>, - ) -> SmallInstVec { - let sp_off = self.get_spillslot_offset(slot); - trace!("load_spillslot: slot {:?} -> sp_off {}", slot, sp_off); - - gen_load_stack_multi::(StackAMode::NominalSPOffset(sp_off, ty), into_regs, ty) - } - - /// Store to a spillslot. - pub fn store_spillslot( - &self, - slot: SpillSlot, - ty: Type, - from_regs: ValueRegs, - ) -> SmallInstVec { - let sp_off = self.get_spillslot_offset(slot); - trace!("store_spillslot: slot {:?} -> sp_off {}", slot, sp_off); - - gen_store_stack_multi::(StackAMode::NominalSPOffset(sp_off, ty), from_regs, ty) - } - /// Get an `args` pseudo-inst, if any, that should appear at the /// very top of the function body prior to regalloc. pub fn take_args(&mut self) -> Option { @@ -2032,24 +1955,26 @@ impl Callee { /// Generate a spill. pub fn gen_spill(&self, to_slot: SpillSlot, from_reg: RealReg) -> M::I { - let ty = M::I::canonical_type_for_rc(Reg::from(from_reg).class()); - self.store_spillslot(to_slot, ty, ValueRegs::one(Reg::from(from_reg))) - .into_iter() - .next() - .unwrap() + let ty = M::I::canonical_type_for_rc(from_reg.class()); + debug_assert_eq!(::I::rc_for_type(ty).unwrap().1, &[ty]); + + let sp_off = self.get_spillslot_offset(to_slot); + trace!("gen_spill: {from_reg:?} into slot {to_slot:?} at offset {sp_off}"); + + let from = StackAMode::NominalSPOffset(sp_off, ty); + ::gen_store_stack(from, Reg::from(from_reg), ty) } /// Generate a reload (fill). pub fn gen_reload(&self, to_reg: Writable, from_slot: SpillSlot) -> M::I { let ty = M::I::canonical_type_for_rc(to_reg.to_reg().class()); - self.load_spillslot( - from_slot, - ty, - writable_value_regs(ValueRegs::one(Reg::from(to_reg.to_reg()))), - ) - .into_iter() - .next() - .unwrap() + debug_assert_eq!(::I::rc_for_type(ty).unwrap().1, &[ty]); + + let sp_off = self.get_spillslot_offset(from_slot); + trace!("gen_reload: {to_reg:?} from slot {from_slot:?} at offset {sp_off}"); + + let from = StackAMode::NominalSPOffset(sp_off, ty); + ::gen_load_stack(from, to_reg.map(Reg::from), ty) } }