Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cranelift: Remove nominal-sp #8643

Merged
merged 8 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading