Skip to content

Commit

Permalink
cranelift: Remove nominal-sp (#8643)
Browse files Browse the repository at this point in the history
* Update the frame layout comment

* Remove more references to nominal SP

* Remove the nominal_sp_offset from backend emit states

* Continue removing references to the nominal sp

* Remove nominal-sp from the aarch64 backend

* Remove nominal-sp from the s390x backend

* Remove nominal-sp from the riscv64 backend

* Remove old comment
  • Loading branch information
elliottt authored May 17, 2024
1 parent 4e62cfd commit e165106
Show file tree
Hide file tree
Showing 32 changed files with 295 additions and 362 deletions.
7 changes: 1 addition & 6 deletions cranelift/codegen/src/isa/aarch64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl Into<AMode> for StackAMode {
StackAMode::IncomingArg(off, stack_args_size) => AMode::IncomingArg {
off: i64::from(stack_args_size) - off,
},
StackAMode::Slot(off) => AMode::NominalSPOffset { off },
StackAMode::Slot(off) => AMode::SlotOffset { off },
StackAMode::OutgoingArg(off) => AMode::SPOffset { off },
}
}
Expand Down Expand Up @@ -1132,11 +1132,6 @@ impl ABIMachineSpec for AArch64MachineDeps {
}
}

/// Get the nominal-SP-to-FP offset from an instruction-emission state.
fn get_nominal_sp_to_fp(s: &EmitState) -> i64 {
s.nominal_sp_to_fp
}

