From 2dd8b1c8d54910f5c2a0fae2267828276d6e9d52 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 1 Aug 2022 16:20:26 -0700 Subject: [PATCH 1/7] Cranelift: Add instructions for getting the current stack/frame pointers and return address This is the initial part of https://github.com/bytecodealliance/wasmtime/issues/4535 --- .../codegen/meta/src/shared/instructions.rs | 35 +++++++++++++ cranelift/codegen/src/isa/aarch64/inst.isle | 34 ++++++++++++ .../codegen/src/isa/aarch64/inst/emit.rs | 9 ++++ cranelift/codegen/src/isa/aarch64/inst/mod.rs | 12 +++++ cranelift/codegen/src/isa/aarch64/lower.isle | 12 +++++ .../codegen/src/isa/aarch64/lower/isle.rs | 13 +++++ .../codegen/src/isa/aarch64/lower_inst.rs | 4 ++ cranelift/codegen/src/isa/s390x/inst.isle | 25 +++++++++ cranelift/codegen/src/isa/s390x/inst/emit.rs | 6 +++ cranelift/codegen/src/isa/s390x/inst/mod.rs | 11 ++++ cranelift/codegen/src/isa/s390x/lower.isle | 10 ++++ cranelift/codegen/src/isa/s390x/lower.rs | 5 +- cranelift/codegen/src/isa/s390x/lower/isle.rs | 15 +++++- cranelift/codegen/src/isa/x64/encoding/rex.rs | 39 +++++++++----- cranelift/codegen/src/isa/x64/inst.isle | 32 +++++++++++- cranelift/codegen/src/isa/x64/inst/args.rs | 20 ++++--- cranelift/codegen/src/isa/x64/inst/emit.rs | 10 ++++ cranelift/codegen/src/isa/x64/inst/mod.rs | 13 +++++ cranelift/codegen/src/isa/x64/lower.isle | 13 +++++ cranelift/codegen/src/isa/x64/lower.rs | 5 +- cranelift/codegen/src/isa/x64/lower/isle.rs | 11 ++++ cranelift/codegen/src/machinst/isle.rs | 10 ++++ cranelift/codegen/src/prelude.isle | 12 +++++ cranelift/codegen/src/verifier/mod.rs | 19 +++++++ .../filetests/isa/aarch64/fp_sp_pc.clif | 43 +++++++++++++++ .../filetests/isa/s390x/fp_sp_pc.clif | 52 +++++++++++++++++++ .../filetests/filetests/isa/x64/fp_sp_pc.clif | 45 ++++++++++++++++ cranelift/interpreter/src/step.rs | 3 ++ 28 files changed, 494 insertions(+), 24 deletions(-) create mode 100644 cranelift/filetests/filetests/isa/aarch64/fp_sp_pc.clif create mode 100644 cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif create mode 100644 cranelift/filetests/filetests/isa/x64/fp_sp_pc.clif diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index d7ceb2d03408..93d64dae3f8f 100644 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -1314,6 +1314,41 @@ pub(crate) fn define( .other_side_effects(true), ); + ig.push( + Inst::new( + "get_frame_pointer", + r#" + Get the address in the frame pointer register. + + Usage of this instruction requires setting `preserve_frame_pointers` to `true`. + "#, + &formats.nullary, + ) + .operands_out(vec![addr]), + ); + + ig.push( + Inst::new( + "get_stack_pointer", + r#" + Get the address in the stack pointer register. + "#, + &formats.nullary, + ) + .operands_out(vec![addr]), + ); + + ig.push( + Inst::new( + "get_return_address", + r#" + Get the PC where this function will transfer control to when it returns. + "#, + &formats.nullary, + ) + .operands_out(vec![addr]), + ); + let TableOffset = &TypeVar::new( "TableOffset", "An unsigned table offset", diff --git a/cranelift/codegen/src/isa/aarch64/inst.isle b/cranelift/codegen/src/isa/aarch64/inst.isle index 58f5c0ea3e98..659594f3542e 100644 --- a/cranelift/codegen/src/isa/aarch64/inst.isle +++ b/cranelift/codegen/src/isa/aarch64/inst.isle @@ -165,6 +165,12 @@ (rd WritableReg) (rm Reg)) + ;; Like `Move` but with a particular `PReg` source (for implementing CLIF + ;; instructions like `get_stack_pointer`). + (MovPReg + (rd WritableReg) + (rm PReg)) + ;; A MOV[Z,N,K] with a 16-bit immediate. (MovWide (op MoveWideOp) @@ -2426,3 +2432,31 @@ ;; And finally, copy the preordained AtomicCASLoop output reg to its destination. ;; Also, x24 and x28 are trashed. (mov64_from_real 27))) + +;; Helper for emitting `MInst.MovPReg` instructions. +(decl mov_preg (PReg) Reg) +(rule (mov_preg src) + (let ((dst WritableReg (temp_writable_reg $I64)) + (_ Unit (emit (MInst.MovPReg dst src)))) + dst)) + +(decl preg_sp () PReg) +(extern constructor preg_sp preg_sp) + +(decl preg_fp () PReg) +(extern constructor preg_fp preg_fp) + +(decl preg_link () PReg) +(extern constructor preg_link preg_link) + +(decl aarch64_sp () Reg) +(rule (aarch64_sp) + (mov_preg (preg_sp))) + +(decl aarch64_fp () Reg) +(rule (aarch64_fp) + (mov_preg (preg_fp))) + +(decl aarch64_link () Reg) +(rule (aarch64_link) + (mov_preg (preg_link))) diff --git a/cranelift/codegen/src/isa/aarch64/inst/emit.rs b/cranelift/codegen/src/isa/aarch64/inst/emit.rs index b504e675be61..5bd70f3df622 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/emit.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/emit.rs @@ -1337,6 +1337,15 @@ impl MachInstEmit for Inst { } } } + &Inst::MovPReg { rd, rm } => { + let rd = allocs.next_writable(rd); + let rm: Reg = rm.into(); + debug_assert!([regs::fp_reg(), regs::stack_reg(), regs::link_reg()].contains(&rm)); + assert!(rm.class() == RegClass::Int); + assert!(rd.to_reg().class() == rm.class()); + let size = OperandSize::Size64; + Inst::Mov { size, rd, rm }.emit(&[], sink, emit_info, state); + } &Inst::MovWide { op, rd, imm, size } => { let rd = allocs.next_writable(rd); sink.put4(enc_move_wide(op, rd, imm, size)); diff --git a/cranelift/codegen/src/isa/aarch64/inst/mod.rs b/cranelift/codegen/src/isa/aarch64/inst/mod.rs index 74a094d9f515..e105be05a78f 100644 --- a/cranelift/codegen/src/isa/aarch64/inst/mod.rs +++ b/cranelift/codegen/src/isa/aarch64/inst/mod.rs @@ -649,6 +649,13 @@ fn aarch64_get_operands VReg>(inst: &Inst, collector: &mut Operan collector.reg_def(rd); collector.reg_use(rm); } + &Inst::MovPReg { rd, rm } => { + debug_assert!( + [regs::fp_reg(), regs::stack_reg(), regs::link_reg()].contains(&rm.into()) + ); + debug_assert!(rd.to_reg().is_virtual()); + collector.reg_def(rd); + } &Inst::MovWide { op, rd, .. } => match op { MoveWideOp::MovK => collector.reg_mod(rd), _ => collector.reg_def(rd), @@ -1482,6 +1489,11 @@ impl Inst { let rm = pretty_print_ireg(rm, size, allocs); format!("mov {}, {}", rd, rm) } + &Inst::MovPReg { rd, rm } => { + let rd = pretty_print_ireg(rd.to_reg(), OperandSize::Size64, allocs); + let rm = show_ireg_sized(rm.into(), OperandSize::Size64); + format!("mov {}, {}", rd, rm) + } &Inst::MovWide { op, rd, diff --git a/cranelift/codegen/src/isa/aarch64/lower.isle b/cranelift/codegen/src/isa/aarch64/lower.isle index ce536c119809..98fa987a3df5 100644 --- a/cranelift/codegen/src/isa/aarch64/lower.isle +++ b/cranelift/codegen/src/isa/aarch64/lower.isle @@ -1687,6 +1687,7 @@ ;;;; Rules for `uunarrow` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + (rule (lower (has_type (ty_vec128_int ty) (uunarrow x y))) (if (zero_value y)) (uqxtn x (lane_size ty))) @@ -1723,3 +1724,14 @@ (rule (lower (debugtrap)) (side_effect (brk))) + +;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;;; + +(rule (lower (get_frame_pointer)) + (aarch64_fp)) + +(rule (lower (get_stack_pointer)) + (aarch64_sp)) + +(rule (lower (get_return_address)) + (aarch64_link)) diff --git a/cranelift/codegen/src/isa/aarch64/lower/isle.rs b/cranelift/codegen/src/isa/aarch64/lower/isle.rs index d13e4123ffcb..aa9928d91a58 100644 --- a/cranelift/codegen/src/isa/aarch64/lower/isle.rs +++ b/cranelift/codegen/src/isa/aarch64/lower/isle.rs @@ -26,6 +26,7 @@ use crate::{ isa::unwind::UnwindInst, machinst::{ty_bits, InsnOutput, LowerCtx, VCodeConstant, VCodeConstantData}, }; +use regalloc2::PReg; use std::boxed::Box; use std::convert::TryFrom; use std::vec::Vec; @@ -466,4 +467,16 @@ where rd.to_reg() } + + fn preg_sp(&mut self) -> PReg { + super::regs::stack_reg().to_real_reg().unwrap().into() + } + + fn preg_fp(&mut self) -> PReg { + super::regs::fp_reg().to_real_reg().unwrap().into() + } + + fn preg_link(&mut self) -> PReg { + super::regs::link_reg().to_real_reg().unwrap().into() + } } diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index 3570286c045f..e626e7492fea 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -53,6 +53,10 @@ pub(crate) fn lower_insn_to_regs>( point, as constants are rematerialized at use-sites" ), + Opcode::GetFramePointer | Opcode::GetStackPointer | Opcode::GetReturnAddress => { + implemented_in_isle(ctx) + } + Opcode::Iadd => implemented_in_isle(ctx), Opcode::Isub => implemented_in_isle(ctx), Opcode::UaddSat | Opcode::SaddSat | Opcode::UsubSat | Opcode::SsubSat => { diff --git a/cranelift/codegen/src/isa/s390x/inst.isle b/cranelift/codegen/src/isa/s390x/inst.isle index 60f8c7e80f9c..86f8fa8ff64e 100644 --- a/cranelift/codegen/src/isa/s390x/inst.isle +++ b/cranelift/codegen/src/isa/s390x/inst.isle @@ -373,6 +373,11 @@ (rd WritableReg) (rm Reg)) + ;; Like `Mov64` but with a particular physical register source. + (MovPReg + (rd WritableReg) + (rm PReg)) + ;; A 32-bit move instruction with a full 32-bit immediate. (Mov32Imm (rd WritableReg) @@ -2473,6 +2478,26 @@ (rule (emit_load $I64 dst addr) (emit (MInst.Load64 dst addr))) +;; Helper for creating `MInst.MovPReg` instructions. +(decl mov_preg (PReg) Reg) +(rule (mov_preg src) + (let ((dst WritableReg (temp_writable_reg $I64)) + (_ Unit (emit (MInst.MovPReg dst src)))) + dst)) + +(decl preg_r14 () PReg) +(extern constructor preg_r14 preg_r14) + +(decl preg_r15 () PReg) +(extern constructor preg_r15 preg_r15) + +(decl r14 () Reg) +(rule (r14) + (mov_preg (preg_r14))) + +(decl r15 () Reg) +(rule (r15) + (mov_preg (preg_r15))) ;; Helpers for accessing argument / return value slots ;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/s390x/inst/emit.rs b/cranelift/codegen/src/isa/s390x/inst/emit.rs index 6ef6ccbcbc10..08d9074e757e 100644 --- a/cranelift/codegen/src/isa/s390x/inst/emit.rs +++ b/cranelift/codegen/src/isa/s390x/inst/emit.rs @@ -2082,6 +2082,12 @@ impl MachInstEmit for Inst { let opcode = 0xb904; // LGR put(sink, &enc_rre(opcode, rd.to_reg(), rm)); } + &Inst::MovPReg { rd, rm } => { + let rm: Reg = rm.into(); + debug_assert!([regs::gpr(14), regs::gpr(15)].contains(&rm)); + let rd = allocs.next_writable(rd); + Inst::Mov64 { rd, rm }.emit(&[], sink, emit_info, state); + } &Inst::Mov32 { rd, rm } => { let rd = allocs.next_writable(rd); let rm = allocs.next(rm); diff --git a/cranelift/codegen/src/isa/s390x/inst/mod.rs b/cranelift/codegen/src/isa/s390x/inst/mod.rs index ad5af092bc6b..f46182ddff97 100644 --- a/cranelift/codegen/src/isa/s390x/inst/mod.rs +++ b/cranelift/codegen/src/isa/s390x/inst/mod.rs @@ -144,6 +144,7 @@ impl Inst { | Inst::StoreMultiple64 { .. } | Inst::Mov32 { .. } | Inst::Mov64 { .. } + | Inst::MovPReg { .. } | Inst::Mov32Imm { .. } | Inst::Mov32SImm16 { .. } | Inst::Mov64SImm16 { .. } @@ -624,6 +625,11 @@ fn s390x_get_operands VReg>(inst: &Inst, collector: &mut OperandC collector.reg_def(rd); collector.reg_use(rm); } + &Inst::MovPReg { rd, rm } => { + debug_assert!([regs::gpr(14), regs::gpr(15)].contains(&rm.into())); + debug_assert!(rd.to_reg().is_virtual()); + collector.reg_def(rd); + } &Inst::Mov32 { rd, rm } => { collector.reg_def(rd); collector.reg_use(rm); @@ -1787,6 +1793,11 @@ impl Inst { let rm = pretty_print_reg(rm, allocs); format!("lgr {}, {}", rd, rm) } + &Inst::MovPReg { rd, rm } => { + let rd = pretty_print_reg(rd.to_reg(), allocs); + let rm = show_reg(rm.into()); + format!("lgr {}, {}", rd, rm) + } &Inst::Mov32 { rd, rm } => { let rd = pretty_print_reg(rd.to_reg(), allocs); let rm = pretty_print_reg(rm, allocs); diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index 5978eeaa89bb..227602817d6a 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -3619,3 +3619,13 @@ (_ Unit (output_builder_push builder ret))) (lower_call_rets abi tail builder))) +;;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;; + +(rule (lower (get_stack_pointer)) + (r15)) + +(rule (lower (get_frame_pointer)) + (load64 (memarg_stack_off 0 0))) + +(rule (lower (get_return_address)) + (r14)) diff --git a/cranelift/codegen/src/isa/s390x/lower.rs b/cranelift/codegen/src/isa/s390x/lower.rs index df93c47023dd..3bce8f8b778b 100644 --- a/cranelift/codegen/src/isa/s390x/lower.rs +++ b/cranelift/codegen/src/isa/s390x/lower.rs @@ -181,7 +181,10 @@ impl LowerBackend for S390xBackend { | Opcode::Return | Opcode::StackAddr | Opcode::FuncAddr - | Opcode::SymbolValue => { + | Opcode::SymbolValue + | Opcode::GetFramePointer + | Opcode::GetStackPointer + | Opcode::GetReturnAddress => { unreachable!( "implemented in ISLE: inst = `{}`, type = `{:?}`", ctx.dfg().display_inst(ir_inst), diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs index 34eec0097f56..cebbde4f4d7e 100644 --- a/cranelift/codegen/src/isa/s390x/lower/isle.rs +++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs @@ -6,8 +6,8 @@ pub mod generated_code; // Types that the generated ISLE code uses via `use super::*`. use crate::isa::s390x::abi::S390xMachineDeps; use crate::isa::s390x::inst::{ - stack_reg, writable_gpr, zero_reg, CallIndInfo, CallInfo, Cond, Inst as MInst, MemArg, UImm12, - UImm16Shifted, UImm32Shifted, + regs, stack_reg, writable_gpr, zero_reg, CallIndInfo, CallInfo, Cond, Inst as MInst, MemArg, + UImm12, UImm16Shifted, UImm32Shifted, }; use crate::isa::s390x::settings::Flags as IsaFlags; use crate::machinst::isle::*; @@ -21,6 +21,7 @@ use crate::{ isa::unwind::UnwindInst, machinst::{InsnOutput, LowerCtx, VCodeConstant, VCodeConstantData}, }; +use regalloc2::PReg; use std::boxed::Box; use std::cell::Cell; use std::convert::TryFrom; @@ -670,6 +671,16 @@ where fn emit(&mut self, inst: &MInst) -> Unit { self.lower_ctx.emit(inst.clone()); } + + #[inline] + fn preg_r14(&mut self) -> PReg { + regs::gpr(14).to_real_reg().unwrap().into() + } + + #[inline] + fn preg_r15(&mut self) -> PReg { + stack_reg().to_real_reg().unwrap().into() + } } /// Zero-extend the low `from_bits` bits of `value` to a full u64. diff --git a/cranelift/codegen/src/isa/x64/encoding/rex.rs b/cranelift/codegen/src/isa/x64/encoding/rex.rs index bfa3c089bee4..598a09de4367 100644 --- a/cranelift/codegen/src/isa/x64/encoding/rex.rs +++ b/cranelift/codegen/src/isa/x64/encoding/rex.rs @@ -298,16 +298,29 @@ pub(crate) fn emit_std_enc_mem( prefixes.emit(sink); + let mem_e = match mem_e.clone() { + Amode::RbpOffset { simm32, flags } => { + let base = regs::rbp(); + Amode::ImmReg { + simm32, + base, + flags, + } + } + other => other, + }; + match mem_e { + Amode::RbpOffset { .. } => unreachable!(), Amode::ImmReg { simm32, base, .. } => { // If this is an access based off of RSP, it may trap with a stack overflow if it's the // first touch of a new stack page. - if *base == regs::rsp() && !can_trap && info.flags.enable_probestack() { + if base == regs::rsp() && !can_trap && info.flags.enable_probestack() { sink.add_trap(TrapCode::StackOverflow); } // First, the REX byte. - let enc_e = int_reg_enc(*base); + let enc_e = int_reg_enc(base); rex.emit_two_op(sink, enc_g, enc_e); // Now the opcode(s). These include any other prefixes the caller @@ -319,7 +332,7 @@ pub(crate) fn emit_std_enc_mem( // Now the mod/rm and associated immediates. This is // significantly complicated due to the multiple special cases. - if *simm32 == 0 + if simm32 == 0 && enc_e != regs::ENC_RSP && enc_e != regs::ENC_RBP && enc_e != regs::ENC_R12 @@ -329,10 +342,10 @@ pub(crate) fn emit_std_enc_mem( // replaced by a single mask-and-compare check. We should do // that because this routine is likely to be hot. sink.put1(encode_modrm(0, enc_g & 7, enc_e & 7)); - } else if *simm32 == 0 && (enc_e == regs::ENC_RSP || enc_e == regs::ENC_R12) { + } else if simm32 == 0 && (enc_e == regs::ENC_RSP || enc_e == regs::ENC_R12) { sink.put1(encode_modrm(0, enc_g & 7, 4)); sink.put1(0x24); - } else if low8_will_sign_extend_to_32(*simm32) + } else if low8_will_sign_extend_to_32(simm32) && enc_e != regs::ENC_RSP && enc_e != regs::ENC_R12 { @@ -340,9 +353,9 @@ pub(crate) fn emit_std_enc_mem( sink.put1((simm32 & 0xFF) as u8); } else if enc_e != regs::ENC_RSP && enc_e != regs::ENC_R12 { sink.put1(encode_modrm(2, enc_g & 7, enc_e & 7)); - sink.put4(*simm32); + sink.put4(simm32); } else if (enc_e == regs::ENC_RSP || enc_e == regs::ENC_R12) - && low8_will_sign_extend_to_32(*simm32) + && low8_will_sign_extend_to_32(simm32) { // REX.B distinguishes RSP from R12 sink.put1(encode_modrm(1, enc_g & 7, 4)); @@ -353,7 +366,7 @@ pub(crate) fn emit_std_enc_mem( // REX.B distinguishes RSP from R12 sink.put1(encode_modrm(2, enc_g & 7, 4)); sink.put1(0x24); - sink.put4(*simm32); + sink.put4(simm32); } else { unreachable!("ImmReg"); } @@ -385,14 +398,14 @@ pub(crate) fn emit_std_enc_mem( } // modrm, SIB, immediates. - if low8_will_sign_extend_to_32(*simm32) && enc_index != regs::ENC_RSP { + if low8_will_sign_extend_to_32(simm32) && enc_index != regs::ENC_RSP { sink.put1(encode_modrm(1, enc_g & 7, 4)); - sink.put1(encode_sib(*shift, enc_index & 7, enc_base & 7)); - sink.put1(*simm32 as u8); + sink.put1(encode_sib(shift, enc_index & 7, enc_base & 7)); + sink.put1(simm32 as u8); } else if enc_index != regs::ENC_RSP { sink.put1(encode_modrm(2, enc_g & 7, 4)); - sink.put1(encode_sib(*shift, enc_index & 7, enc_base & 7)); - sink.put4(*simm32); + sink.put1(encode_sib(shift, enc_index & 7, enc_base & 7)); + sink.put4(simm32); } else { panic!("ImmRegRegShift"); } diff --git a/cranelift/codegen/src/isa/x64/inst.isle b/cranelift/codegen/src/isa/x64/inst.isle index 519688813f70..f5e5c0c94eb2 100644 --- a/cranelift/codegen/src/isa/x64/inst.isle +++ b/cranelift/codegen/src/isa/x64/inst.isle @@ -104,6 +104,11 @@ (src Gpr) (dst WritableGpr)) + ;; Like `MovRR` but with a physical register source (for implementing + ;; CLIF instructions like `get_stack_pointer`). + (MovPReg (src PReg) + (dst WritableGpr)) + ;; Zero-extended loads, except for 64 bits: movz (bl bq wl wq lq) addr ;; reg. ;; @@ -795,7 +800,11 @@ ;; pointer). The appropriate relocation is emitted so ;; that the resulting immediate makes this Amode refer to ;; the given MachLabel. - (RipRelative (target MachLabel)))) + (RipRelative (target MachLabel)) + + ;; Load `rbp + sign_extend(simm32)`. + (RbpOffset (simm32 u32) + (flags MemFlags)))) ;; Some Amode constructor helpers. @@ -3333,3 +3342,24 @@ (decl synthetic_amode_to_xmm_mem (SyntheticAmode) XmmMem) (rule (synthetic_amode_to_xmm_mem amode) (synthetic_amode_to_reg_mem amode)) + +;; Helper for creating `MovPReg` instructions. +(decl mov_preg (PReg) Reg) +(rule (mov_preg preg) + (let ((dst WritableGpr (temp_writable_gpr)) + (_ Unit (emit (MInst.MovPReg preg dst)))) + dst)) + +(decl preg_rbp () PReg) +(extern constructor preg_rbp preg_rbp) + +(decl preg_rsp () PReg) +(extern constructor preg_rsp preg_rsp) + +(decl x64_rbp () Reg) +(rule (x64_rbp) + (mov_preg (preg_rbp))) + +(decl x64_rsp () Reg) +(rule (x64_rsp) + (mov_preg (preg_rsp))) diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index 04da226f89cb..fa9f2fdf08e1 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -319,8 +319,8 @@ impl Amode { collector.reg_use(base.to_reg()); collector.reg_use(index.to_reg()); } - Amode::RipRelative { .. } => { - // RIP isn't involved in regalloc. + Amode::RipRelative { .. } | Amode::RbpOffset { .. } => { + // RIP and RBP aren't involved in regalloc. } } } @@ -338,16 +338,17 @@ impl Amode { collector.reg_late_use(base.to_reg()); collector.reg_late_use(index.to_reg()); } - Amode::RipRelative { .. } => { - // RIP isn't involved in regalloc. + Amode::RipRelative { .. } | Amode::RbpOffset { .. } => { + // RIP and RBP aren't involved in regalloc. } } } pub(crate) fn get_flags(&self) -> MemFlags { match self { - Amode::ImmReg { flags, .. } => *flags, - Amode::ImmRegRegShift { flags, .. } => *flags, + Amode::RbpOffset { flags, .. } + | Amode::ImmReg { flags, .. } + | Amode::ImmRegRegShift { flags, .. } => *flags, Amode::RipRelative { .. } => MemFlags::trusted(), } } @@ -383,6 +384,7 @@ impl Amode { index: Gpr::new(allocs.next(*index)).unwrap(), }, &Amode::RipRelative { target } => Amode::RipRelative { target }, + &Amode::RbpOffset { simm32, flags } => Amode::RbpOffset { simm32, flags }, } } @@ -401,6 +403,12 @@ impl Amode { impl PrettyPrint for Amode { fn pretty_print(&self, _size: u8, allocs: &mut AllocationConsumer<'_>) -> String { match self { + Amode::RbpOffset { simm32, flags } => Amode::ImmReg { + simm32: *simm32, + base: regs::rbp(), + flags: *flags, + } + .pretty_print(8, allocs), Amode::ImmReg { simm32, base, .. } => { // Note: size is always 8; the address is 64 bits, // even if the addressed operand is smaller. diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs index 9552993ee0de..b00d6b6dd473 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit.rs +++ b/cranelift/codegen/src/isa/x64/inst/emit.rs @@ -678,6 +678,16 @@ pub(crate) fn emit( ); } + Inst::MovPReg { src, dst } => { + let src: Reg = (*src).into(); + debug_assert!([regs::rsp(), regs::rbp()].contains(&src)); + let src = Gpr::new(src).unwrap(); + let size = OperandSize::Size64; + let dst = allocs.next(dst.to_reg().to_reg()); + let dst = WritableGpr::from_writable_reg(Writable::from_reg(dst)).unwrap(); + Inst::MovRR { size, src, dst }.emit(&[], sink, info, state); + } + Inst::MovzxRmR { ext_mode, src, dst } => { let dst = allocs.next(dst.to_reg().to_reg()); let (opcodes, num_opcodes, mut rex_flags) = match ext_mode { diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index 4ceda3198b39..cd947454543b 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/cranelift/codegen/src/isa/x64/inst/mod.rs @@ -92,6 +92,7 @@ impl Inst { | Inst::Mov64MR { .. } | Inst::MovRM { .. } | Inst::MovRR { .. } + | Inst::MovPReg { .. } | Inst::MovsxRmR { .. } | Inst::MovzxRmR { .. } | Inst::MulHi { .. } @@ -1428,6 +1429,13 @@ impl PrettyPrint for Inst { ) } + Inst::MovPReg { src, dst } => { + let src: Reg = (*src).into(); + let src = regs::show_ireg_sized(src, 8); + let dst = pretty_print_reg(dst.to_reg().to_reg(), 8, allocs); + format!("{} {}, {}", ljustify("movq".to_string()), src, dst) + } + Inst::MovzxRmR { ext_mode, src, dst, .. } => { @@ -1991,6 +1999,11 @@ fn x64_get_operands VReg>(inst: &Inst, collector: &mut OperandCol collector.reg_use(src.to_reg()); collector.reg_def(dst.to_writable_reg()); } + Inst::MovPReg { dst, src } => { + debug_assert!([regs::rsp(), regs::rbp()].contains(&(*src).into())); + debug_assert!(dst.to_reg().to_reg().is_virtual()); + collector.reg_def(dst.to_writable_reg()); + } Inst::XmmToGpr { src, dst, .. } => { collector.reg_use(src.to_reg()); collector.reg_def(dst.to_writable_reg()); diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index 24e0672018f6..199fe8d483ad 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -2842,3 +2842,16 @@ (rule (lower (call_indirect sig_ref val inputs)) (gen_call_indirect sig_ref val inputs)) + +;;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;; + +(rule (lower (get_frame_pointer)) + (x64_rbp)) + +(rule (lower (get_stack_pointer)) + (x64_rsp)) + +(rule (lower (get_return_address)) + (x64_load $I64 + (Amode.RbpOffset 8 (mem_flags_trusted)) + (ExtKind.None))) diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index 2fd5c2e8ddd3..d372d50dd53e 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -928,7 +928,10 @@ fn lower_insn_to_regs>( | Opcode::Call | Opcode::CallIndirect | Opcode::Trapif - | Opcode::Trapff => { + | Opcode::Trapff + | Opcode::GetFramePointer + | Opcode::GetStackPointer + | Opcode::GetReturnAddress => { implemented_in_isle(ctx); } diff --git a/cranelift/codegen/src/isa/x64/lower/isle.rs b/cranelift/codegen/src/isa/x64/lower/isle.rs index 8344ae0c6166..70b5ccb2b61f 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle.rs +++ b/cranelift/codegen/src/isa/x64/lower/isle.rs @@ -31,6 +31,7 @@ use crate::{ VCodeConstant, VCodeConstantData, }, }; +use regalloc2::PReg; use smallvec::SmallVec; use std::boxed::Box; use std::convert::TryFrom; @@ -636,6 +637,16 @@ where self.gen_call_common(abi, num_rets, caller, args) } + + #[inline] + fn preg_rbp(&mut self) -> PReg { + regs::rbp().to_real_reg().unwrap().into() + } + + #[inline] + fn preg_rsp(&mut self) -> PReg { + regs::rsp().to_real_reg().unwrap().into() + } } impl IsleContext<'_, C, Flags, IsaFlags, 6> diff --git a/cranelift/codegen/src/machinst/isle.rs b/cranelift/codegen/src/machinst/isle.rs index f463f6e88361..0976667d1ea6 100644 --- a/cranelift/codegen/src/machinst/isle.rs +++ b/cranelift/codegen/src/machinst/isle.rs @@ -884,6 +884,16 @@ macro_rules! isle_prelude_methods { fn sink_inst(&mut self, inst: Inst) { self.lower_ctx.sink_inst(inst); } + + #[inline] + fn mem_flags_trusted(&mut self) -> MemFlags { + MemFlags::trusted() + } + + #[inline] + fn preg_to_reg(&mut self, preg: PReg) -> Reg { + preg.into() + } }; } diff --git a/cranelift/codegen/src/prelude.isle b/cranelift/codegen/src/prelude.isle index 381327b1cb51..6f915141608c 100644 --- a/cranelift/codegen/src/prelude.isle +++ b/cranelift/codegen/src/prelude.isle @@ -84,6 +84,7 @@ (type OptionWritableReg (primitive OptionWritableReg)) (type VecReg extern (enum)) (type VecWritableReg extern (enum)) +(type PReg (primitive PReg)) ;; Construct a `ValueRegs` of one register. (decl value_reg (Reg) ValueRegs) @@ -196,6 +197,10 @@ (let ((regs ValueRegs (put_in_regs val))) (value_regs_get regs 0))) +;; Convert a `PReg` into a `Reg` +(decl preg_to_reg (PReg) Reg) +(extern constructor preg_to_reg preg_to_reg) + ;;;; Common Mach Types ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (type MachLabel (primitive MachLabel)) @@ -293,6 +298,12 @@ (decl lane_type (Type) Type) (extern constructor lane_type lane_type) +;;;; `cranelift_codegen::ir::MemFlags ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +;; `MemFlags::trusted` +(decl mem_flags_trusted () MemFlags) +(extern constructor mem_flags_trusted mem_flags_trusted) + ;;;; Helper Clif Extractors ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; An extractor that only matches types that can fit in 16 bits. @@ -890,3 +901,4 @@ (convert Value InstOutput output_value) (convert Offset32 u32 offset32_to_u32) (convert ExternalName BoxExternalName box_external_name) +(convert PReg Reg preg_to_reg) diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index b9bc0c2ee4e7..49755a6ae380 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -721,6 +721,25 @@ impl<'a> Verifier<'a> { )); } } + NullAry { + opcode: Opcode::GetFramePointer, + } => { + if let Some(isa) = &self.isa { + if !isa.flags().preserve_frame_pointers() { + return errors.fatal(( + inst, + self.context(inst), + "`get_frame_pointer` cannot be used with enabling `preserve_frame_pointers`" + )); + } + } else { + return errors.fatal(( + inst, + self.context(inst), + "`get_frame_pointer` requires an ISA!", + )); + } + } Unary { opcode: Opcode::Bitcast, arg, diff --git a/cranelift/filetests/filetests/isa/aarch64/fp_sp_pc.clif b/cranelift/filetests/filetests/isa/aarch64/fp_sp_pc.clif new file mode 100644 index 000000000000..5d1ecd3d352f --- /dev/null +++ b/cranelift/filetests/filetests/isa/aarch64/fp_sp_pc.clif @@ -0,0 +1,43 @@ +test compile precise-output +set preserve_frame_pointers=true +target aarch64 + +function %fp() -> i64 { +block0: + v0 = get_frame_pointer.i64 + return v0 +} + +; stp fp, lr, [sp, #-16]! +; mov fp, sp +; block0: +; mov x0, fp +; ldp fp, lr, [sp], #16 +; ret + +function %sp() -> i64 { +block0: + v0 = get_stack_pointer.i64 + return v0 +} + +; stp fp, lr, [sp, #-16]! +; mov fp, sp +; block0: +; mov x0, sp +; ldp fp, lr, [sp], #16 +; ret + +function %return_address() -> i64 { +block0: + v0 = get_return_address.i64 + return v0 +} + +; stp fp, lr, [sp, #-16]! +; mov fp, sp +; block0: +; mov x0, lr +; ldp fp, lr, [sp], #16 +; ret + diff --git a/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif b/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif new file mode 100644 index 000000000000..31bf754f2c0e --- /dev/null +++ b/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif @@ -0,0 +1,52 @@ +test compile precise-output +set preserve_frame_pointers=true +target s390x + +function %fp() -> i64 { +block0: + v0 = get_frame_pointer.i64 + return v0 +} + +; stmg %r14, %r15, 112(%r15) +; lgr %r1, %r15 +; aghi %r15, -160 +; virtual_sp_offset_adjust 160 +; stg %r1, 0(%r15) +; block0: +; lg %r2, 0(%r15) +; lmg %r14, %r15, 272(%r15) +; br %r14 + +function %sp() -> i64 { +block0: + v0 = get_stack_pointer.i64 + return v0 +} + +; stmg %r14, %r15, 112(%r15) +; lgr %r1, %r15 +; aghi %r15, -160 +; virtual_sp_offset_adjust 160 +; stg %r1, 0(%r15) +; block0: +; lgr %r2, %r15 +; lmg %r14, %r15, 272(%r15) +; br %r14 + +function %return_address() -> i64 { +block0: + v0 = get_return_address.i64 + return v0 +} + +; stmg %r14, %r15, 112(%r15) +; lgr %r1, %r15 +; aghi %r15, -160 +; virtual_sp_offset_adjust 160 +; stg %r1, 0(%r15) +; block0: +; lgr %r2, %r14 +; lmg %r14, %r15, 272(%r15) +; br %r14 + diff --git a/cranelift/filetests/filetests/isa/x64/fp_sp_pc.clif b/cranelift/filetests/filetests/isa/x64/fp_sp_pc.clif new file mode 100644 index 000000000000..9a9990ddb20f --- /dev/null +++ b/cranelift/filetests/filetests/isa/x64/fp_sp_pc.clif @@ -0,0 +1,45 @@ +test compile precise-output +set preserve_frame_pointers=true +target x86_64 + +function %fp() -> i64 { +block0: + v0 = get_frame_pointer.i64 + return v0 +} + +; pushq %rbp +; movq %rsp, %rbp +; block0: +; movq %rbp, %rax +; movq %rbp, %rsp +; popq %rbp +; ret + +function %sp() -> i64 { +block0: + v0 = get_stack_pointer.i64 + return v0 +} + +; pushq %rbp +; movq %rsp, %rbp +; block0: +; movq %rsp, %rax +; movq %rbp, %rsp +; popq %rbp +; ret + +function %return_address() -> i64 { +block0: + v0 = get_return_address.i64 + return v0 +} + +; pushq %rbp +; movq %rsp, %rbp +; block0: +; movq 8(%rbp), %rax +; movq %rbp, %rsp +; popq %rbp +; ret diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 38ebcae11cbf..f966398fe8e6 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -1019,6 +1019,9 @@ where Opcode::ExtractVector => { unimplemented!("ExtractVector not supported"); } + Opcode::GetFramePointer => unimplemented!("GetFramePointer"), + Opcode::GetStackPointer => unimplemented!("GetStackPointer"), + Opcode::GetReturnAddress => unimplemented!("GetReturnAddress"), }) } From e866c5f8cbe79dce9b52e16cf38163feb6f98a59 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 2 Aug 2022 09:42:00 -0700 Subject: [PATCH 2/7] x64: Remove `Amode::RbpOffset` and use `Amode::ImmReg` instead We just special case getting operands from `Amode`s now. --- cranelift/codegen/src/isa/x64/encoding/rex.rs | 15 +----------- cranelift/codegen/src/isa/x64/inst.isle | 6 +---- cranelift/codegen/src/isa/x64/inst/args.rs | 23 +++++++------------ cranelift/codegen/src/isa/x64/lower.isle | 2 +- 4 files changed, 11 insertions(+), 35 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/encoding/rex.rs b/cranelift/codegen/src/isa/x64/encoding/rex.rs index 598a09de4367..862caeb63e93 100644 --- a/cranelift/codegen/src/isa/x64/encoding/rex.rs +++ b/cranelift/codegen/src/isa/x64/encoding/rex.rs @@ -298,20 +298,7 @@ pub(crate) fn emit_std_enc_mem( prefixes.emit(sink); - let mem_e = match mem_e.clone() { - Amode::RbpOffset { simm32, flags } => { - let base = regs::rbp(); - Amode::ImmReg { - simm32, - base, - flags, - } - } - other => other, - }; - - match mem_e { - Amode::RbpOffset { .. } => unreachable!(), + match *mem_e { Amode::ImmReg { simm32, base, .. } => { // If this is an access based off of RSP, it may trap with a stack overflow if it's the // first touch of a new stack page. diff --git a/cranelift/codegen/src/isa/x64/inst.isle b/cranelift/codegen/src/isa/x64/inst.isle index f5e5c0c94eb2..b632d3897498 100644 --- a/cranelift/codegen/src/isa/x64/inst.isle +++ b/cranelift/codegen/src/isa/x64/inst.isle @@ -800,11 +800,7 @@ ;; pointer). The appropriate relocation is emitted so ;; that the resulting immediate makes this Amode refer to ;; the given MachLabel. - (RipRelative (target MachLabel)) - - ;; Load `rbp + sign_extend(simm32)`. - (RbpOffset (simm32 u32) - (flags MemFlags)))) + (RipRelative (target MachLabel)))) ;; Some Amode constructor helpers. diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index fa9f2fdf08e1..c1bd4526bf4f 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -313,14 +313,16 @@ impl Amode { ) { match self { Amode::ImmReg { base, .. } => { - collector.reg_use(*base); + if *base != regs::rbp() && *base != regs::rsp() { + collector.reg_use(*base); + } } Amode::ImmRegRegShift { base, index, .. } => { collector.reg_use(base.to_reg()); collector.reg_use(index.to_reg()); } - Amode::RipRelative { .. } | Amode::RbpOffset { .. } => { - // RIP and RBP aren't involved in regalloc. + Amode::RipRelative { .. } => { + // RIP isn't involved in regalloc. } } } @@ -338,17 +340,15 @@ impl Amode { collector.reg_late_use(base.to_reg()); collector.reg_late_use(index.to_reg()); } - Amode::RipRelative { .. } | Amode::RbpOffset { .. } => { - // RIP and RBP aren't involved in regalloc. + Amode::RipRelative { .. } => { + // RIP isn't involved in regalloc. } } } pub(crate) fn get_flags(&self) -> MemFlags { match self { - Amode::RbpOffset { flags, .. } - | Amode::ImmReg { flags, .. } - | Amode::ImmRegRegShift { flags, .. } => *flags, + Amode::ImmReg { flags, .. } | Amode::ImmRegRegShift { flags, .. } => *flags, Amode::RipRelative { .. } => MemFlags::trusted(), } } @@ -384,7 +384,6 @@ impl Amode { index: Gpr::new(allocs.next(*index)).unwrap(), }, &Amode::RipRelative { target } => Amode::RipRelative { target }, - &Amode::RbpOffset { simm32, flags } => Amode::RbpOffset { simm32, flags }, } } @@ -403,12 +402,6 @@ impl Amode { impl PrettyPrint for Amode { fn pretty_print(&self, _size: u8, allocs: &mut AllocationConsumer<'_>) -> String { match self { - Amode::RbpOffset { simm32, flags } => Amode::ImmReg { - simm32: *simm32, - base: regs::rbp(), - flags: *flags, - } - .pretty_print(8, allocs), Amode::ImmReg { simm32, base, .. } => { // Note: size is always 8; the address is 64 bits, // even if the addressed operand is smaller. diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index 199fe8d483ad..dcea073743c4 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -2853,5 +2853,5 @@ (rule (lower (get_return_address)) (x64_load $I64 - (Amode.RbpOffset 8 (mem_flags_trusted)) + (Amode.ImmReg 8 (preg_rbp) (mem_flags_trusted)) (ExtKind.None))) From e2273282298b13008451337b3896f5680fcd1261 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 2 Aug 2022 10:06:28 -0700 Subject: [PATCH 3/7] Fix s390x `get_return_address`; require `preserve_frame_pointers=true` --- cranelift/codegen/meta/src/shared/instructions.rs | 2 ++ cranelift/codegen/src/isa/s390x/inst.isle | 12 +++++------- cranelift/codegen/src/isa/s390x/lower.isle | 4 +++- cranelift/codegen/src/isa/s390x/lower/isle.rs | 14 +++++++------- cranelift/codegen/src/verifier/mod.rs | 7 ++++--- .../filetests/filetests/isa/s390x/fp_sp_pc.clif | 2 +- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index 93d64dae3f8f..8e01d8e34d6b 100644 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -1343,6 +1343,8 @@ pub(crate) fn define( "get_return_address", r#" Get the PC where this function will transfer control to when it returns. + + Usage of this instruction requires setting `preserve_frame_pointers` to `true`. "#, &formats.nullary, ) diff --git a/cranelift/codegen/src/isa/s390x/inst.isle b/cranelift/codegen/src/isa/s390x/inst.isle index 86f8fa8ff64e..34d4792a2e82 100644 --- a/cranelift/codegen/src/isa/s390x/inst.isle +++ b/cranelift/codegen/src/isa/s390x/inst.isle @@ -1565,6 +1565,10 @@ (decl memarg_stack_off (i64 i64) MemArg) (extern constructor memarg_stack_off memarg_stack_off) +;; Create a `MemArg` referring to an offset from the initial SP. +(decl memarg_initial_sp_offset (i64) MemArg) +(extern constructor memarg_initial_sp_offset memarg_initial_sp_offset) + ;; Form the sum of two offset values, and check that the result is ;; a valid `MemArg::Symbol` offset (i.e. is even and fits into i32). (decl pure memarg_symbol_offset_sum (i64 i64) i32) @@ -2485,16 +2489,10 @@ (_ Unit (emit (MInst.MovPReg dst src)))) dst)) -(decl preg_r14 () PReg) -(extern constructor preg_r14 preg_r14) - (decl preg_r15 () PReg) (extern constructor preg_r15 preg_r15) -(decl r14 () Reg) -(rule (r14) - (mov_preg (preg_r14))) - +;; Copy `r15`, the physical stack register, into a virtual register. (decl r15 () Reg) (rule (r15) (mov_preg (preg_r15))) diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index 227602817d6a..f476b5f22ffc 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -3628,4 +3628,6 @@ (load64 (memarg_stack_off 0 0))) (rule (lower (get_return_address)) - (r14)) + ;; The return address is 14 pointer-sized slots above the SP. So our + ;; offset is `14 * 8 = 112`. + (load64 (memarg_initial_sp_offset 112))) diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs index cebbde4f4d7e..67d90f657fe3 100644 --- a/cranelift/codegen/src/isa/s390x/lower/isle.rs +++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs @@ -6,8 +6,8 @@ pub mod generated_code; // Types that the generated ISLE code uses via `use super::*`. use crate::isa::s390x::abi::S390xMachineDeps; use crate::isa::s390x::inst::{ - regs, stack_reg, writable_gpr, zero_reg, CallIndInfo, CallInfo, Cond, Inst as MInst, MemArg, - UImm12, UImm16Shifted, UImm32Shifted, + stack_reg, writable_gpr, zero_reg, CallIndInfo, CallInfo, Cond, Inst as MInst, MemArg, UImm12, + UImm16Shifted, UImm32Shifted, }; use crate::isa::s390x::settings::Flags as IsaFlags; use crate::machinst::isle::*; @@ -604,6 +604,11 @@ where MemArg::reg_plus_off(stack_reg(), base + off, MemFlags::trusted()) } + #[inline] + fn memarg_initial_sp_offset(&mut self, off: i64) -> MemArg { + MemArg::InitialSPOffset { off } + } + #[inline] fn memarg_symbol(&mut self, name: ExternalName, offset: i32, flags: MemFlags) -> MemArg { MemArg::Symbol { @@ -672,11 +677,6 @@ where self.lower_ctx.emit(inst.clone()); } - #[inline] - fn preg_r14(&mut self) -> PReg { - regs::gpr(14).to_real_reg().unwrap().into() - } - #[inline] fn preg_r15(&mut self) -> PReg { stack_reg().to_real_reg().unwrap().into() diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 49755a6ae380..34eed7f51a14 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -722,21 +722,22 @@ impl<'a> Verifier<'a> { } } NullAry { - opcode: Opcode::GetFramePointer, + opcode: Opcode::GetFramePointer | Opcode::GetReturnAddress, } => { if let Some(isa) = &self.isa { if !isa.flags().preserve_frame_pointers() { return errors.fatal(( inst, self.context(inst), - "`get_frame_pointer` cannot be used with enabling `preserve_frame_pointers`" + "`get_frame_pointer`/`get_return_address` cannot be used without \ + enabling `preserve_frame_pointers`", )); } } else { return errors.fatal(( inst, self.context(inst), - "`get_frame_pointer` requires an ISA!", + "`get_frame_pointer`/`get_return_address` require an ISA!", )); } } diff --git a/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif b/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif index 31bf754f2c0e..0de44e6a13c5 100644 --- a/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif +++ b/cranelift/filetests/filetests/isa/s390x/fp_sp_pc.clif @@ -46,7 +46,7 @@ block0: ; virtual_sp_offset_adjust 160 ; stg %r1, 0(%r15) ; block0: -; lgr %r2, %r14 +; lg %r2, 272(%r15) ; lmg %r14, %r15, 272(%r15) ; br %r14 From 45bf4cb8d34086e5a5e8dd5c9d6f9a5dff7f1968 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 2 Aug 2022 13:05:13 -0700 Subject: [PATCH 4/7] Assert that `Amode::ImmRegRegShift` doesn't use rbp/rsp --- cranelift/codegen/src/isa/x64/inst/args.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index c1bd4526bf4f..5750cc0ee72c 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -318,7 +318,11 @@ impl Amode { } } Amode::ImmRegRegShift { base, index, .. } => { + debug_assert_ne!(base.to_reg(), regs::rbp()); + debug_assert_ne!(base.to_reg(), regs::rsp()); collector.reg_use(base.to_reg()); + debug_assert_ne!(index.to_reg(), regs::rbp()); + debug_assert_ne!(index.to_reg(), regs::rsp()); collector.reg_use(index.to_reg()); } Amode::RipRelative { .. } => { From f10c6fd6082e43f244f450a63f2e11589b6bdd0b Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 2 Aug 2022 13:05:36 -0700 Subject: [PATCH 5/7] Handle non-allocatable registers in Amode::with_allocs --- cranelift/codegen/src/isa/x64/inst/args.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index 5750cc0ee72c..d42398282de7 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -369,11 +369,18 @@ impl Amode { simm32, base, flags, - } => Amode::ImmReg { - simm32, - flags, - base: allocs.next(base), - }, + } => { + let base = if base == regs::rsp() || base == regs::rbp() { + base + } else { + allocs.next(base) + }; + Amode::ImmReg { + simm32, + flags, + base, + } + } &Amode::ImmRegRegShift { simm32, base, From ad01c28b834dfa8527be17f234918ac460ba0470 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 2 Aug 2022 13:12:10 -0700 Subject: [PATCH 6/7] Use "stack" instead of "r15" on s390x --- cranelift/codegen/src/isa/s390x/inst.isle | 12 ++++++------ cranelift/codegen/src/isa/s390x/lower.isle | 6 +++--- cranelift/codegen/src/isa/s390x/lower/isle.rs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cranelift/codegen/src/isa/s390x/inst.isle b/cranelift/codegen/src/isa/s390x/inst.isle index 34d4792a2e82..585c5b1584e1 100644 --- a/cranelift/codegen/src/isa/s390x/inst.isle +++ b/cranelift/codegen/src/isa/s390x/inst.isle @@ -2489,13 +2489,13 @@ (_ Unit (emit (MInst.MovPReg dst src)))) dst)) -(decl preg_r15 () PReg) -(extern constructor preg_r15 preg_r15) +(decl preg_stack () PReg) +(extern constructor preg_stack preg_stack) -;; Copy `r15`, the physical stack register, into a virtual register. -(decl r15 () Reg) -(rule (r15) - (mov_preg (preg_r15))) +;; Copy the physical stack register into a virtual register. +(decl sp () Reg) +(rule (sp) + (mov_preg (preg_stack))) ;; Helpers for accessing argument / return value slots ;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index f476b5f22ffc..31451f103a57 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -3622,12 +3622,12 @@ ;;;; Rules for `get_{frame,stack}_pointer` and `get_return_address` ;;;;;;;;;;;; (rule (lower (get_stack_pointer)) - (r15)) + (sp)) (rule (lower (get_frame_pointer)) (load64 (memarg_stack_off 0 0))) (rule (lower (get_return_address)) - ;; The return address is 14 pointer-sized slots above the SP. So our - ;; offset is `14 * 8 = 112`. + ;; The return address is 14 pointer-sized slots above the initial SP. So + ;; our offset is `14 * 8 = 112`. (load64 (memarg_initial_sp_offset 112))) diff --git a/cranelift/codegen/src/isa/s390x/lower/isle.rs b/cranelift/codegen/src/isa/s390x/lower/isle.rs index 67d90f657fe3..5d6a02ca0b3f 100644 --- a/cranelift/codegen/src/isa/s390x/lower/isle.rs +++ b/cranelift/codegen/src/isa/s390x/lower/isle.rs @@ -678,7 +678,7 @@ where } #[inline] - fn preg_r15(&mut self) -> PReg { + fn preg_stack(&mut self) -> PReg { stack_reg().to_real_reg().unwrap().into() } } From 3ba1e8fd613efed2220a88697bd49984b1c0c674 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 2 Aug 2022 13:12:46 -0700 Subject: [PATCH 7/7] r14 is an allocatable register on s390x, so it shouldn't be used with `MovPReg` --- cranelift/codegen/src/isa/s390x/inst/emit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cranelift/codegen/src/isa/s390x/inst/emit.rs b/cranelift/codegen/src/isa/s390x/inst/emit.rs index 08d9074e757e..bb85f9d576ff 100644 --- a/cranelift/codegen/src/isa/s390x/inst/emit.rs +++ b/cranelift/codegen/src/isa/s390x/inst/emit.rs @@ -2084,7 +2084,7 @@ impl MachInstEmit for Inst { } &Inst::MovPReg { rd, rm } => { let rm: Reg = rm.into(); - debug_assert!([regs::gpr(14), regs::gpr(15)].contains(&rm)); + debug_assert!([regs::gpr(15)].contains(&rm)); let rd = allocs.next_writable(rd); Inst::Mov64 { rd, rm }.emit(&[], sink, emit_info, state); }