From 75a4d27932c1e84b153bf45b684d6d1247808366 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 3 Dec 2024 11:32:25 -0700 Subject: [PATCH] pulley: Implement `get_frame_pointer` (#9658) * pulley: Implement `get_frame_pointer` This commit implements the CLIF `get_frame_pointer` instruction which will be needed by trampolines in Wasmtime. This is implemented by generalizing the preexisting `GetSp` `MInst` into a "get special". This the additionally removes the extended `get_sp` opcode in Pulley as it's not necessary as `xmov` can operate on this register. * Review comments --- .../codegen/src/isa/pulley_shared/inst.isle | 23 ++++-- .../src/isa/pulley_shared/inst/args.rs | 14 ++++ .../src/isa/pulley_shared/inst/emit.rs | 2 +- .../codegen/src/isa/pulley_shared/inst/mod.rs | 11 ++- .../codegen/src/isa/pulley_shared/lower.isle | 7 +- .../src/isa/pulley_shared/lower/isle.rs | 9 +++ .../isa/pulley32/get_stack_pointer.clif | 18 ----- .../filetests/isa/pulley32/special_regs.clif | 70 +++++++++++++++++++ .../isa/pulley64/get_stack_pointer.clif | 18 ----- .../filetests/isa/pulley64/special_regs.clif | 70 +++++++++++++++++++ pulley/fuzz/src/interp.rs | 1 - pulley/src/interp.rs | 6 -- pulley/src/lib.rs | 2 - 13 files changed, 194 insertions(+), 57 deletions(-) delete mode 100644 cranelift/filetests/filetests/isa/pulley32/get_stack_pointer.clif create mode 100644 cranelift/filetests/filetests/isa/pulley32/special_regs.clif delete mode 100644 cranelift/filetests/filetests/isa/pulley64/get_stack_pointer.clif create mode 100644 cranelift/filetests/filetests/isa/pulley64/special_regs.clif diff --git a/cranelift/codegen/src/isa/pulley_shared/inst.isle b/cranelift/codegen/src/isa/pulley_shared/inst.isle index 2ad356f622fd..6ad0c4998931 100644 --- a/cranelift/codegen/src/isa/pulley_shared/inst.isle +++ b/cranelift/codegen/src/isa/pulley_shared/inst.isle @@ -39,8 +39,11 @@ ;; Nothing. (Nop) - ;; Get the stack pointer. - (GetSp (dst WritableXReg)) + ;; Move a special register (e.g. sp, fp, lr, etc) in to a general-purpose + ;; register. + (GetSpecial + (dst WritableXReg) + (reg XReg)) ;; Return. (Ret) @@ -378,11 +381,17 @@ (rule (pulley_trap_if cond size src1 src2 code) (SideEffectNoResult.Inst (MInst.TrapIf cond size src1 src2 code))) -(decl pulley_get_sp () XReg) -(rule (pulley_get_sp) - (let ((reg WritableXReg (temp_writable_xreg)) - (_ Unit (emit (MInst.GetSp reg)))) - reg)) +(decl sp_reg () XReg) +(extern constructor sp_reg sp_reg) + +(decl fp_reg () XReg) +(extern constructor fp_reg fp_reg) + +(decl pulley_get_special (XReg) XReg) +(rule (pulley_get_special reg) + (let ((dst WritableXReg (temp_writable_xreg)) + (_ Unit (emit (MInst.GetSpecial dst reg)))) + dst)) (decl pulley_xconst8 (i8) XReg) (rule (pulley_xconst8 x) diff --git a/cranelift/codegen/src/isa/pulley_shared/inst/args.rs b/cranelift/codegen/src/isa/pulley_shared/inst/args.rs index 468f31a13643..3affe26e9348 100644 --- a/cranelift/codegen/src/isa/pulley_shared/inst/args.rs +++ b/cranelift/codegen/src/isa/pulley_shared/inst/args.rs @@ -126,6 +126,20 @@ impl XReg { /// Index of the first "special" register, or the end of which registers /// regalloc is allowed to use. pub const SPECIAL_START: u8 = pulley_interpreter::regs::XReg::SPECIAL_START; + + /// Returns whether this is a "special" physical register for pulley. + pub fn is_special(&self) -> bool { + match self.as_pulley() { + Some(reg) => reg.is_special(), + None => false, + } + } + + /// Returns the pulley-typed register, if this is a phyiscal register. + pub fn as_pulley(&self) -> Option { + let enc = self.to_real_reg()?.hw_enc(); + Some(pulley_interpreter::XReg::new(enc).unwrap()) + } } pub use super::super::lower::isle::generated_code::ExtKind; diff --git a/cranelift/codegen/src/isa/pulley_shared/inst/emit.rs b/cranelift/codegen/src/isa/pulley_shared/inst/emit.rs index 3382fc1d1ee2..e15cdf59fec7 100644 --- a/cranelift/codegen/src/isa/pulley_shared/inst/emit.rs +++ b/cranelift/codegen/src/isa/pulley_shared/inst/emit.rs @@ -188,7 +188,7 @@ fn pulley_emit

( Inst::Nop => todo!(), - Inst::GetSp { dst } => enc::get_sp(sink, dst), + Inst::GetSpecial { dst, reg } => enc::xmov(sink, dst, reg), Inst::Ret => enc::ret(sink), diff --git a/cranelift/codegen/src/isa/pulley_shared/inst/mod.rs b/cranelift/codegen/src/isa/pulley_shared/inst/mod.rs index 2bfb34cdc8a7..e81bc63109b8 100644 --- a/cranelift/codegen/src/isa/pulley_shared/inst/mod.rs +++ b/cranelift/codegen/src/isa/pulley_shared/inst/mod.rs @@ -79,8 +79,12 @@ fn pulley_get_operands(inst: &mut Inst, collector: &mut impl OperandVisitor) { collector.reg_use(src2); } - Inst::GetSp { dst } => { + Inst::GetSpecial { dst, reg } => { collector.reg_def(dst); + // Note that this is explicitly ignored as this is only used for + // special registers that don't participate in register allocation + // such as the stack pointer, frame pointer, etc. + assert!(reg.is_special()); } Inst::LoadExtName { @@ -568,9 +572,10 @@ impl Inst { Inst::Ret => format!("ret"), - Inst::GetSp { dst } => { + Inst::GetSpecial { dst, reg } => { let dst = format_reg(*dst.to_reg()); - format!("{dst} = get_sp") + let reg = format_reg(**reg); + format!("xmov {dst}, {reg}") } Inst::LoadExtName { dst, name, offset } => { diff --git a/cranelift/codegen/src/isa/pulley_shared/lower.isle b/cranelift/codegen/src/isa/pulley_shared/lower.isle index b03d3b1e33d5..7670b31339ad 100644 --- a/cranelift/codegen/src/isa/pulley_shared/lower.isle +++ b/cranelift/codegen/src/isa/pulley_shared/lower.isle @@ -114,7 +114,12 @@ ;;;; Rules for `get_stack_pointer` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (rule (lower (get_stack_pointer)) - (pulley_get_sp)) + (pulley_get_special (sp_reg))) + +;;;; Rules for `get_frame_pointer` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(rule (lower (get_frame_pointer)) + (pulley_get_special (fp_reg))) ;;;; Rules for `return` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/pulley_shared/lower/isle.rs b/cranelift/codegen/src/isa/pulley_shared/lower/isle.rs index f75fe027f239..1f696123d16a 100644 --- a/cranelift/codegen/src/isa/pulley_shared/lower/isle.rs +++ b/cranelift/codegen/src/isa/pulley_shared/lower/isle.rs @@ -10,6 +10,7 @@ use crate::ir::{condcodes::*, immediates::*, types::*, *}; use crate::isa::pulley_shared::{ abi::*, inst::{FReg, OperandSize, VReg, WritableFReg, WritableVReg, WritableXReg, XReg}, + lower::regs, *, }; use crate::machinst::{ @@ -101,6 +102,14 @@ where fn emit(&mut self, arg0: &MInst) -> Unit { self.lower_ctx.emit(arg0.clone().into()); } + + fn sp_reg(&mut self) -> XReg { + XReg::new(regs::stack_reg()).unwrap() + } + + fn fp_reg(&mut self) -> XReg { + XReg::new(regs::fp_reg()).unwrap() + } } /// The main entry point for lowering with ISLE. diff --git a/cranelift/filetests/filetests/isa/pulley32/get_stack_pointer.clif b/cranelift/filetests/filetests/isa/pulley32/get_stack_pointer.clif deleted file mode 100644 index f269efa92e36..000000000000 --- a/cranelift/filetests/filetests/isa/pulley32/get_stack_pointer.clif +++ /dev/null @@ -1,18 +0,0 @@ -test compile precise-output -target pulley32 - -function %get_stack_pointer() -> i32 { -block0: - v0 = get_stack_pointer.i32 - return v0 -} - -; VCode: -; block0: -; x0 = get_sp -; ret -; -; Disassembled: -; get_sp x0 -; ret - diff --git a/cranelift/filetests/filetests/isa/pulley32/special_regs.clif b/cranelift/filetests/filetests/isa/pulley32/special_regs.clif new file mode 100644 index 000000000000..d46e04c741f5 --- /dev/null +++ b/cranelift/filetests/filetests/isa/pulley32/special_regs.clif @@ -0,0 +1,70 @@ +test compile precise-output +set preserve_frame_pointers +target pulley32 + +function %get_stack_pointer() -> i32 { +block0: + v0 = get_stack_pointer.i32 + return v0 +} + +; VCode: +; x30 = xconst8 -16 +; x27 = xadd32 x27, x30 +; store64 sp+8, x28 // flags = notrap aligned +; store64 sp+0, x29 // flags = notrap aligned +; x29 = xmov x27 +; block0: +; xmov x0, x27 +; x28 = load64_u sp+8 // flags = notrap aligned +; x29 = load64_u sp+0 // flags = notrap aligned +; x30 = xconst8 16 +; x27 = xadd32 x27, x30 +; ret +; +; Disassembled: +; xconst8 spilltmp0, -16 +; xadd32 sp, sp, spilltmp0 +; store64_offset8 sp, 8, lr +; store64 sp, fp +; xmov fp, sp +; xmov x0, sp +; load64_offset8 lr, sp, 8 +; load64 fp, sp +; xconst8 spilltmp0, 16 +; xadd32 sp, sp, spilltmp0 +; ret + +function %get_frame_pointer() -> i32 { +block0: + v0 = get_frame_pointer.i32 + return v0 +} + +; VCode: +; x30 = xconst8 -16 +; x27 = xadd32 x27, x30 +; store64 sp+8, x28 // flags = notrap aligned +; store64 sp+0, x29 // flags = notrap aligned +; x29 = xmov x27 +; block0: +; xmov x0, x29 +; x28 = load64_u sp+8 // flags = notrap aligned +; x29 = load64_u sp+0 // flags = notrap aligned +; x30 = xconst8 16 +; x27 = xadd32 x27, x30 +; ret +; +; Disassembled: +; xconst8 spilltmp0, -16 +; xadd32 sp, sp, spilltmp0 +; store64_offset8 sp, 8, lr +; store64 sp, fp +; xmov fp, sp +; xmov x0, fp +; load64_offset8 lr, sp, 8 +; load64 fp, sp +; xconst8 spilltmp0, 16 +; xadd32 sp, sp, spilltmp0 +; ret + diff --git a/cranelift/filetests/filetests/isa/pulley64/get_stack_pointer.clif b/cranelift/filetests/filetests/isa/pulley64/get_stack_pointer.clif deleted file mode 100644 index e3df1187e779..000000000000 --- a/cranelift/filetests/filetests/isa/pulley64/get_stack_pointer.clif +++ /dev/null @@ -1,18 +0,0 @@ -test compile precise-output -target pulley64 - -function %get_stack_pointer() -> i64 { -block0: - v0 = get_stack_pointer.i64 - return v0 -} - -; VCode: -; block0: -; x0 = get_sp -; ret -; -; Disassembled: -; get_sp x0 -; ret - diff --git a/cranelift/filetests/filetests/isa/pulley64/special_regs.clif b/cranelift/filetests/filetests/isa/pulley64/special_regs.clif new file mode 100644 index 000000000000..ad0dcac017b5 --- /dev/null +++ b/cranelift/filetests/filetests/isa/pulley64/special_regs.clif @@ -0,0 +1,70 @@ +test compile precise-output +set preserve_frame_pointers +target pulley64 + +function %get_stack_pointer() -> i64 { +block0: + v0 = get_stack_pointer.i64 + return v0 +} + +; VCode: +; x30 = xconst8 -16 +; x27 = xadd32 x27, x30 +; store64 sp+8, x28 // flags = notrap aligned +; store64 sp+0, x29 // flags = notrap aligned +; x29 = xmov x27 +; block0: +; xmov x0, x27 +; x28 = load64_u sp+8 // flags = notrap aligned +; x29 = load64_u sp+0 // flags = notrap aligned +; x30 = xconst8 16 +; x27 = xadd32 x27, x30 +; ret +; +; Disassembled: +; xconst8 spilltmp0, -16 +; xadd32 sp, sp, spilltmp0 +; store64_offset8 sp, 8, lr +; store64 sp, fp +; xmov fp, sp +; xmov x0, sp +; load64_offset8 lr, sp, 8 +; load64 fp, sp +; xconst8 spilltmp0, 16 +; xadd32 sp, sp, spilltmp0 +; ret + +function %get_frame_pointer() -> i64 { +block0: + v0 = get_frame_pointer.i64 + return v0 +} + +; VCode: +; x30 = xconst8 -16 +; x27 = xadd32 x27, x30 +; store64 sp+8, x28 // flags = notrap aligned +; store64 sp+0, x29 // flags = notrap aligned +; x29 = xmov x27 +; block0: +; xmov x0, x29 +; x28 = load64_u sp+8 // flags = notrap aligned +; x29 = load64_u sp+0 // flags = notrap aligned +; x30 = xconst8 16 +; x27 = xadd32 x27, x30 +; ret +; +; Disassembled: +; xconst8 spilltmp0, -16 +; xadd32 sp, sp, spilltmp0 +; store64_offset8 sp, 8, lr +; store64 sp, fp +; xmov fp, sp +; xmov x0, fp +; load64_offset8 lr, sp, 8 +; load64 fp, sp +; xconst8 spilltmp0, 16 +; xadd32 sp, sp, spilltmp0 +; ret + diff --git a/pulley/fuzz/src/interp.rs b/pulley/fuzz/src/interp.rs index 826d0ad30f86..e0f990e7ea93 100644 --- a/pulley/fuzz/src/interp.rs +++ b/pulley/fuzz/src/interp.rs @@ -125,7 +125,6 @@ fn extended_op_is_safe_for_fuzzing(op: &ExtendedOp) -> bool { match op { ExtendedOp::Trap(_) => true, ExtendedOp::Nop(_) => true, - ExtendedOp::GetSp(GetSp { dst, .. }) => !dst.is_special(), ExtendedOp::CallIndirectHost(_) => false, } } diff --git a/pulley/src/interp.rs b/pulley/src/interp.rs index 1724f2a7d910..536606923308 100644 --- a/pulley/src/interp.rs +++ b/pulley/src/interp.rs @@ -1248,12 +1248,6 @@ impl ExtendedOpVisitor for Interpreter<'_> { ControlFlow::Break(Done::Trap(self.pc.as_ptr())) } - fn get_sp(&mut self, dst: XReg) -> ControlFlow { - let sp = self.state[XReg::sp].get_u64(); - self.state[dst].set_u64(sp); - ControlFlow::Continue(()) - } - fn call_indirect_host(&mut self, sig: u8) -> ControlFlow { let _ = sig; // TODO: should stash this somewhere ControlFlow::Break(Done::ReturnToHost) diff --git a/pulley/src/lib.rs b/pulley/src/lib.rs index abb91df56b01..06f882ec9fdf 100644 --- a/pulley/src/lib.rs +++ b/pulley/src/lib.rs @@ -203,8 +203,6 @@ macro_rules! for_each_extended_op { /// Do nothing. nop = Nop; - /// Copy the special `sp` stack pointer register into an `x` register. - get_sp = GetSp { dst: XReg }; /// A special opcode to use an indirect function call to reenter the /// host from the interpreter. ///