fn get_machine_env(flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv {
if flags.enable_pinned_reg() {
static MACHINE_ENV: OnceLock<MachineEnv> = OnceLock::new();
Expand Down
12 changes: 5 additions & 7 deletions cranelift/codegen/src/isa/aarch64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -1178,19 +1178,17 @@
(IncomingArg
(off i64))

;; Offset from the "nominal stack pointer", which is where the real SP is
;; just after stack and spill slots are allocated in the function prologue.
;; Offset into the slot area of the stack, which lies just above the
;; outgoing argument area that's setup by the function prologue.
;; At emission time, this is converted to `SPOffset` with a fixup added to
;; the offset constant. The fixup is a running value that is tracked as
;; emission iterates through instructions in linear order, and can be
;; adjusted up and down with [Inst::VirtualSPOffsetAdj].
;;
;; The standard ABI is in charge of handling this (by emitting the
;; adjustment meta-instructions). It maintains the invariant that "nominal
;; SP" is where the actual SP is after the function prologue and before
;; clobber pushes. See the diagram in the documentation for
;; [crate::isa::aarch64::abi](the ABI module) for more details.
(NominalSPOffset
;; adjustment meta-instructions). See the diagram in the documentation
;; for [crate::isa::aarch64::abi](the ABI module) for more details.
(SlotOffset
(off i64))))

;; A memory argument to a load/store-pair.
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/aarch64/inst/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ impl PrettyPrint for AMode {
&AMode::SPOffset { .. }
| &AMode::FPOffset { .. }
| &AMode::IncomingArg { .. }
| &AMode::NominalSPOffset { .. }
| &AMode::SlotOffset { .. }
| &AMode::RegOffset { .. } => {
panic!("Unexpected pseudo mem-arg mode: {:?}", self)
}
Expand Down
15 changes: 6 additions & 9 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ pub fn mem_finalize(
| &AMode::SPOffset { off }
| &AMode::FPOffset { off }
| &AMode::IncomingArg { off }
| &AMode::NominalSPOffset { off } => {
| &AMode::SlotOffset { off } => {
let basereg = match mem {
&AMode::RegOffset { rn, .. } => rn,
&AMode::SPOffset { .. }
| &AMode::NominalSPOffset { .. }
| &AMode::SlotOffset { .. }
| &AMode::IncomingArg { .. } => stack_reg(),
&AMode::FPOffset { .. } => fp_reg(),
_ => unreachable!(),
Expand All @@ -42,10 +42,10 @@ pub fn mem_finalize(
+ frame_layout.outgoing_args_size,
) - off
}
&AMode::NominalSPOffset { .. } => {
&AMode::SlotOffset { .. } => {
let adj = i64::from(state.frame_layout().outgoing_args_size);
trace!(
"mem_finalize: nominal SP offset {} + adj {} -> {}",
"mem_finalize: slot offset {} + adj {} -> {}",
off,
adj,
off + adj
Expand Down Expand Up @@ -651,8 +651,6 @@ fn enc_asimd_mod_imm(rd: Writable<Reg>, q_op: u32, cmode: u32, imm: u8) -> u32 {
/// State carried between emissions of a sequence of instructions.
#[derive(Default, Clone, Debug)]
pub struct EmitState {
/// Offset of FP from nominal-SP.
pub(crate) nominal_sp_to_fp: i64,
/// Safepoint stack map for upcoming instruction, as provided to `pre_safepoint()`.
stack_map: Option<StackMap>,
/// Only used during fuzz-testing. Otherwise, it is a zero-sized struct and
Expand All @@ -664,7 +662,6 @@ pub struct EmitState {
impl MachInstEmitState<Inst> for EmitState {
fn new(abi: &Callee<AArch64MachineDeps>, ctrl_plane: ControlPlane) -> Self {
EmitState {
nominal_sp_to_fp: abi.frame_size() as i64,
stack_map: None,
ctrl_plane,
frame_layout: abi.frame_layout().clone(),
Expand Down Expand Up @@ -1076,7 +1073,7 @@ impl MachInstEmit for Inst {
&AMode::SPOffset { .. }
| &AMode::FPOffset { .. }
| &AMode::IncomingArg { .. }
| &AMode::NominalSPOffset { .. }
| &AMode::SlotOffset { .. }
| &AMode::Const { .. }
| &AMode::RegOffset { .. } => {
panic!("Should not see {:?} here!", mem)
Expand Down Expand Up @@ -1170,7 +1167,7 @@ impl MachInstEmit for Inst {
&AMode::SPOffset { .. }
| &AMode::FPOffset { .. }
| &AMode::IncomingArg { .. }
| &AMode::NominalSPOffset { .. }
| &AMode::SlotOffset { .. }
| &AMode::Const { .. }
| &AMode::RegOffset { .. } => {
panic!("Should not see {:?} here!", mem)
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/aarch64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ fn memarg_operands(memarg: &mut AMode, collector: &mut impl OperandVisitor) {
AMode::Label { .. } => {}
AMode::SPPreIndexed { .. } | AMode::SPPostIndexed { .. } => {}
AMode::FPOffset { .. } | AMode::IncomingArg { .. } => {}
AMode::SPOffset { .. } | AMode::NominalSPOffset { .. } => {}
AMode::SPOffset { .. } | AMode::SlotOffset { .. } => {}
AMode::RegOffset { rn, .. } => {
collector.reg_use(rn);
}
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/aarch64/pcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ fn check_addr<'a>(
&AMode::SPOffset { .. }
| &AMode::FPOffset { .. }
| &AMode::IncomingArg { .. }
| &AMode::NominalSPOffset { .. }
| &AMode::SlotOffset { .. }
| &AMode::SPPostIndexed { .. }
| &AMode::SPPreIndexed { .. } => {
// We trust ABI code (for now!) and no lowering rules
Expand Down
5 changes: 0 additions & 5 deletions cranelift/codegen/src/isa/riscv64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,11 +682,6 @@ impl ABIMachineSpec for Riscv64MachineDeps {
}
}

/// Get the nominal-SP-to-FP offset from an instruction-emission state.
fn get_nominal_sp_to_fp(s: &EmitState) -> i64 {
s.nominal_sp_to_fp
}

fn get_machine_env(_flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv {
static MACHINE_ENV: OnceLock<MachineEnv> = OnceLock::new();
MACHINE_ENV.get_or_init(create_reg_enviroment)
Expand Down
26 changes: 12 additions & 14 deletions cranelift/codegen/src/isa/riscv64/inst/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,17 @@ pub enum AMode {
/// Offset from the frame pointer.
FPOffset(i64),

/// Offset from the "nominal stack pointer", which is where the real SP is
/// just after stack and spill slots are allocated in the function prologue.
/// Offset into the slot area of the stack, which lies just above the
/// outgoing argument area that's setup by the function prologue.
/// At emission time, this is converted to `SPOffset` with a fixup added to
/// the offset constant. The fixup is a running value that is tracked as
/// emission iterates through instructions in linear order, and can be
/// adjusted up and down with [Inst::VirtualSPOffsetAdj].
///
/// The standard ABI is in charge of handling this (by emitting the
/// adjustment meta-instructions). It maintains the invariant that "nominal
/// SP" is where the actual SP is after the function prologue and before
/// clobber pushes. See the diagram in the documentation for
/// [crate::isa::riscv64::abi](the ABI module) for more details.
NominalSPOffset(i64),
/// adjustment meta-instructions). See the diagram in the documentation
/// for [crate::isa::aarch64::abi](the ABI module) for more details.
SlotOffset(i64),

/// Offset into the argument area.
IncomingArg(i64),
Expand All @@ -120,7 +118,7 @@ impl AMode {
// Registers used in these modes aren't allocatable.
AMode::SPOffset(..)
| AMode::FPOffset(..)
| AMode::NominalSPOffset(..)
| AMode::SlotOffset(..)
| AMode::IncomingArg(..)
| AMode::Const(..)
| AMode::Label(..) => {}
Expand All @@ -132,15 +130,15 @@ impl AMode {
&AMode::RegOffset(reg, ..) => Some(reg),
&AMode::SPOffset(..) => Some(stack_reg()),
&AMode::FPOffset(..) => Some(fp_reg()),
&AMode::NominalSPOffset(..) => Some(stack_reg()),
&AMode::SlotOffset(..) => Some(stack_reg()),
&AMode::IncomingArg(..) => Some(stack_reg()),
&AMode::Const(..) | AMode::Label(..) => None,
}
}

pub(crate) fn get_offset_with_state(&self, state: &EmitState) -> i64 {
match self {
&AMode::NominalSPOffset(offset) => {
&AMode::SlotOffset(offset) => {
offset + i64::from(state.frame_layout().outgoing_args_size)
}

Expand Down Expand Up @@ -171,7 +169,7 @@ impl AMode {
| &AMode::SPOffset(..)
| &AMode::FPOffset(..)
| &AMode::IncomingArg(..)
| &AMode::NominalSPOffset(..) => None,
| &AMode::SlotOffset(..) => None,
}
}
}
Expand All @@ -185,8 +183,8 @@ impl Display for AMode {
&AMode::SPOffset(offset, ..) => {
write!(f, "{}(sp)", offset)
}
&AMode::NominalSPOffset(offset, ..) => {
write!(f, "{}(nominal_sp)", offset)
&AMode::SlotOffset(offset, ..) => {
write!(f, "{}(slot)", offset)
}
&AMode::IncomingArg(offset) => {
write!(f, "-{}(incoming_arg)", offset)
Expand All @@ -211,7 +209,7 @@ impl Into<AMode> for StackAMode {
AMode::IncomingArg(i64::from(stack_args_size) - offset)
}
StackAMode::OutgoingArg(offset) => AMode::SPOffset(offset),
StackAMode::Slot(offset) => AMode::NominalSPOffset(offset),
StackAMode::Slot(offset) => AMode::SlotOffset(offset),
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ pub enum EmitVState {
/// State carried between emissions of a sequence of instructions.
#[derive(Default, Clone, Debug)]
pub struct EmitState {
pub(crate) nominal_sp_to_fp: i64,
/// Safepoint stack map for upcoming instruction, as provided to `pre_safepoint()`.
stack_map: Option<StackMap>,
/// Only used during fuzz-testing. Otherwise, it is a zero-sized struct and
Expand All @@ -70,7 +69,6 @@ impl MachInstEmitState<Inst> for EmitState {
ctrl_plane: ControlPlane,
) -> Self {
EmitState {
nominal_sp_to_fp: abi.frame_size() as i64,
stack_map: None,
ctrl_plane,
vstate: EmitVState::Unknown,
Expand Down
5 changes: 2 additions & 3 deletions cranelift/codegen/src/isa/riscv64/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,10 @@ impl generated_code::Context for RV64IsleContext<'_, '_, MInst, Riscv64Backend>
}

fn gen_stack_slot_amode(&mut self, ss: StackSlot, offset: i64) -> AMode {
// Offset from beginning of stackslot area, which is at nominal SP (see
// [MemArg::NominalSPOffset] for more details on nominal SP tracking).
// Offset from beginning of stackslot area.
let stack_off = self.lower_ctx.abi().sized_stackslot_offsets()[ss] as i64;
let sp_off: i64 = stack_off + offset;
AMode::NominalSPOffset(sp_off)
AMode::SlotOffset(sp_off)
}

fn gen_const_amode(&mut self, c: VCodeConstant) -> AMode {
Expand Down
13 changes: 4 additions & 9 deletions cranelift/codegen/src/isa/s390x/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@
//! +---------------------------+
//! | ... |
//! | spill slots |
//! | (accessed via nominal SP) |
//! | (accessed via SP) |
//! | ... |
//! | stack slots |
//! | (accessed via nominal SP) |
//! nominal SP ---------------> | (alloc'd by prologue) |
//! | (accessed via SP) |
//! | (alloc'd by prologue) |
//! +---------------------------+
//! | ... |
//! | args for call |
Expand Down Expand Up @@ -193,7 +193,7 @@ impl Into<MemArg> for StackAMode {
match self {
// Argument area always begins at the initial SP.
StackAMode::IncomingArg(off, _) => MemArg::InitialSPOffset { off },
StackAMode::Slot(off) => MemArg::NominalSPOffset { off },
StackAMode::Slot(off) => MemArg::SlotOffset { off },
StackAMode::OutgoingArg(off) => {
MemArg::reg_plus_off(stack_reg(), off, MemFlags::trusted())
}
Expand Down Expand Up @@ -765,11 +765,6 @@ impl ABIMachineSpec for S390xMachineDeps {
}
}

/// Get the nominal-SP-to-FP offset from an instruction-emission state.
fn get_nominal_sp_to_fp(s: &EmitState) -> i64 {
s.initial_sp_offset
}

fn get_machine_env(_flags: &settings::Flags, _call_conv: isa::CallConv) -> &MachineEnv {
static MACHINE_ENV: OnceLock<MachineEnv> = OnceLock::new();
MACHINE_ENV.get_or_init(create_machine_env)
Expand Down
16 changes: 7 additions & 9 deletions cranelift/codegen/src/isa/s390x/inst/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,17 @@ pub enum MemArg {
/// Offset from the stack pointer at function entry.
InitialSPOffset { off: i64 },

/// Offset from the "nominal stack pointer", which is where the real SP is
/// just after stack and spill slots are allocated in the function prologue.
/// Offset into the slot area of the stack, which lies just above the
/// outgoing argument area that's setup by the function prologue.
/// At emission time, this is converted to `SPOffset` with a fixup added to
/// the offset constant. The fixup is a running value that is tracked as
/// emission iterates through instructions in linear order, and can be
/// adjusted up and down with [Inst::VirtualSPOffsetAdj].
///
/// The standard ABI is in charge of handling this (by emitting the
/// adjustment meta-instructions). It maintains the invariant that "nominal
/// SP" is where the actual SP is after the function prologue and before
/// clobber pushes. See the diagram in the documentation for
/// [crate::isa::s390x::abi](the ABI module) for more details.
NominalSPOffset { off: i64 },
/// adjustment meta-instructions). See the diagram in the documentation
/// for [crate::isa::aarch64::abi](the ABI module) for more details.
SlotOffset { off: i64 },
}

impl MemArg {
Expand Down Expand Up @@ -98,7 +96,7 @@ impl MemArg {
MemArg::Label { .. } => MemFlags::trusted(),
MemArg::Symbol { flags, .. } => *flags,
MemArg::InitialSPOffset { .. } => MemFlags::trusted(),
MemArg::NominalSPOffset { .. } => MemFlags::trusted(),
MemArg::SlotOffset { .. } => MemFlags::trusted(),
}
}
}
Expand Down Expand Up @@ -268,7 +266,7 @@ impl PrettyPrint for MemArg {
} => format!("{} + {}", name.display(None), offset),
// Eliminated by `mem_finalize()`.
&MemArg::InitialSPOffset { .. }
| &MemArg::NominalSPOffset { .. }
| &MemArg::SlotOffset { .. }
| &MemArg::RegOffset { .. } => {
panic!("Unexpected pseudo mem-arg mode (stack-offset or generic reg-offset)!")
}
Expand Down
8 changes: 3 additions & 5 deletions cranelift/codegen/src/isa/s390x/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,17 @@ pub fn mem_finalize(
let mem = match mem {
&MemArg::RegOffset { off, .. }
| &MemArg::InitialSPOffset { off }
| &MemArg::NominalSPOffset { off } => {
| &MemArg::SlotOffset { off } => {
let base = match mem {
&MemArg::RegOffset { reg, .. } => reg,
&MemArg::InitialSPOffset { .. } | &MemArg::NominalSPOffset { .. } => stack_reg(),
&MemArg::InitialSPOffset { .. } | &MemArg::SlotOffset { .. } => stack_reg(),
_ => unreachable!(),
};
let adj = match mem {
&MemArg::InitialSPOffset { .. } => {
state.initial_sp_offset + i64::from(state.frame_layout().outgoing_args_size)
}
&MemArg::NominalSPOffset { .. } => {
i64::from(state.frame_layout().outgoing_args_size)
}
&MemArg::SlotOffset { .. } => i64::from(state.frame_layout().outgoing_args_size),
_ => 0,
};
let off = off + adj;
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/s390x/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ fn memarg_operands(memarg: &mut MemArg, collector: &mut impl OperandVisitor) {
MemArg::RegOffset { reg, .. } => {
collector.reg_use(reg);
}
MemArg::InitialSPOffset { .. } | MemArg::NominalSPOffset { .. } => {}
MemArg::InitialSPOffset { .. } | MemArg::SlotOffset { .. } => {}
}
// mem_finalize might require %r1 to hold (part of) the address.
// Conservatively assume this will always be necessary here.
Expand Down
Loading

0 comments on commit e165106

Please sign in to comment.