Skip to content

Cranelift: Remove ArgumentPurpose::StructReturn #4618

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

Closed
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
24 changes: 1 addition & 23 deletions cranelift/codegen/src/ir/extfunc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@ impl Signature {
.count()
}

/// Does this signature take an struct return pointer parameter?
pub fn uses_struct_return_param(&self) -> bool {
self.uses_special_param(ArgumentPurpose::StructReturn)
}

/// Does this return more than one normal value? (Pre-struct return
/// legalization)
pub fn is_multi_return(&self) -> bool {
Expand Down Expand Up @@ -242,16 +237,6 @@ pub enum ArgumentPurpose {
/// A C struct passed as argument.
StructArgument(u32),

/// Struct return pointer.
///
/// When a function needs to return more data than will fit in registers, the caller passes a
/// pointer to a memory location where the return value can be written. In some ABIs, this
/// struct return pointer is passed in a specific register.
///
/// This argument kind can also appear as a return value for ABIs that require a function with
/// a `StructReturn` pointer argument to also return that pointer in a register.
StructReturn,

/// The link register.
///
/// Most RISC architectures implement calls by saving the return address in a designated
Expand Down Expand Up @@ -299,7 +284,6 @@ impl fmt::Display for ArgumentPurpose {
f.write_str(match self {
Self::Normal => "normal",
Self::StructArgument(size) => return write!(f, "sarg({})", size),
Self::StructReturn => "sret",
Self::Link => "link",
Self::FramePointer => "fp",
Self::CalleeSaved => "csr",
Expand All @@ -315,7 +299,6 @@ impl FromStr for ArgumentPurpose {
fn from_str(s: &str) -> Result<Self, ()> {
match s {
"normal" => Ok(Self::Normal),
"sret" => Ok(Self::StructReturn),
"link" => Ok(Self::Link),
"fp" => Ok(Self::FramePointer),
"csr" => Ok(Self::CalleeSaved),
Expand Down Expand Up @@ -391,20 +374,15 @@ mod tests {
fn argument_type() {
let t = AbiParam::new(I32);
assert_eq!(t.to_string(), "i32");
let mut t = t.uext();
let t = t.uext();
assert_eq!(t.to_string(), "i32 uext");
assert_eq!(t.sext().to_string(), "i32 sext");
t.purpose = ArgumentPurpose::StructReturn;
assert_eq!(t.to_string(), "i32 uext sret");
t.legalized_to_pointer = true;
assert_eq!(t.to_string(), "i32 ptr uext sret");
}

#[test]
fn argument_purpose() {
let all_purpose = [
(ArgumentPurpose::Normal, "normal"),
(ArgumentPurpose::StructReturn, "sret"),
(ArgumentPurpose::Link, "link"),
(ArgumentPurpose::FramePointer, "fp"),
(ArgumentPurpose::CalleeSaved, "csr"),
Expand Down
1 change: 0 additions & 1 deletion cranelift/codegen/src/isa/aarch64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ impl ABIMachineSpec for AArch64MachineDeps {
| &ir::ArgumentPurpose::Normal
| &ir::ArgumentPurpose::StackLimit
| &ir::ArgumentPurpose::SignatureId
| &ir::ArgumentPurpose::StructReturn
| &ir::ArgumentPurpose::StructArgument(_) => {}
_ => panic!(
"Unsupported argument purpose {:?} in signature: {:?}",
Expand Down
1 change: 0 additions & 1 deletion cranelift/codegen/src/isa/s390x/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@ impl ABIMachineSpec for S390xMachineDeps {
| &ir::ArgumentPurpose::Normal
| &ir::ArgumentPurpose::StackLimit
| &ir::ArgumentPurpose::SignatureId
| &ir::ArgumentPurpose::StructReturn
| &ir::ArgumentPurpose::StructArgument(_) => {}
_ => panic!(
"Unsupported argument purpose {:?} in signature: {:?}",
Expand Down
1 change: 0 additions & 1 deletion cranelift/codegen/src/isa/x64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ impl ABIMachineSpec for X64ABIMachineSpec {
| &ir::ArgumentPurpose::Normal
| &ir::ArgumentPurpose::StackLimit
| &ir::ArgumentPurpose::SignatureId
| &ir::ArgumentPurpose::StructReturn
| &ir::ArgumentPurpose::StructArgument(_) => {}
_ => panic!(
"Unsupported argument purpose {:?} in signature: {:?}",
Expand Down
33 changes: 6 additions & 27 deletions cranelift/codegen/src/machinst/abi_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
use super::abi::*;
use crate::binemit::StackMap;
use crate::ir::types::*;
use crate::ir::{ArgumentExtension, ArgumentPurpose, DynamicStackSlot, Signature, StackSlot};
use crate::ir::{ArgumentExtension, DynamicStackSlot, Signature, StackSlot};
use crate::isa::TargetIsa;
use crate::settings;
use crate::CodegenResult;
Expand Down Expand Up @@ -548,8 +548,6 @@ impl ABISig {
sig: &ir::Signature,
flags: &settings::Flags,
) -> CodegenResult<ABISig> {
let sig = ensure_struct_return_ptr_is_returned(sig);

// Compute args and retvals from signature. Handle retvals first,
// because we may need to add a return-area arg to the args.
let (rets, sized_stack_ret_space, _) = M::compute_arg_locs(
Expand Down Expand Up @@ -778,7 +776,7 @@ impl<M: ABIMachineSpec> ABICalleeImpl<M> {
trace!("ABI: func signature {:?}", f.signature);

let flags = isa.flags().clone();
let ir_sig = ensure_struct_return_ptr_is_returned(&f.signature);
let ir_sig = f.signature.clone();
let sig = ABISig::from_func_sig::<M>(&ir_sig, &flags)?;

let call_conv = f.signature.call_conv;
Expand Down Expand Up @@ -1031,23 +1029,6 @@ fn gen_store_stack_multi<M: ABIMachineSpec>(
ret
}

fn ensure_struct_return_ptr_is_returned(sig: &ir::Signature) -> ir::Signature {
let params_structret = sig
.params
.iter()
.find(|p| p.purpose == ArgumentPurpose::StructReturn);
let rets_have_structret = sig.returns.len() > 0
&& sig
.returns
.iter()
.any(|arg| arg.purpose == ArgumentPurpose::StructReturn);
let mut sig = sig.clone();
if params_structret.is_some() && !rets_have_structret {
sig.returns.insert(0, params_structret.unwrap().clone());
}
sig
}

impl<M: ABIMachineSpec> ABICallee for ABICalleeImpl<M> {
type I = M::I;

Expand Down Expand Up @@ -1633,14 +1614,13 @@ pub enum CallDest {
impl<M: ABIMachineSpec> ABICallerImpl<M> {
/// Create a callsite ABI object for a call directly to the specified function.
pub fn from_func(
sig: &ir::Signature,
ir_sig: &ir::Signature,
extname: &ir::ExternalName,
dist: RelocDistance,
caller_conv: isa::CallConv,
flags: &settings::Flags,
) -> CodegenResult<ABICallerImpl<M>> {
let ir_sig = ensure_struct_return_ptr_is_returned(sig);
let sig = ABISig::from_func_sig::<M>(&ir_sig, flags)?;
let sig = ABISig::from_func_sig::<M>(ir_sig, flags)?;
let (uses, defs, clobbers) = sig.call_uses_defs_clobbers::<M>();
Ok(ABICallerImpl {
sig,
Expand All @@ -1658,14 +1638,13 @@ impl<M: ABIMachineSpec> ABICallerImpl<M> {
/// Create a callsite ABI object for a call to a function pointer with the
/// given signature.
pub fn from_ptr(
sig: &ir::Signature,
ir_sig: &ir::Signature,
ptr: Reg,
opcode: ir::Opcode,
caller_conv: isa::CallConv,
flags: &settings::Flags,
) -> CodegenResult<ABICallerImpl<M>> {
let ir_sig = ensure_struct_return_ptr_is_returned(sig);
let sig = ABISig::from_func_sig::<M>(&ir_sig, flags)?;
let sig = ABISig::from_func_sig::<M>(ir_sig, flags)?;
let (uses, defs, clobbers) = sig.call_uses_defs_clobbers::<M>();
Ok(ABICallerImpl {
sig,
Expand Down
18 changes: 0 additions & 18 deletions cranelift/codegen/src/machinst/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,24 +722,6 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
for insn in self.vcode.abi().gen_copy_arg_to_regs(i, regs).into_iter() {
self.emit(insn);
}
if self.abi().signature().params[i].purpose == ArgumentPurpose::StructReturn {
assert!(regs.len() == 1);
let ty = self.abi().signature().params[i].value_type;
// The ABI implementation must have ensured that a StructReturn
// arg is present in the return values.
let struct_ret_idx = self
.abi()
.signature()
.returns
.iter()
.position(|ret| ret.purpose == ArgumentPurpose::StructReturn)
.expect("StructReturn return value not present!");
self.emit(I::gen_move(
Writable::from_reg(self.retval_regs[struct_ret_idx].regs()[0]),
regs.regs()[0].to_reg(),
ty,
));
}
}
if let Some(insn) = self.vcode.abi().gen_retval_area_setup() {
self.emit(insn);
Expand Down
3 changes: 1 addition & 2 deletions cranelift/docs/ir.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ paramlist : param { "," param }
retlist : paramlist
param : type [paramext] [paramspecial]
paramext : "uext" | "sext"
paramspecial : "sret" | "link" | "fp" | "csr" | "vmctx" | "sigid" | "stack_limit"
paramspecial : "link" | "fp" | "csr" | "vmctx" | "sigid" | "stack_limit"
callconv : "fast" | "cold" | "system_v" | "windows_fastcall"
| "wasmtime_system_v" | "wasmtime_fastcall"
| "apple_aarch64" | "wasmtime_apple_aarch64"
Expand All @@ -419,7 +419,6 @@ system, a function's calling convention is only fully determined by a

| Name | Description |
| ----------| ---------- |
| sret | pointer to a return value in memory |
| link | return address |
| fp | the initial value of the frame pointer |
| csr | callee-saved register |
Expand Down
6 changes: 3 additions & 3 deletions cranelift/filetests/filetests/isa/x64/atomic-cas-bug.clif
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ function u0:31(i64, i32, i32, i8, i8) -> i32, i32 system_v {
gv6 = symbol colocated u1:35
gv7 = symbol colocated u1:36
gv8 = symbol colocated u1:37
sig0 = (i64 sret, i64, i64, i64, i64) system_v
sig1 = (i64 sret, i64, i64, i64, i64) system_v
sig0 = (i64, i64, i64, i64, i64) system_v
sig1 = (i64, i64, i64, i64, i64) system_v
sig2 = (i64, i64) system_v
sig3 = (i64 sret, i64, i64, i64, i64) system_v
sig3 = (i64, i64, i64, i64, i64) system_v
sig4 = (i64, i64) system_v
sig5 = (i64, i64) system_v
fn0 = colocated u0:11 sig0
Expand Down
20 changes: 0 additions & 20 deletions cranelift/filetests/filetests/isa/x64/struct-ret.clif

This file was deleted.

6 changes: 3 additions & 3 deletions cranelift/filetests/filetests/parser/call.clif
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ block1:
v2 = f32const 0.0
return v1, v2
}
; sameln: function %r1() -> i32, f32
; sameln: function %r1() -> i32, f32
; nextln: block1:
; nextln: v1 = iconst.i32 3
; nextln: v2 = f32const 0.0
Expand Down Expand Up @@ -84,11 +84,11 @@ block0:
; check: return

; Special purpose function arguments
function %special1(i32 sret, i32 fp, i32 csr, i32 link) -> i32 link, i32 fp, i32 csr, i32 sret {
function %special1(i32, i32 fp, i32 csr, i32 link) -> i32 link, i32 fp, i32 csr, i32 {
block0(v1: i32, v2: i32, v3: i32, v4: i32):
return v4, v2, v3, v1
}
; check: function %special1(i32 sret, i32 fp, i32 csr, i32 link) -> i32 link, i32 fp, i32 csr, i32 sret fast {
; check: function %special1(i32, i32 fp, i32 csr, i32 link) -> i32 link, i32 fp, i32 csr, i32 fast {
; check: block0(v1: i32, v2: i32, v3: i32, v4: i32):
; check: return v4, v2, v3, v1
; check: }
4 changes: 2 additions & 2 deletions cranelift/reader/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3175,12 +3175,12 @@ mod tests {
assert_eq!(sig.returns.len(), 0);
assert_eq!(sig.call_conv, CallConv::SystemV);

let sig2 = Parser::new("(i8 uext, f32, f64, i32 sret) -> i32 sext, f64 system_v")
let sig2 = Parser::new("(i8 uext, f32, f64, i32) -> i32 sext, f64 system_v")
.parse_signature()
.unwrap();
assert_eq!(
sig2.to_string(),
"(i8 uext, f32, f64, i32 sret) -> i32 sext, f64 system_v"
"(i8 uext, f32, f64, i32) -> i32 sext, f64 system_v"
);
assert_eq!(sig2.call_conv, CallConv::SystemV);

Expand Down