Repository: bytecodealliance/wasmtime
Pull request #8886 (2024-06-28 16:30:41)

winch: Save state before emitting `br_if`
Fixes: https://github.com/bytecodealliance/wasmtime/issues/8848

Similar to all the control instructions, any state must be explicitly saved before emitting the code for `br_if`.

This commit ensures that live locals and registers are explicilty saved before emitting the code for `br_if`. Prior to this commit, live locals and registers were not saved every time causing incorrect behavior in cases where the calculation of the conditional argument didn't trigger a spill.

This change introduces the explicit spill after calculating the branch condition argument to minimize memory traffic in case the conditional is already in a register.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/winch/codegen/src/visitor.rs b/winch/codegen/src/visitor.rs
index 97b60cecd..fae3189df 100644
--- a/winch/codegen/src/visitor.rs
+++ b/winch/codegen/src/visitor.rs
@@ -1711,6 +1711,11 @@ where
                 self.masm,
                 |ctx, masm| ctx.pop_to_reg(masm, None),
             );
+            // Explicitly save any live registers and locals before setting up
+            // the branch state.
+            // In some cases, calculating the `top` value above, will result in
+            // a spill, thus the following one will result in a no-op.
+            self.context.spill(self.masm);
             frame.top_abi_results::<M, _>(
                 &mut self.context,
                 self.masm,


Test patch:
diff --git a/tests/disas/winch/x64/br_if/save_state_before_br_emission.wat b/tests/disas/winch/x64/br_if/save_state_before_br_emission.wat
new file mode 100644
index 000000000..016361685
--- /dev/null
+++ b/tests/disas/winch/x64/br_if/save_state_before_br_emission.wat
@@ -0,0 +1,105 @@
+;;! target = "x86_64"
+;;! test = "winch"
+(module
+  (func (param f64 f64 f64 f64) (result f32 f64)
+    f64.const 0
+    local.get 0
+    i64.const 0
+    f64.const 0
+    i64.const 0
+    local.get 0
+    f64.const 0
+    i64.const 1
+    i32.const 1
+    i64.const 1
+    f32.const 0
+    local.get 0
+
+    i32.const 0
+    br_if 0
+
+    drop
+    drop
+    drop
+    drop
+    drop
+    drop
+    i64.reinterpret_f64
+    i64.const 0
+    i64.xor
+    drop
+    drop
+    drop
+    drop
+    drop
+    drop
+    f32.const 0
+    f64.const 0
+  )
+)
+;; wasm[0]::function[0]:
+;;       pushq   %rbp
+;;       movq    %rsp, %rbp
+;;       movq    8(%rdi), %r11
+;;       movq    (%r11), %r11
+;;       addq    $0x50, %r11
+;;       cmpq    %rsp, %r11
+;;       ja      0x101
+;;   1b: movq    %rdi, %r14
+;;       subq    $0x38, %rsp
+;;       movq    %rdi, 0x30(%rsp)
+;;       movq    %rsi, 0x28(%rsp)
+;;       movsd   %xmm0, 0x20(%rsp)
+;;       movsd   %xmm1, 0x18(%rsp)
+;;       movsd   %xmm2, 0x10(%rsp)
+;;       movsd   %xmm3, 8(%rsp)
+;;       movq    %rdx, (%rsp)
+;;       movl    $0, %eax
+;;       movsd   0x20(%rsp), %xmm15
+;;       subq    $8, %rsp
+;;       movsd   %xmm15, (%rsp)
+;;       movsd   0x28(%rsp), %xmm15
+;;       subq    $8, %rsp
+;;       movsd   %xmm15, (%rsp)
+;;       movsd   0x30(%rsp), %xmm15
+;;       subq    $8, %rsp
+;;       movsd   %xmm15, (%rsp)
+;;       movsd   (%rsp), %xmm0
+;;       addq    $8, %rsp
+;;       subq    $4, %rsp
+;;       movss   0x72(%rip), %xmm15
+;;       movss   %xmm15, (%rsp)
+;;       testl   %eax, %eax
+;;       je      0xb6
+;;   a4: movl    (%rsp), %r11d
+;;       movl    %r11d, 0x10(%rsp)
+;;       addq    $0x10, %rsp
+;;       jmp     0xeb
+;;   b6: addq    $4, %rsp
+;;       movsd   (%rsp), %xmm0
+;;       addq    $8, %rsp
+;;       movq    %xmm0, %rax
+;;       xorq    $0, %rax
+;;       addq    $8, %rsp
+;;       movsd   0x38(%rip), %xmm0
+;;       subq    $4, %rsp
+;;       movss   0x23(%rip), %xmm15
+;;       movss   %xmm15, (%rsp)
+;;       movq    4(%rsp), %rax
+;;       movl    (%rsp), %r11d
+;;       addq    $4, %rsp
+;;       movl    %r11d, (%rax)
+;;       addq    $0x38, %rsp
+;;       popq    %rbp
+;;       retq
+;;  101: ud2
+;;  103: addb    %al, (%rax)
+;;  105: addb    %al, (%rax)
+;;  107: addb    %al, (%rax)
+;;  109: addb    %al, (%rax)
+;;  10b: addb    %al, (%rax)
+;;  10d: addb    %al, (%rax)
+;;  10f: addb    %al, (%rax)
+;;  111: addb    %al, (%rax)
+;;  113: addb    %al, (%rax)
+;;  115: addb    %al, (%rax)