Repository: bytecodealliance/wasmtime
Pull request #8270 (2024-03-31 14:07:54)

Fix the SystemV abi of i128 arguments
i128 arguments must always be passed either fully in registers or fully on the stack according to GCC. LLVM until recently allowed splitting it into a half passed in a register and a half passed on the stack. This was recently fixed. For cg_clif to remain ABI compatible with cg_llvm, it is necessary to apply the same fix to Cranelift.

See also https://blog.rust-lang.org/2024/03/30/i128-layout-update.html


Patch:
diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs
index 451f6501f..c7c5a4a82 100644
--- a/cranelift/codegen/src/isa/x64/abi.rs
+++ b/cranelift/codegen/src/isa/x64/abi.rs
@@ -153,9 +153,10 @@ impl ABIMachineSpec for X64ABIMachineSpec {
             //
             // - If `enable_llvm_abi_extensions` is set in the flags, each
             //   `i128` is split into two `i64`s and assigned exactly as if it
-            //   were two consecutive 64-bit args. This is consistent with LLVM's
-            //   behavior, and is needed for some uses of Cranelift (e.g., the
-            //   rustc backend).
+            //   were two consecutive 64-bit args, except that if one of the
+            //   two halves is forced onto the stack, the other half is too.
+            //   This is consistent with LLVM's behavior, and is needed for
+            //   some uses of Cranelift (e.g., the rustc backend).
             //
             // - Otherwise, both SysV and Fastcall specify behavior (use of
             //   vector register, a register pair, or passing by reference
@@ -214,6 +215,62 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                 continue;
             }
 
+            // SystemV dictates that 128bit int parameters are always either
+            // passed in two registers or on the stack, so handle that as a
+            // special case before the loop below.
+            if param.value_type == types::I128
+                && args_or_rets == ArgsOrRets::Args
+                && call_conv == CallConv::SystemV
+            {
+                let mut slots = ABIArgSlotVec::new();
+                match (
+                    get_intreg_for_arg(&CallConv::SystemV, next_gpr, next_param_idx),
+                    get_intreg_for_arg(&CallConv::SystemV, next_gpr + 1, next_param_idx + 1),
+                ) {
+                    (Some(reg1), Some(reg2)) => {
+                        slots.push(ABIArgSlot::Reg {
+                            reg: reg1.to_real_reg().unwrap(),
+                            ty: ir::types::I64,
+                            extension: ir::ArgumentExtension::None,
+                        });
+                        slots.push(ABIArgSlot::Reg {
+                            reg: reg2.to_real_reg().unwrap(),
+                            ty: ir::types::I64,
+                            extension: ir::ArgumentExtension::None,
+                        });
+                    }
+                    _ => {
+                        let size = 16;
+
+                        // Align.
+                        next_stack = align_to(next_stack, size);
+
+                        slots.push(ABIArgSlot::Stack {
+                            offset: next_stack as i64,
+                            ty: ir::types::I64,
+                            extension: param.extension,
+                        });
+                        slots.push(ABIArgSlot::Stack {
+                            offset: next_stack as i64 + 8,
+                            ty: ir::types::I64,
+                            extension: param.extension,
+                        });
+                        next_stack += size;
+                    }
+                };
+                // Unconditionally increment next_gpr even when storing the
+                // argument on the stack to prevent reusing a possibly
+                // remaining register for the next argument.
+                next_gpr += 2;
+                next_param_idx += 2;
+
+                args.push(ABIArg::Slots {
+                    slots,
+                    purpose: param.purpose,
+                });
+                continue;
+            }
+
             let mut slots = ABIArgSlotVec::new();
             for (ix, (rc, reg_ty)) in rcs.iter().zip(reg_tys.iter()).enumerate() {
                 let last_slot = last_param && ix == rcs.len() - 1;


Test patch:
diff --git a/cranelift/filetests/filetests/isa/x64/llvm-abi-no-split-stack-u128.clif b/cranelift/filetests/filetests/isa/x64/llvm-abi-no-split-stack-u128.clif
new file mode 100644
index 000000000..8375060bc
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/x64/llvm-abi-no-split-stack-u128.clif
@@ -0,0 +1,30 @@
+test compile precise-output
+set enable_llvm_abi_extensions=1
+target x86_64
+
+function u0:0(i64, i128, i128, i128) -> i128 system_v {
+block0(v0: i64, v1: i128, v2: i128, v3: i128):
+    return v3
+}
+
+; VCode:
+;   pushq   %rbp
+;   movq    %rsp, %rbp
+; block0:
+;   movq    16(%rbp), %rax
+;   movq    24(%rbp), %rdx
+;   movq    %rbp, %rsp
+;   popq    %rbp
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   pushq %rbp
+;   movq %rsp, %rbp
+; block1: ; offset 0x4
+;   movq 0x10(%rbp), %rax
+;   movq 0x18(%rbp), %rdx
+;   movq %rbp, %rsp
+;   popq %rbp
+;   retq
+