Winch: Fix issue with multivalue returns (bytecodealliance#10370)

Author: jeffcharles

tests/disas/winch/x64/local/v128_multivalue.wat

@@ -0,0 +1,44 @@
+;;! target = "x86_64"
+;;! test = "winch"
+
+(module
+  (func (export "") (param v128) (result i64 v128 i64)
+    i64.const 0
+    local.get 0
+    i64.const 0
+  )
+)
+;; wasm[0]::function[0]:
+;;       pushq   %rbp
+;;       movq    %rsp, %rbp
+;;       movq    8(%rsi), %r11
+;;       movq    0x10(%r11), %r11
+;;       addq    $0x48, %r11
+;;       cmpq    %rsp, %r11
+;;       ja      0x90
+;;   1c: movq    %rsi, %r14
+;;       subq    $0x30, %rsp
+;;       movq    %rsi, 0x28(%rsp)
+;;       movq    %rdx, 0x20(%rsp)
+;;       movdqu  %xmm0, 0x10(%rsp)
+;;       movq    %rdi, 8(%rsp)
+;;       movq    $0, %rax
+;;       movdqu  0x10(%rsp), %xmm15
+;;       subq    $0x10, %rsp
+;;       movdqu  %xmm15, (%rsp)
+;;       subq    $8, %rsp
+;;       movq    8(%rsp), %r11
+;;       movq    %r11, (%rsp)
+;;       movq    0x10(%rsp), %r11
+;;       movq    %r11, 8(%rsp)
+;;       movq    $0, 0x10(%rsp)
+;;       movq    0x20(%rsp), %rcx
+;;       movdqu  (%rsp), %xmm15
+;;       addq    $0x10, %rsp
+;;       movdqu  %xmm15, (%rcx)
+;;       popq    %r11
+;;       movq    %r11, 0x10(%rcx)
+;;       addq    $0x30, %rsp
+;;       popq    %rbp
+;;       retq
+;;   90: ud2

tests/misc_testsuite/winch/issue-10357.wast

@@ -0,0 +1,20 @@
+;;! simd = true
+
+;; See https://github.com/bytecodealliance/wasmtime/issues/10357
+
+(module
+  (func (export "test") (result i64 v128 v128)
+    i32.const 0
+    v128.const i32x4 0x00000000 0x00000000 0x68732efe 0x74727473
+    i64.const 1
+    call 1
+  )
+
+  (func (param i32 v128 i64) (result i64 v128 v128)
+    i64.const 0
+    local.get 1
+    v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000
+  )
+)
+
+(assert_return (invoke "test") (i64.const 0) (v128.const i32x4 0x00000000 0x00000000 0x68732efe 0x74727473) (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000))

tests/misc_testsuite/winch/simd_multivalue.wast

@@ -7,3 +7,13 @@
 ;; test 0 consts
 (module (func (export "consts") (result v128) (result v128) (v128.const i64x2 0 0) (v128.const i64x2 0 0)))
 (assert_return (invoke "consts") (v128.const i64x2 0 0) (v128.const i64x2 0 0))
+
+;; test case where vector is neither the first return value nor the last return value
+(module
+  (func (export "not-first-nor-last") (param v128) (result i64 v128 i64)
+    i64.const 0
+    local.get 0
+    i64.const 0
+  )
+)
+(assert_return (invoke "not-first-nor-last" (v128.const i32x4 1 2 3 4)) (i64.const 0) (v128.const i32x4 1 2 3 4) (i64.const 0))

winch/codegen/src/masm.rs

@@ -1471,31 +1471,60 @@ pub(crate) trait MacroAssembler {
         let word_bytes = <Self::ABI as abi::ABI>::word_bytes();
         let scratch = scratch!(Self);
 
-        let mut dst_offs = dst.as_u32() - bytes;
-        let mut src_offs = src.as_u32() - bytes;
-
         let word_bytes = word_bytes as u32;
-        while remaining >= word_bytes {
-            remaining -= word_bytes;
-            dst_offs += word_bytes;
-            src_offs += word_bytes;
 
-            self.load_ptr(
-                self.address_from_sp(SPOffset::from_u32(src_offs))?,
-                writable!(scratch),
-            )?;
-            self.store_ptr(
-                scratch.into(),
-                self.address_from_sp(SPOffset::from_u32(dst_offs))?,
-            )?;
+        let mut dst_offs;
+        let mut src_offs;
+        match direction {
+            MemMoveDirection::LowToHigh => {
+                dst_offs = dst.as_u32() - bytes;
+                src_offs = src.as_u32() - bytes;
+                while remaining >= word_bytes {
+                    remaining -= word_bytes;
+                    dst_offs += word_bytes;
+                    src_offs += word_bytes;
+
+                    self.load_ptr(
+                        self.address_from_sp(SPOffset::from_u32(src_offs))?,
+                        writable!(scratch),
+                    )?;
+                    self.store_ptr(
+                        scratch.into(),
+                        self.address_from_sp(SPOffset::from_u32(dst_offs))?,
+                    )?;
+                }
+            }
+            MemMoveDirection::HighToLow => {
+                // Go from the end to the beginning to handle overlapping addresses.
+                src_offs = src.as_u32();
+                dst_offs = dst.as_u32();
+                while remaining >= word_bytes {
+                    self.load_ptr(
+                        self.address_from_sp(SPOffset::from_u32(src_offs))?,
+                        writable!(scratch),
+                    )?;
+                    self.store_ptr(
+                        scratch.into(),
+                        self.address_from_sp(SPOffset::from_u32(dst_offs))?,
+                    )?;
+
+                    remaining -= word_bytes;
+                    src_offs -= word_bytes;
+                    dst_offs -= word_bytes;
+                }
+            }
         }
 
         if remaining > 0 {
             let half_word = word_bytes / 2;
             let ptr_size = OperandSize::from_bytes(half_word as u8);
             debug_assert!(remaining == half_word);
-            dst_offs += half_word;
-            src_offs += half_word;
+            // Need to move the offsets ahead in the `LowToHigh` case to
+            // compensate for the initial subtraction of `bytes`.
+            if direction == MemMoveDirection::LowToHigh {
+                dst_offs += half_word;
+                src_offs += half_word;
+            }
 
             self.load(
                 self.address_from_sp(SPOffset::from_u32(src_offs))?,