Skip to content

Cranelift: Adjust virtual SP after tail call-conv callees return #6586

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
fitzgen merged 2 commits into from
Jun 16, 2023
Merged

Cranelift: Adjust virtual SP after tail call-conv callees return #6586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a10b14
Cranelift: Adjust virtual SP after `tail` call-conv callees return
fitzgen Jun 15, 2023
7454676
Cranelift: Have `fuzzgen` generate functions with the `tail` calling …
fitzgen Jun 15, 2023
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
2 changes: 1 addition & 1 deletion cranelift/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ harness = false

[dependencies]
cfg-if = { workspace = true }
cranelift-codegen = { workspace = true, features = ["disas"] }
cranelift-codegen = { workspace = true, features = ["disas", "trace-log"] }
Comment thread
cfallin marked this conversation as resolved.
cranelift-entity = { workspace = true }
cranelift-interpreter = { workspace = true }
cranelift-reader = { workspace = true }
Expand Down
5 changes: 5 additions & 0 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 @@ -990,6 +990,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
tmp: Writable<Reg>,
callee_conv: isa::CallConv,
caller_conv: isa::CallConv,
callee_pop_size: u32,
) -> SmallVec<[Inst; 2]> {
let mut insts = SmallVec::new();
match &dest {
Expand All @@ -1002,6 +1003,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
opcode,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
}),
&CallDest::ExtName(ref name, RelocDistance::Far) => {
Expand All @@ -1019,6 +1021,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
opcode,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
});
}
Expand All @@ -1031,6 +1034,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
opcode,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
}),
}
Expand Down Expand Up @@ -1073,6 +1077,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
opcode: Opcode::Call,
caller_callconv: call_conv,
callee_callconv: call_conv,
callee_pop_size: 0,
}),
});
insts
Expand Down
15 changes: 15 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3165,6 +3165,13 @@ impl MachInstEmit for Inst {
if info.opcode.is_call() {
sink.add_call_site(info.opcode);
}

let callee_pop_size = i64::from(info.callee_pop_size);
state.virtual_sp_offset -= callee_pop_size;
trace!(
"call adjusts virtual sp offset by {callee_pop_size} -> {}",
state.virtual_sp_offset
);
}
&Inst::CallInd { ref info } => {
if let Some(s) = state.take_stack_map() {
Expand All @@ -3175,6 +3182,13 @@ impl MachInstEmit for Inst {
if info.opcode.is_call() {
sink.add_call_site(info.opcode);
}

let callee_pop_size = i64::from(info.callee_pop_size);
state.virtual_sp_offset -= callee_pop_size;
trace!(
"call adjusts virtual sp offset by {callee_pop_size} -> {}",
state.virtual_sp_offset
);
}
&Inst::CondBr {
taken,
Expand Down Expand Up @@ -3587,6 +3601,7 @@ impl MachInstEmit for Inst {
opcode: Opcode::CallIndirect,
caller_callconv: CallConv::AppleAarch64,
callee_callconv: CallConv::AppleAarch64,
callee_pop_size: 0,
}),
}
.emit(&[], sink, emit_info, state);
Expand Down
2 changes: 2 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst/emit_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6084,6 +6084,7 @@ fn test_aarch64_binemit() {
opcode: Opcode::Call,
caller_callconv: CallConv::SystemV,
callee_callconv: CallConv::SystemV,
callee_pop_size: 0,
}),
},
"00000094",
Expand All @@ -6100,6 +6101,7 @@ fn test_aarch64_binemit() {
opcode: Opcode::CallIndirect,
caller_callconv: CallConv::SystemV,
callee_callconv: CallConv::SystemV,
callee_pop_size: 0,
}),
},
"40013FD6",
Expand Down
8 changes: 8 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ pub struct CallInfo {
pub caller_callconv: CallConv,
/// Callee calling convention.
pub callee_callconv: CallConv,
/// The number of bytes that the callee will pop from the stack for the
/// caller, if any. (Used for popping stack arguments with the `tail`
/// calling convention.)
pub callee_pop_size: u32,
}

