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

Gate support for implicit return area pointers behind an option #9511

Merged
merged 7 commits into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
19 changes: 19 additions & 0 deletions cranelift/codegen/meta/src/shared/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,25 @@ pub(crate) fn define() -> SettingGroup {
false,
);

settings.add_bool(
"enable_multi_ret_implicit_sret",
"Enable support for sret arg introduction when there are too many ret vals.",
r#"
When there are more returns than available return registers, the
return value has to be returned through the introduction of a
return area pointer. Normally this return area pointer has to be
introduced as `ArgumentPurpose::StructReturn` parameter, but for
back compat reasons Cranelift also supports implicitly introducing
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
this parameter and writing the return values through it.

**This option violates the ABI of all targets and the used ABI should
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
not be assumed to remain the same between Cranelift versions.**

This option is **deprecated** and will be removed in the future.
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
"#,
false,
);

settings.add_bool(
"unwind_info",
"Generate unwind information.",
Expand Down
11 changes: 10 additions & 1 deletion cranelift/codegen/src/isa/aarch64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use alloc::boxed::Box;
use alloc::vec::Vec;
use regalloc2::{MachineEnv, PReg, PRegSet};
use smallvec::{smallvec, SmallVec};
use std::borrow::ToOwned;
use std::sync::OnceLock;

// We use a generic implementation that factors out AArch64 and x64 ABI commonalities, because
Expand Down Expand Up @@ -297,7 +298,7 @@ impl ABIMachineSpec for AArch64MachineDeps {
};

let push_to_reg = if is_winch_return {
// Winch uses the first registry to return the last result
// Winch uses the first register to return the last result
i == params.len() - 1
} else {
// Use max_per_class_reg_vals & remaining_reg_vals otherwise
Expand Down Expand Up @@ -330,6 +331,14 @@ impl ABIMachineSpec for AArch64MachineDeps {

// Spill to the stack

if args_or_rets == ArgsOrRets::Rets && !flags.enable_multi_ret_implicit_sret() {
return Err(crate::CodegenError::Unsupported(
"Too many return values to fit in registers. \
Use a StructReturn argument instead"
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
.to_owned(),
));
}

// Compute the stack slot's size.
let size = (ty_bits(param.value_type) / 8) as u32;

Expand Down
11 changes: 10 additions & 1 deletion cranelift/codegen/src/isa/pulley_shared/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use alloc::{boxed::Box, vec::Vec};
use core::marker::PhantomData;
use regalloc2::{MachineEnv, PReg, PRegSet};
use smallvec::{smallvec, SmallVec};
use std::borrow::ToOwned;
use std::sync::OnceLock;

/// Support for the Pulley ABI from the callee side (within a function body).
Expand Down Expand Up @@ -52,7 +53,7 @@ where

fn compute_arg_locs(
call_conv: isa::CallConv,
_flags: &settings::Flags,
flags: &settings::Flags,
params: &[ir::AbiParam],
args_or_rets: ArgsOrRets,
add_ret_area_ptr: bool,
Expand Down Expand Up @@ -113,6 +114,14 @@ where
extension: param.extension,
});
} else {
if args_or_rets == ArgsOrRets::Rets && !flags.enable_multi_ret_implicit_sret() {
return Err(crate::CodegenError::Unsupported(
"Too many return values to fit in registers. \
Use a StructReturn argument instead"
.to_owned(),
));
}

// Compute size and 16-byte stack alignment happens
// separately after all args.
let size = reg_ty.bits() / 8;
Expand Down
11 changes: 10 additions & 1 deletion cranelift/codegen/src/isa/riscv64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use alloc::vec::Vec;
use regalloc2::{MachineEnv, PReg, PRegSet};

use smallvec::{smallvec, SmallVec};
use std::borrow::ToOwned;
use std::sync::OnceLock;

/// Support for the Riscv64 ABI from the callee side (within a function body).
Expand Down Expand Up @@ -88,7 +89,7 @@ impl ABIMachineSpec for Riscv64MachineDeps {

fn compute_arg_locs(
call_conv: isa::CallConv,
_flags: &settings::Flags,
flags: &settings::Flags,
params: &[ir::AbiParam],
args_or_rets: ArgsOrRets,
add_ret_area_ptr: bool,
Expand Down Expand Up @@ -154,6 +155,14 @@ impl ABIMachineSpec for Riscv64MachineDeps {
extension: param.extension,
});
} else {
if args_or_rets == ArgsOrRets::Rets && !flags.enable_multi_ret_implicit_sret() {
return Err(crate::CodegenError::Unsupported(
"Too many return values to fit in registers. \
Use a StructReturn argument instead"
.to_owned(),
));
}

// Compute size and 16-byte stack alignment happens
// separately after all args.
let size = reg_ty.bits() / 8;
Expand Down
11 changes: 10 additions & 1 deletion cranelift/codegen/src/isa/s390x/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ use crate::CodegenResult;
use alloc::vec::Vec;
use regalloc2::{MachineEnv, PRegSet};
use smallvec::{smallvec, SmallVec};
use std::borrow::ToOwned;
use std::sync::OnceLock;

// We use a generic implementation that factors out ABI commonalities.
Expand Down Expand Up @@ -290,7 +291,7 @@ impl ABIMachineSpec for S390xMachineDeps {

fn compute_arg_locs(
call_conv: isa::CallConv,
_flags: &settings::Flags,
flags: &settings::Flags,
params: &[ir::AbiParam],
args_or_rets: ArgsOrRets,
add_ret_area_ptr: bool,
Expand Down Expand Up @@ -384,6 +385,14 @@ impl ABIMachineSpec for S390xMachineDeps {
extension: param.extension,
}
} else {
if args_or_rets == ArgsOrRets::Rets && !flags.enable_multi_ret_implicit_sret() {
return Err(crate::CodegenError::Unsupported(
"Too many return values to fit in registers. \
Use a StructReturn argument instead"
.to_owned(),
));
}

// Compute size. Every argument or return value takes a slot of
// at least 8 bytes.
let size = (ty_bits(param.value_type) / 8) as u32;
Expand Down
9 changes: 9 additions & 0 deletions cranelift/codegen/src/isa/x64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use alloc::vec::Vec;
use args::*;
use regalloc2::{MachineEnv, PReg, PRegSet};
use smallvec::{smallvec, SmallVec};
use std::borrow::ToOwned;
use std::sync::OnceLock;

/// Support for the x64 ABI from the callee side (within a function body).
Expand Down Expand Up @@ -337,6 +338,14 @@ impl ABIMachineSpec for X64ABIMachineSpec {
extension: param.extension,
});
} else {
if args_or_rets == ArgsOrRets::Rets && !flags.enable_multi_ret_implicit_sret() {
return Err(crate::CodegenError::Unsupported(
"Too many return values to fit in registers. \
Use a StructReturn argument instead"
.to_owned(),
));
}

let size = reg_ty.bytes();
let size = if call_conv == CallConv::Winch
&& args_or_rets == ArgsOrRets::Rets
Expand Down
3 changes: 3 additions & 0 deletions cranelift/codegen/src/machinst/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,9 @@ impl SigSet {
/* extra ret-area ptr = */ false,
ArgsAccumulator::new(&mut self.abi_args),
)?;
if !flags.enable_multi_ret_implicit_sret() {
assert_eq!(sized_stack_ret_space, 0);
}
let rets_end = u32::try_from(self.abi_args.len()).unwrap();

// To avoid overflow issues, limit the return size to something reasonable.
Expand Down
8 changes: 0 additions & 8 deletions cranelift/codegen/src/machinst/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,14 +470,6 @@ macro_rules! isle_lower_prelude_methods {
}
}

fn abi_sized_stack_arg_space(&mut self, abi: Sig) -> i64 {
self.lower_ctx.sigs()[abi].sized_stack_arg_space()
}

fn abi_sized_stack_ret_space(&mut self, abi: Sig) -> i64 {
self.lower_ctx.sigs()[abi].sized_stack_ret_space()
}

fn abi_arg_only_slot(&mut self, arg: &ABIArg) -> Option<ABIArgSlot> {
match arg {
&ABIArg::Slots { ref slots, .. } => {
Expand Down
10 changes: 1 addition & 9 deletions cranelift/codegen/src/prelude_lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@

;; Extract a constant `i32` from a value defined by an `iconst`.
;; The value is sign extended to 32 bits.
(spec (i32_from_iconst arg)
(spec (i32_from_iconst arg)
(provide (= arg (extract 31 0 (sign_ext 64 result))))
(require (= result (sign_ext (widthof result) arg))))
(decl i32_from_iconst (i32) Value)
Expand Down Expand Up @@ -1044,14 +1044,6 @@
(decl abi_no_ret_arg () Sig)
(extern extractor abi_no_ret_arg abi_no_ret_arg)

;; Size of the argument area.
(decl abi_sized_stack_arg_space (Sig) i64)
(extern constructor abi_sized_stack_arg_space abi_sized_stack_arg_space)

;; Size of the return-value area.
(decl abi_sized_stack_ret_space (Sig) i64)
(extern constructor abi_sized_stack_ret_space abi_sized_stack_ret_space)

;; Incoming return area pointer (must be present).
(decl abi_unwrap_ret_area_ptr () Reg)
(extern constructor abi_unwrap_ret_area_ptr abi_unwrap_ret_area_ptr)
Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/aarch64/call.clif
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ test compile precise-output
set unwind_info=false
set enable_probestack=false
set enable_llvm_abi_extensions
set enable_multi_ret_implicit_sret
target aarch64

function %f1(i64) -> i64 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
target aarch64

;; Test the `tail` calling convention with non-tail calls and stack arguments.
Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/pulley32/call.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
target pulley32

function %colocated_args_i64_rets_i64() -> i64 {
Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/pulley64/call.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
target pulley64

function %colocated_args_i64_rets_i64() -> i64 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/call.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
target riscv64gc has_v has_zbkb has_zba has_zbb has_zbc has_zbs


Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/issue8847.clif
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ test compile
set bb_padding_log2_minus_one=4
set enable_alias_analysis=false
set enable_llvm_abi_extensions=true
set enable_multi_ret_implicit_sret
set machine_code_cfg_info=true
set enable_jump_tables=false
set enable_heap_access_spectre_mitigation=false
Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/issue8866.clif
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ test compile
set bb_padding_log2_minus_one=4
set enable_alias_analysis=false
set enable_llvm_abi_extensions=true
set enable_multi_ret_implicit_sret
set machine_code_cfg_info=true
set enable_jump_tables=false
set enable_heap_access_spectre_mitigation=false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set enable_nan_canonicalization=true
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
test compile precise-output
set enable_nan_canonicalization=true
set enable_multi_ret_implicit_sret
set enable_nan_canonicalization=true
target riscv64

function %f1(f64, f64) -> f64 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64gc has_v has_zvl4096b

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-abi.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
target riscv64 has_v

;; Tests both ABI and Regalloc spill/reload.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-band.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-bnot.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-bor.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-bxor.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-ceil.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-fabs.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
1 change: 1 addition & 0 deletions cranelift/filetests/filetests/isa/riscv64/simd-fadd.clif
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
test compile precise-output
set enable_multi_ret_implicit_sret
set unwind_info=false
target riscv64 has_v

Expand Down
Loading
Loading