Skip to content

cranelift: Simplify StackAMode variants #8316

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.

Sign up for GitHub

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

Merged
jameysharp merged 1 commit into from
Apr 9, 2024
Merged

cranelift: Simplify StackAMode variants #8316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions cranelift/codegen/src/isa/aarch64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ impl Into<AMode> for StackAMode {
fn into(self) -> AMode {
match self {
// Argument area begins after saved frame pointer + return address.
StackAMode::ArgOffset(off, _ty) => AMode::FPOffset { off: off + 16 },
StackAMode::NominalSPOffset(off, _ty) => AMode::NominalSPOffset { off },
StackAMode::SPOffset(off, _ty) => AMode::SPOffset { off },
StackAMode::IncomingArg(off) => AMode::FPOffset { off: off + 16 },
StackAMode::Slot(off) => AMode::NominalSPOffset { off },
StackAMode::OutgoingArg(off) => AMode::SPOffset { off },
}
}
}
Expand Down Expand Up @@ -457,7 +457,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
insts
}

fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>, _ty: Type) -> Inst {
fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>) -> Inst {
// FIXME: Do something different for dynamic types?
let mem = mem.into();
Inst::LoadAddr { rd: into_reg, mem }
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/isa/riscv64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
insts
}

fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>, _ty: Type) -> Inst {
fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>) -> Inst {
Inst::LoadAddr {
rd: into_reg,
mem: mem.into(),
Expand Down
6 changes: 3 additions & 3 deletions cranelift/codegen/src/isa/riscv64/inst/args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,9 @@ impl Into<AMode> for StackAMode {
fn into(self) -> AMode {
match self {
// Argument area begins after saved lr + fp.
StackAMode::ArgOffset(offset, _ty) => AMode::FPOffset(offset + 16),
StackAMode::SPOffset(offset, _ty) => AMode::SPOffset(offset),
StackAMode::NominalSPOffset(offset, _ty) => AMode::NominalSPOffset(offset),
StackAMode::IncomingArg(offset) => AMode::FPOffset(offset + 16),
StackAMode::OutgoingArg(offset) => AMode::SPOffset(offset),
StackAMode::Slot(offset) => AMode::NominalSPOffset(offset),
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions cranelift/codegen/src/isa/s390x/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ impl Into<MemArg> for StackAMode {
fn into(self) -> MemArg {
match self {
// Argument area always begins at the initial SP.
StackAMode::ArgOffset(off, _ty) => MemArg::InitialSPOffset { off },
StackAMode::NominalSPOffset(off, _ty) => MemArg::NominalSPOffset { off },
StackAMode::SPOffset(off, _ty) => {
StackAMode::IncomingArg(off) => MemArg::InitialSPOffset { off },
StackAMode::Slot(off) => MemArg::NominalSPOffset { off },
StackAMode::OutgoingArg(off) => {
MemArg::reg_plus_off(stack_reg(), off, MemFlags::trusted())
}
}
Expand Down Expand Up @@ -495,7 +495,7 @@ impl ABIMachineSpec for S390xMachineDeps {
insts
}

fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>, _ty: Type) -> Inst {
fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>) -> Inst {
let mem = mem.into();
Inst::LoadAddr { rd: into_reg, mem }
}
Expand Down
27 changes: 13 additions & 14 deletions cranelift/codegen/src/isa/x64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
]
}

fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>, _ty: Type) -> Self::I {
fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>) -> Self::I {
let mem: SyntheticAmode = mem.into();
Inst::lea(mem, into_reg)
}
Expand Down Expand Up @@ -1018,27 +1018,26 @@ impl From<StackAMode> for SyntheticAmode {
// We enforce a 128 MB stack-frame size limit above, so these
// `expect()`s should never fail.
match amode {
StackAMode::ArgOffset(off, _ty) => {
let off =
i32::try_from(off + 16) // frame pointer + return address
.expect(
"Offset in ArgOffset is greater than 2GB; should hit impl limit first",
);
StackAMode::IncomingArg(off) => {
let off = i32::try_from(off + 16) // frame pointer + return address
.expect(
"Offset in IncomingArg is greater than 2GB; should hit impl limit first",
);
SyntheticAmode::Real(Amode::ImmReg {
simm32: off,
base: regs::rbp(),
flags: MemFlags::trusted(),
})
}
StackAMode::NominalSPOffset(off, _ty) => {
let off = i32::try_from(off).expect(
"Offset in NominalSPOffset is greater than 2GB; should hit impl limit first",
);
StackAMode::Slot(off) => {
let off = i32::try_from(off)
.expect("Offset in Slot is greater than 2GB; should hit impl limit first");
SyntheticAmode::nominal_sp_offset(off)
}
StackAMode::SPOffset(off, _ty) => {
let off = i32::try_from(off)
.expect("Offset in SPOffset is greater than 2GB; should hit impl limit first");
StackAMode::OutgoingArg(off) => {
let off = i32::try_from(off).expect(
"Offset in OutgoingArg is greater than 2GB; should hit impl limit first",
);
SyntheticAmode::Real(Amode::ImmReg {
simm32: off,
base: regs::rsp(),
Expand Down
52 changes: 21 additions & 31 deletions cranelift/codegen/src/machinst/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,12 @@ pub enum ArgsOrRets {
/// appropriate addressing mode.
#[derive(Clone, Copy, Debug)]
pub enum StackAMode {
/// Offset into the current frame's argument area, possibly making use of a
/// specific type for a scaled indexing operation.
ArgOffset(i64, ir::Type),
/// Offset from the nominal stack pointer, possibly making use of a specific
/// type for a scaled indexing operation.
NominalSPOffset(i64, ir::Type),
/// Offset from the real stack pointer, possibly making use of a specific
/// type for a scaled indexing operation.
SPOffset(i64, ir::Type),
/// Offset into the current frame's argument area.
IncomingArg(i64),
/// Offset within the stack slots in the current frame.
Slot(i64),
/// Offset into the callee frame's argument area.
OutgoingArg(i64),
}

/// Trait implemented by machine-specific backend to represent ISA flags.
Expand Down Expand Up @@ -447,7 +444,7 @@ pub trait ABIMachineSpec {

/// Generate an instruction to compute an address of a stack slot (FP- or
/// SP-based offset).
fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>, ty: Type) -> Self::I;
fn gen_get_stack_addr(mem: StackAMode, into_reg: Writable<Reg>) -> Self::I;

/// Get a fixed register to use to compute a stack limit. This is needed for
/// certain sequences generated after the register allocator has already
Expand Down Expand Up @@ -1475,7 +1472,7 @@ impl<M: ABIMachineSpec> Callee<M> {
ty
};
insts.push(M::gen_load_stack(
StackAMode::ArgOffset(offset, ty),
StackAMode::IncomingArg(offset),
*into_reg,
ty,
));
Expand All @@ -1500,9 +1497,8 @@ impl<M: ABIMachineSpec> Callee<M> {
} else {
// Buffer address is implicitly defined by the ABI.
insts.push(M::gen_get_stack_addr(
StackAMode::ArgOffset(offset, I8),
StackAMode::IncomingArg(offset),
into_reg,
I8,
));
}
}
Expand All @@ -1523,7 +1519,7 @@ impl<M: ABIMachineSpec> Callee<M> {
// This was allocated in the `init` routine.
let addr_reg = self.arg_temp_reg[idx].unwrap();
insts.push(M::gen_load_stack(
StackAMode::ArgOffset(offset, ty),
StackAMode::IncomingArg(offset),
addr_reg,
ty,
));
Expand Down Expand Up @@ -1694,17 +1690,13 @@ impl<M: ABIMachineSpec> Callee<M> {
// [MemArg::NominalSPOffset] for more details on nominal SP tracking).
let stack_off = self.sized_stackslots[slot] as i64;
let sp_off: i64 = stack_off + (offset as i64);
M::gen_get_stack_addr(StackAMode::NominalSPOffset(sp_off, I8), into_reg, I8)
M::gen_get_stack_addr(StackAMode::Slot(sp_off), into_reg)
}

/// Produce an instruction that computes a dynamic stackslot address.
pub fn dynamic_stackslot_addr(&self, slot: DynamicStackSlot, into_reg: Writable<Reg>) -> M::I {
let stack_off = self.dynamic_stackslots[slot] as i64;
M::gen_get_stack_addr(
StackAMode::NominalSPOffset(stack_off, I64X2XN),
into_reg,
I64X2XN,
)
M::gen_get_stack_addr(StackAMode::Slot(stack_off), into_reg)
}

/// Get an `args` pseudo-inst, if any, that should appear at the
Expand Down Expand Up @@ -1977,7 +1969,7 @@ impl<M: ABIMachineSpec> Callee<M> {
let sp_off = self.get_spillslot_offset(to_slot);
trace!("gen_spill: {from_reg:?} into slot {to_slot:?} at offset {sp_off}");

let from = StackAMode::NominalSPOffset(sp_off, ty);
let from = StackAMode::Slot(sp_off);
<M>::gen_store_stack(from, Reg::from(from_reg), ty)
}

Expand All @@ -1989,7 +1981,7 @@ impl<M: ABIMachineSpec> Callee<M> {
let sp_off = self.get_spillslot_offset(from_slot);
trace!("gen_reload: {to_reg:?} from slot {from_slot:?} at offset {sp_off}");

let from = StackAMode::NominalSPOffset(sp_off, ty);
let from = StackAMode::Slot(sp_off);
<M>::gen_load_stack(from, to_reg.map(Reg::from), ty)
}
}
Expand Down Expand Up @@ -2242,9 +2234,8 @@ impl<M: ABIMachineSpec> CallSite<M> {
let src_ptr = from_regs.only_reg().unwrap();
let dst_ptr = ctx.alloc_tmp(M::word_type()).only_reg().unwrap();
ctx.emit(M::gen_get_stack_addr(
StackAMode::SPOffset(offset, I8),
StackAMode::OutgoingArg(offset),
dst_ptr,
I8,
));
// Emit a memcpy from `src_ptr` to `dst_ptr` of `size` bytes.
// N.B.: because we process StructArg params *first*, this is
Expand Down Expand Up @@ -2283,9 +2274,9 @@ impl<M: ABIMachineSpec> CallSite<M> {
"tail calls require frame pointers to be enabled"
);

StackAMode::ArgOffset(offset, ty)
StackAMode::IncomingArg(offset)
} else {
StackAMode::SPOffset(offset, ty)
StackAMode::OutgoingArg(offset)
};
ctx.emit(M::gen_store_stack(amode, vreg, ty))
}
Expand Down Expand Up @@ -2391,9 +2382,9 @@ impl<M: ABIMachineSpec> CallSite<M> {
} => {
assert_eq!(from_regs.len(), 1);
let vreg = from_regs.regs()[0];
let amode = StackAMode::SPOffset(offset, ty);
let amode = StackAMode::OutgoingArg(offset);
let tmp = ctx.alloc_tmp(M::word_type()).only_reg().unwrap();
ctx.emit(M::gen_get_stack_addr(amode, tmp, ty));
ctx.emit(M::gen_get_stack_addr(amode, tmp));
let tmp = tmp.to_reg();
ctx.emit(M::gen_store_base_offset(tmp, 0, vreg, ty));
let loc = match pointer {
Expand Down Expand Up @@ -2504,7 +2495,7 @@ impl<M: ABIMachineSpec> CallSite<M> {
sig_data.sized_stack_arg_space()
};
insts.push(M::gen_load_stack(
StackAMode::SPOffset(offset + ret_area_base, ty),
StackAMode::OutgoingArg(offset + ret_area_base),
*into_reg,
ty,
));
Expand Down Expand Up @@ -2541,9 +2532,8 @@ impl<M: ABIMachineSpec> CallSite<M> {
let rd = ctx.alloc_tmp(word_type).only_reg().unwrap();
let ret_area_base = ctx.sigs()[self.sig].sized_stack_arg_space();
ctx.emit(M::gen_get_stack_addr(
StackAMode::SPOffset(ret_area_base, I8),
StackAMode::OutgoingArg(ret_area_base),
rd,
I8,
));
let moves = self.gen_arg(ctx, i.into(), ValueRegs::one(rd.to_reg()));
self.emit_arg_moves(ctx, moves);
Expand Down