/// Additional information for CallInd instructions, left out of line to lower the size of the Inst
Expand All @@ -112,6 +116,10 @@ pub struct CallIndInfo {
pub caller_callconv: CallConv,
/// Callee calling convention.
pub callee_callconv: CallConv,
/// The number of bytes that the callee will pop from the stack for the
/// caller, if any. (Used for popping stack arguments with the `tail`
/// calling convention.)
pub callee_pop_size: u32,
}

/// Additional information for JTSequence instructions, left out of line to lower the size of the Inst
Expand Down
6 changes: 6 additions & 0 deletions cranelift/codegen/src/isa/riscv64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
opcode: Opcode::Call,
callee_callconv: CallConv::SystemV,
caller_callconv: CallConv::SystemV,
callee_pop_size: 0,
}),
});
}
Expand Down Expand Up @@ -527,6 +528,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
tmp: Writable<Reg>,
callee_conv: isa::CallConv,
caller_conv: isa::CallConv,
callee_pop_size: u32,
) -> SmallVec<[Self::I; 2]> {
let mut insts = SmallVec::new();
match &dest {
Expand All @@ -539,6 +541,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
opcode,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
}),
&CallDest::ExtName(ref name, RelocDistance::Far) => {
Expand All @@ -556,6 +559,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
opcode,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
});
}
Expand All @@ -568,6 +572,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
opcode,
caller_callconv: caller_conv,
callee_callconv: callee_conv,
callee_pop_size,
}),
}),
}
Expand Down Expand Up @@ -609,6 +614,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {
opcode: Opcode::Call,
caller_callconv: call_conv,
callee_callconv: call_conv,
callee_pop_size: 0,
}),
});
insts
Expand Down
15 changes: 15 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::ir::TrapCode;
use crate::isa::riscv64::inst::*;
use crate::isa::riscv64::inst::{zero_reg, AluOPRRR};
use crate::machinst::{AllocationConsumer, Reg, Writable};
use crate::trace;
use cranelift_control::ControlPlane;
use regalloc2::Allocation;

Expand Down Expand Up @@ -847,6 +848,13 @@ impl MachInstEmit for Inst {
.emit(&[], sink, emit_info, state);
}
}

let callee_pop_size = i64::from(info.callee_pop_size);
state.virtual_sp_offset -= callee_pop_size;
trace!(
"call adjusts virtual sp offset by {callee_pop_size} -> {}",
state.virtual_sp_offset
);
}
&Inst::CallInd { ref info } => {
let rn = allocs.next(info.rn);
Expand All @@ -863,6 +871,13 @@ impl MachInstEmit for Inst {
offset: Imm12::zero(),
}
.emit(&[], sink, emit_info, state);

let callee_pop_size = i64::from(info.callee_pop_size);
state.virtual_sp_offset -= callee_pop_size;
trace!(
"call adjusts virtual sp offset by {callee_pop_size} -> {}",
state.virtual_sp_offset
);
}

