Gate support for implicit return area pointers behind an option

It implements an incorrect ABI and may be removed in the future due to
complexity reasons. By requiring to enable an option to use it, it
becomes harder to accidentally hit the ABI issue and allows a more
deprecation and eventual removal.

cranelift/codegen/meta/src/shared/settings.rs

@@ -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
+            this parameter and writing the return values through it.
+
+            **This option violates the ABI of all targets and the used ABI should
+            not be assumed to remain the same between Cranelift versions.**
+
+            This option is **deprecated** and will be removed in the future.
+        "#,
+        false,
+    );
+
     settings.add_bool(
         "unwind_info",
         "Generate unwind information.",

cranelift/codegen/src/isa/aarch64/abi.rs

@@ -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
@@ -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
@@ -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"
+                        .to_owned(),
+                ));
+            }
+
             // Compute the stack slot's size.
             let size = (ty_bits(param.value_type) / 8) as u32;
 

cranelift/codegen/src/isa/pulley_shared/abi.rs

@@ -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).
@@ -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,
@@ -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;

cranelift/codegen/src/isa/riscv64/abi.rs

@@ -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).
@@ -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,
@@ -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;

cranelift/codegen/src/isa/s390x/abi.rs

@@ -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.
@@ -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,
@@ -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;

cranelift/codegen/src/isa/x64/abi.rs

@@ -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).
@@ -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

cranelift/codegen/src/machinst/abi.rs

@@ -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.

cranelift/filetests/filetests/isa/aarch64/call.clif

@@ -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 {

cranelift/filetests/filetests/isa/aarch64/tail-call-conv.clif

@@ -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.

cranelift/filetests/filetests/isa/pulley32/call.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 target pulley32
 
 function %colocated_args_i64_rets_i64() -> i64 {

cranelift/filetests/filetests/isa/pulley64/call.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 target pulley64
 
 function %colocated_args_i64_rets_i64() -> i64 {

cranelift/filetests/filetests/isa/riscv64/bitcast-scalar-vector.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/call.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64
 

cranelift/filetests/filetests/isa/riscv64/issue-6954.clif

@@ -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
 
 

cranelift/filetests/filetests/isa/riscv64/issue8847.clif

@@ -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

cranelift/filetests/filetests/isa/riscv64/issue8866.clif

@@ -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

cranelift/filetests/filetests/isa/riscv64/nan-canonicalization-has_v.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set enable_nan_canonicalization=true
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/nan-canonicalization.clif

@@ -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 {

cranelift/filetests/filetests/isa/riscv64/simd-abi-large-spill.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64gc has_v has_zvl4096b
 

cranelift/filetests/filetests/isa/riscv64/simd-abi.clif

@@ -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.

cranelift/filetests/filetests/isa/riscv64/simd-avg_round.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-band.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-bitselect.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-bnot.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-bor.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-bxor.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-ceil.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-extractlane.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fabs.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fadd.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-eq.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-ge.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-gt.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-le.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-lt.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-ne.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-one.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-ord.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-ueq.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-uge.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v
 

cranelift/filetests/filetests/isa/riscv64/simd-fcmp-ugt.clif

@@ -1,4 +1,5 @@
 test compile precise-output
+set enable_multi_ret_implicit_sret
 set unwind_info=false
 target riscv64 has_v