From ed861ba5f538894d49c6ad59759cabb93f063cfb Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 11 Aug 2022 16:48:12 -0700 Subject: [PATCH 1/2] Cranelift: Remove the `ABICaller` trait It has only one implementation: the `ABICallerImpl` struct. We can just use that directly rather than having extra, unnecessary layers of generics and abstractions. --- cranelift/codegen/src/isa/x64/lower/isle.rs | 4 +- cranelift/codegen/src/machinst/abi.rs | 76 --------------------- cranelift/codegen/src/machinst/abi_impl.rs | 54 ++++++++++----- 3 files changed, 38 insertions(+), 96 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs index dcf1c0ea9311..78de5e2b2992 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle.rs +++ b/cranelift/codegen/src/isa/x64/lower/isle.rs @@ -30,8 +30,8 @@ use crate::{ }, }, machinst::{ - isle::*, valueregs, ABICaller, InsnInput, InsnOutput, Lower, MachAtomicRmwOp, MachInst, - VCodeConstant, VCodeConstantData, + isle::*, valueregs, InsnInput, InsnOutput, Lower, MachAtomicRmwOp, MachInst, VCodeConstant, + VCodeConstantData, }, }; use regalloc2::PReg; diff --git a/cranelift/codegen/src/machinst/abi.rs b/cranelift/codegen/src/machinst/abi.rs index 676298812899..4c8a6f77baa8 100644 --- a/cranelift/codegen/src/machinst/abi.rs +++ b/cranelift/codegen/src/machinst/abi.rs @@ -1,83 +1,7 @@ //! ABI definitions. -use crate::machinst::*; use smallvec::SmallVec; /// A small vector of instructions (with some reasonable size); appropriate for /// a small fixed sequence implementing one operation. pub type SmallInstVec = SmallVec<[I; 4]>; - -/// Trait implemented by an object that tracks ABI-related state and can -/// generate code while emitting a *call* to a function. -/// -/// An instance of this trait returns information for a *particular* -/// callsite. It will usually be computed from the called function's -/// signature. -/// -/// Unlike `Callee`, methods on this trait are not invoked directly by the -/// machine-independent code. Rather, the machine-specific lowering code will -/// typically create an `ABICaller` when creating machine instructions for an IR -/// call instruction inside `lower()`, directly emit the arg and and retval -/// copies, and attach the register use/def info to the call. -/// -/// This trait is thus provided for convenience to the backends. -pub trait ABICaller { - /// The instruction type for the ISA associated with this ABI. - type I: VCodeInst; - - /// Get the number of arguments expected. - fn num_args(&self) -> usize; - - /// Emit a copy of an argument value from a source register, prior to the call. - /// For large arguments with associated stack buffer, this may load the address - /// of the buffer into the argument register, if required by the ABI. - fn emit_copy_regs_to_arg(&self, ctx: &mut Lower, idx: usize, from_reg: ValueRegs); - - /// Emit a copy of a large argument into its associated stack buffer, if any. - /// We must be careful to perform all these copies (as necessary) before setting - /// up the argument registers, since we may have to invoke memcpy(), which could - /// clobber any registers already set up. The back-end should call this routine - /// for all arguments before calling emit_copy_regs_to_arg for all arguments. - fn emit_copy_regs_to_buffer( - &self, - ctx: &mut Lower, - idx: usize, - from_reg: ValueRegs, - ); - - /// Emit a copy a return value into a destination register, after the call returns. - fn emit_copy_retval_to_regs( - &self, - ctx: &mut Lower, - idx: usize, - into_reg: ValueRegs>, - ); - - /// Emit code to pre-adjust the stack, prior to argument copies and call. - fn emit_stack_pre_adjust(&self, ctx: &mut Lower); - - /// Emit code to post-adjust the satck, after call return and return-value copies. - fn emit_stack_post_adjust(&self, ctx: &mut Lower); - - /// Accumulate outgoing arguments. This ensures that the caller (as - /// identified via the CTX argument) allocates enough space in the - /// prologue to hold all arguments and return values for this call. - /// There is no code emitted at the call site, everything is done - /// in the caller's function prologue. - fn accumulate_outgoing_args_size(&self, ctx: &mut Lower); - - /// Emit the call itself. - /// - /// The returned instruction should have proper use- and def-sets according - /// to the argument registers, return-value registers, and clobbered - /// registers for this function signature in this ABI. - /// - /// (Arg registers are uses, and retval registers are defs. Clobbered - /// registers are also logically defs, but should never be read; their - /// values are "defined" (to the regalloc) but "undefined" in every other - /// sense.) - /// - /// This function should only be called once, as it is allowed to re-use - /// parts of the ABICaller object in emitting instructions. - fn emit_call(&mut self, ctx: &mut Lower); -} diff --git a/cranelift/codegen/src/machinst/abi_impl.rs b/cranelift/codegen/src/machinst/abi_impl.rs index a650c0176d6c..efd1d13f0d34 100644 --- a/cranelift/codegen/src/machinst/abi_impl.rs +++ b/cranelift/codegen/src/machinst/abi_impl.rs @@ -1730,10 +1730,9 @@ fn adjust_stack_and_nominal_sp(ctx: &mut Lower, off: i3 ctx.emit(M::gen_nominal_sp_adj(-amt)); } -impl ABICaller for ABICallerImpl { - type I = M::I; - - fn num_args(&self) -> usize { +impl ABICallerImpl { + /// Get the number of arguments expected. + pub fn num_args(&self) -> usize { if self.sig.stack_ret_arg.is_some() { self.sig.args.len() - 1 } else { @@ -1741,24 +1740,26 @@ impl ABICaller for ABICallerImpl { } } - fn accumulate_outgoing_args_size(&self, ctx: &mut Lower) { - let off = self.sig.sized_stack_arg_space + self.sig.sized_stack_ret_space; - ctx.abi().accumulate_outgoing_args_size(off as u32); - } - - fn emit_stack_pre_adjust(&self, ctx: &mut Lower) { + /// Emit code to pre-adjust the stack, prior to argument copies and call. + pub fn emit_stack_pre_adjust(&self, ctx: &mut Lower) { let off = self.sig.sized_stack_arg_space + self.sig.sized_stack_ret_space; adjust_stack_and_nominal_sp::(ctx, off as i32, /* is_sub = */ true) } - fn emit_stack_post_adjust(&self, ctx: &mut Lower) { + /// Emit code to post-adjust the satck, after call return and return-value copies. + pub fn emit_stack_post_adjust(&self, ctx: &mut Lower) { let off = self.sig.sized_stack_arg_space + self.sig.sized_stack_ret_space; adjust_stack_and_nominal_sp::(ctx, off as i32, /* is_sub = */ false) } - fn emit_copy_regs_to_buffer( + /// Emit a copy of a large argument into its associated stack buffer, if any. + /// We must be careful to perform all these copies (as necessary) before setting + /// up the argument registers, since we may have to invoke memcpy(), which could + /// clobber any registers already set up. The back-end should call this routine + /// for all arguments before calling emit_copy_regs_to_arg for all arguments. + pub fn emit_copy_regs_to_buffer( &self, - ctx: &mut Lower, + ctx: &mut Lower, idx: usize, from_regs: ValueRegs, ) { @@ -1788,9 +1789,12 @@ impl ABICaller for ABICallerImpl { } } - fn emit_copy_regs_to_arg( + /// Emit a copy of an argument value from a source register, prior to the call. + /// For large arguments with associated stack buffer, this may load the address + /// of the buffer into the argument register, if required by the ABI. + pub fn emit_copy_regs_to_arg( &self, - ctx: &mut Lower, + ctx: &mut Lower, idx: usize, from_regs: ValueRegs, ) { @@ -1871,9 +1875,10 @@ impl ABICaller for ABICallerImpl { } } - fn emit_copy_retval_to_regs( + /// Emit a copy a return value into a destination register, after the call returns. + pub fn emit_copy_retval_to_regs( &self, - ctx: &mut Lower, + ctx: &mut Lower, idx: usize, into_regs: ValueRegs>, ) { @@ -1907,7 +1912,20 @@ impl ABICaller for ABICallerImpl { } } - fn emit_call(&mut self, ctx: &mut Lower) { + /// Emit the call itself. + /// + /// The returned instruction should have proper use- and def-sets according + /// to the argument registers, return-value registers, and clobbered + /// registers for this function signature in this ABI. + /// + /// (Arg registers are uses, and retval registers are defs. Clobbered + /// registers are also logically defs, but should never be read; their + /// values are "defined" (to the regalloc) but "undefined" in every other + /// sense.) + /// + /// This function should only be called once, as it is allowed to re-use + /// parts of the `ABICallerImpl` object in emitting instructions. + pub fn emit_call(&mut self, ctx: &mut Lower) { let (uses, defs) = ( mem::replace(&mut self.uses, Default::default()), mem::replace(&mut self.defs, Default::default()), From 05e9aa2ddbe2ee439e3f1f4f540fefc135894a48 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 11 Aug 2022 16:59:27 -0700 Subject: [PATCH 2/2] Cranelift: Rename `ABICallerImpl` to `Caller` --- cranelift/codegen/src/isa/aarch64/abi.rs | 2 +- cranelift/codegen/src/isa/aarch64/lower_inst.rs | 4 ++-- cranelift/codegen/src/isa/x64/abi.rs | 2 +- cranelift/codegen/src/isa/x64/lower.rs | 2 +- cranelift/codegen/src/isa/x64/lower/isle.rs | 9 ++++----- cranelift/codegen/src/machinst/abi_impl.rs | 16 ++++++++-------- 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/cranelift/codegen/src/isa/aarch64/abi.rs b/cranelift/codegen/src/isa/aarch64/abi.rs index 4004ae50b195..b30c4e5cb55e 100644 --- a/cranelift/codegen/src/isa/aarch64/abi.rs +++ b/cranelift/codegen/src/isa/aarch64/abi.rs @@ -24,7 +24,7 @@ use smallvec::{smallvec, SmallVec}; pub(crate) type AArch64Callee = Callee; /// Support for the AArch64 ABI from the caller side (at a callsite). -pub(crate) type AArch64ABICaller = ABICallerImpl; +pub(crate) type AArch64Caller = Caller; /// This is the limit for the size of argument and return-value areas on the /// stack. We place a reasonable limit here to avoid integer overflow issues diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index 4b841da050e5..69e795c01846 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -605,7 +605,7 @@ pub(crate) fn lower_insn_to_regs( assert!(inputs.len() == sig.params.len()); assert!(outputs.len() == sig.returns.len()); ( - AArch64ABICaller::from_func(sig, &extname, dist, caller_conv, flags)?, + AArch64Caller::from_func(sig, &extname, dist, caller_conv, flags)?, &inputs[..], ) } @@ -615,7 +615,7 @@ pub(crate) fn lower_insn_to_regs( assert!(inputs.len() - 1 == sig.params.len()); assert!(outputs.len() == sig.returns.len()); ( - AArch64ABICaller::from_ptr(sig, ptr, op, caller_conv, flags)?, + AArch64Caller::from_ptr(sig, ptr, op, caller_conv, flags)?, &inputs[1..], ) } diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs index 718ee85b2d21..86f14faf261a 100644 --- a/cranelift/codegen/src/isa/x64/abi.rs +++ b/cranelift/codegen/src/isa/x64/abi.rs @@ -24,7 +24,7 @@ static STACK_ARG_RET_SIZE_LIMIT: u64 = 128 * 1024 * 1024; pub(crate) type X64Callee = Callee; /// Support for the x64 ABI from the caller side (at a callsite). -pub(crate) type X64ABICaller = ABICallerImpl; +pub(crate) type X64Caller = Caller; /// Implementation of ABI primitives for x64. pub struct X64ABIMachineSpec; diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index dbfb54ebaa44..4da2fe1d1d60 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -279,7 +279,7 @@ fn emit_vm_call( let sig = libcall.signature(call_conv); let caller_conv = ctx.abi().call_conv(); - let mut abi = X64ABICaller::from_func(&sig, &extname, dist, caller_conv, flags)?; + let mut abi = X64Caller::from_func(&sig, &extname, dist, caller_conv, flags)?; abi.emit_stack_pre_adjust(ctx); diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs index 78de5e2b2992..e7fa6e9437d9 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle.rs +++ b/cranelift/codegen/src/isa/x64/lower/isle.rs @@ -24,7 +24,7 @@ use crate::{ settings::Flags, unwind::UnwindInst, x64::{ - abi::{X64ABICaller, X64ABIMachineSpec}, + abi::{X64ABIMachineSpec, X64Caller}, inst::{args::*, regs, CallInfo}, settings::Flags as IsaFlags, }, @@ -673,7 +673,7 @@ impl Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> { let sig = &self.lower_ctx.dfg().signatures[sig_ref]; let num_rets = sig.returns.len(); let abi = ABISig::from_func_sig::(sig, self.flags).unwrap(); - let caller = X64ABICaller::from_func(sig, &extname, dist, caller_conv, self.flags).unwrap(); + let caller = X64Caller::from_func(sig, &extname, dist, caller_conv, self.flags).unwrap(); assert_eq!( inputs.len(&self.lower_ctx.dfg().value_lists) - off, @@ -695,8 +695,7 @@ impl Context for IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> { let num_rets = sig.returns.len(); let abi = ABISig::from_func_sig::(sig, self.flags).unwrap(); let caller = - X64ABICaller::from_ptr(sig, ptr, Opcode::CallIndirect, caller_conv, self.flags) - .unwrap(); + X64Caller::from_ptr(sig, ptr, Opcode::CallIndirect, caller_conv, self.flags).unwrap(); assert_eq!( inputs.len(&self.lower_ctx.dfg().value_lists) - off, @@ -807,7 +806,7 @@ impl IsleContext<'_, '_, MInst, Flags, IsaFlags, 6> { &mut self, abi: ABISig, num_rets: usize, - mut caller: X64ABICaller, + mut caller: X64Caller, (inputs, off): ValueSlice, ) -> InstOutput { caller.emit_stack_pre_adjust(self.lower_ctx); diff --git a/cranelift/codegen/src/machinst/abi_impl.rs b/cranelift/codegen/src/machinst/abi_impl.rs index efd1d13f0d34..ca51d1078f72 100644 --- a/cranelift/codegen/src/machinst/abi_impl.rs +++ b/cranelift/codegen/src/machinst/abi_impl.rs @@ -1638,7 +1638,7 @@ impl Callee { } /// ABI object for a callsite. -pub struct ABICallerImpl { +pub struct Caller { /// The called function's signature. sig: ABISig, /// All uses for the callsite, i.e., function args. @@ -1668,7 +1668,7 @@ pub enum CallDest { Reg(Reg), } -impl ABICallerImpl { +impl Caller { /// Create a callsite ABI object for a call directly to the specified function. pub fn from_func( sig: &ir::Signature, @@ -1676,11 +1676,11 @@ impl ABICallerImpl { dist: RelocDistance, caller_conv: isa::CallConv, flags: &settings::Flags, - ) -> CodegenResult> { + ) -> CodegenResult> { let ir_sig = ensure_struct_return_ptr_is_returned(sig); let sig = ABISig::from_func_sig::(&ir_sig, flags)?; let (uses, defs, clobbers) = sig.call_uses_defs_clobbers::(); - Ok(ABICallerImpl { + Ok(Caller { sig, uses, defs, @@ -1701,11 +1701,11 @@ impl ABICallerImpl { opcode: ir::Opcode, caller_conv: isa::CallConv, flags: &settings::Flags, - ) -> CodegenResult> { + ) -> CodegenResult> { let ir_sig = ensure_struct_return_ptr_is_returned(sig); let sig = ABISig::from_func_sig::(&ir_sig, flags)?; let (uses, defs, clobbers) = sig.call_uses_defs_clobbers::(); - Ok(ABICallerImpl { + Ok(Caller { sig, uses, defs, @@ -1730,7 +1730,7 @@ fn adjust_stack_and_nominal_sp(ctx: &mut Lower, off: i3 ctx.emit(M::gen_nominal_sp_adj(-amt)); } -impl ABICallerImpl { +impl Caller { /// Get the number of arguments expected. pub fn num_args(&self) -> usize { if self.sig.stack_ret_arg.is_some() { @@ -1924,7 +1924,7 @@ impl ABICallerImpl { /// sense.) /// /// This function should only be called once, as it is allowed to re-use - /// parts of the `ABICallerImpl` object in emitting instructions. + /// parts of the `Caller` object in emitting instructions. pub fn emit_call(&mut self, ctx: &mut Lower) { let (uses, defs) = ( mem::replace(&mut self.uses, Default::default()),