&Inst::Jal { dest } => {
Expand Down
2 changes: 2 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ pub struct CallInfo {
pub caller_callconv: CallConv,
pub callee_callconv: CallConv,
pub clobbers: PRegSet,
pub callee_pop_size: u32,
}

/// Additional information for CallInd instructions, left out of line to lower the size of the Inst
Expand All @@ -87,6 +88,7 @@ pub struct CallIndInfo {
pub caller_callconv: CallConv,
pub callee_callconv: CallConv,
pub clobbers: PRegSet,
pub callee_pop_size: u32,
}

/// A branch target. Either unresolved (basic-block index) or resolved (offset
Expand Down
1 change: 1 addition & 0 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 @@ -759,6 +759,7 @@ impl ABIMachineSpec for S390xMachineDeps {
_tmp: Writable<Reg>,
_callee_conv: isa::CallConv,
_caller_conv: isa::CallConv,
_callee_pop_size: u32,
) -> SmallVec<[Inst; 2]> {
unreachable!();
}
Expand Down
15 changes: 14 additions & 1 deletion cranelift/codegen/src/isa/x64/abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
defs: smallvec![],
clobbers: PRegSet::empty(),
opcode: Opcode::Call,
callee_pop_size: 0,
}),
});
}
Expand Down Expand Up @@ -594,11 +595,19 @@ impl ABIMachineSpec for X64ABIMachineSpec {
tmp: Writable<Reg>,
_callee_conv: isa::CallConv,
_caller_conv: isa::CallConv,
callee_pop_size: u32,
) -> SmallVec<[Self::I; 2]> {
let mut insts = SmallVec::new();
match dest {
&CallDest::ExtName(ref name, RelocDistance::Near) => {
insts.push(Inst::call_known(name.clone(), uses, defs, clobbers, opcode));
insts.push(Inst::call_known(
name.clone(),
uses,
defs,
clobbers,
opcode,
callee_pop_size,
));
}
&CallDest::ExtName(ref name, RelocDistance::Far) => {
insts.push(Inst::LoadExtName {
Expand All @@ -613,6 +622,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
defs,
clobbers,
opcode,
callee_pop_size,
));
}
&CallDest::Reg(reg) => {
Expand All @@ -622,6 +632,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
defs,
clobbers,
opcode,
callee_pop_size,
));
}
}
Expand Down Expand Up @@ -651,6 +662,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
offset: 0,
distance: RelocDistance::Far,
});
let callee_pop_size = 0;
insts.push(Inst::call_unknown(
RegMem::reg(temp2.to_reg()),
/* uses = */
Expand All @@ -671,6 +683,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
/* defs = */ smallvec![],
/* clobbers = */ Self::get_regs_clobbered_by_call(call_conv),
Opcode::Call,
callee_pop_size,
));
insts
}
Expand Down
14 changes: 14 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,13 @@ pub(crate) fn emit(
if call_info.opcode.is_call() {
sink.add_call_site(call_info.opcode);
}

let callee_pop_size = i64::from(call_info.callee_pop_size);
state.virtual_sp_offset -= callee_pop_size;
trace!(
"call adjusts virtual sp offset by {callee_pop_size} -> {}",
state.virtual_sp_offset
);
}

Inst::CallUnknown {
Expand Down Expand Up @@ -1623,6 +1630,13 @@ pub(crate) fn emit(
if call_info.opcode.is_call() {
sink.add_call_site(call_info.opcode);
}

let callee_pop_size = i64::from(call_info.callee_pop_size);
state.virtual_sp_offset -= callee_pop_size;
trace!(
"call adjusts virtual sp offset by {callee_pop_size} -> {}",
state.virtual_sp_offset
);
}

Inst::Args { .. } => {}
Expand Down
2 changes: 2 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/emit_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4335,6 +4335,7 @@ fn test_x64_emit() {
smallvec![],
PRegSet::default(),
Opcode::Call,
0,
),
"E800000000",
"call User(userextname0)",
Expand All @@ -4349,6 +4350,7 @@ fn test_x64_emit() {
smallvec![],
PRegSet::default(),
Opcode::CallIndirect,
0,
)
}

Expand Down
8 changes: 8 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ pub struct CallInfo {
pub clobbers: PRegSet,
/// The opcode of this call.
pub opcode: Opcode,
/// The number of bytes that the callee will pop from the stack for the
/// caller, if any. (Used for popping stack arguments with the `tail`
/// calling convention.)
pub callee_pop_size: u32,
}

#[test]
Expand Down Expand Up @@ -515,6 +519,7 @@ impl Inst {
defs: CallRetList,
clobbers: PRegSet,
opcode: Opcode,
callee_pop_size: u32,
) -> Inst {
Inst::CallKnown {
dest,
Expand All @@ -523,6 +528,7 @@ impl Inst {
defs,
clobbers,
opcode,
callee_pop_size,
}),
}
}
Expand All @@ -533,6 +539,7 @@ impl Inst {
defs: CallRetList,
clobbers: PRegSet,
opcode: Opcode,
callee_pop_size: u32,
) -> Inst {
dest.assert_regclass_is(RegClass::Int);
Inst::CallUnknown {
Expand All @@ -542,6 +549,7 @@ impl Inst {
defs,
clobbers,
opcode,
callee_pop_size,
}),
}
}
Expand Down
Loading