Bug 1595466 - Fix multi-value shuffle of stack values toward SP r=lth
authorAndy Wingo <wingo@igalia.com>
Tue, 12 Nov 2019 17:49:10 +0000
changeset 501720 4424f41acdb04b53e7efcf8d64fc9dd1a2c53275
parent 501719 e711f304a2cd05c11e7c8e0231b75be4a70e7493
child 501721 96695741c87a1557ed6122a5606fa22cc43408fb
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslth
bugs1595466
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1595466 - Fix multi-value shuffle of stack values toward SP r=lth This commit fixes a bug whereby shuffling of stack results toward the stack pointer was borked. Just for posterity, here's a wee ASCII art of how this works in the baseline compiler. The error came in the last transition, when shuffling B toward the newly-expanded SP. The previous code was simply bogus. ``` initial (i32.const A) stack --> (local.get B) -> popRegisterResults -> popStackResults state (local.get C) _ | | | | | | | | | | | | | stack | | | | | | | | | height | | | | | | | | | | | | | | | | | sp+-+ v +-+ +-+ +-+ | |B| |A| | +-+ +-+ | |B| | +-+ | V nothing pushed C in %rax; B shuffled toward stack on machine stack; A still on %rsp; A materialized grows 3 values on value value stack down stack ``` Differential Revision: https://phabricator.services.mozilla.com/D52676
js/src/jit-test/tests/wasm/multi-value/block-run.js
js/src/wasm/WasmBaselineCompile.cpp
--- a/js/src/jit-test/tests/wasm/multi-value/block-run.js
+++ b/js/src/jit-test/tests/wasm/multi-value/block-run.js
@@ -221,8 +221,21 @@ wasmFullPass(`
         (local.tee $i)
         (i32.add) ;; sum = i + sum
         (i32.sub (local.get $i) (i32.const 1))
         (i32.eqz (local.tee $i))
         (if (param i32) (result i32)
             (then)
             (else (local.get $i) (br $loop))))))`,
             55);
+
+wasmFullPass(`
+  (module
+    (func (export "run") (result i32)
+      (local i32)
+      i32.const 42
+      local.get 0
+      local.get 0
+      loop (param i32 i32 i32) (result i32)
+        drop
+        drop
+      end))`,
+             42);
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -1837,27 +1837,27 @@ class BaseStackFrame final : public Base
   }
 
   void shuffleStackResultsTowardSP(uint32_t srcHeight, uint32_t destHeight,
                                    uint32_t bytes, Register temp) {
     MOZ_ASSERT(destHeight > srcHeight);
     MOZ_ASSERT(bytes % sizeof(uint32_t) == 0);
     uint32_t destOffset = stackOffset(destHeight);
     uint32_t srcOffset = stackOffset(srcHeight);
-    MOZ_ASSERT(destOffset >= bytes);
-    MOZ_ASSERT(srcOffset >= bytes);
     while (bytes >= sizeof(intptr_t)) {
-      masm.loadPtr(Address(sp_, srcOffset - bytes), temp);
-      masm.storePtr(temp, Address(sp_, destOffset - bytes));
+      masm.loadPtr(Address(sp_, srcOffset), temp);
+      masm.storePtr(temp, Address(sp_, destOffset));
+      destOffset += sizeof(intptr_t);
+      srcOffset += sizeof(intptr_t);
       bytes -= sizeof(intptr_t);
     }
     if (bytes) {
       MOZ_ASSERT(bytes == sizeof(uint32_t));
-      masm.load32(Address(sp_, srcOffset - bytes), temp);
-      masm.store32(temp, Address(sp_, destOffset - bytes));
+      masm.load32(Address(sp_, srcOffset), temp);
+      masm.store32(temp, Address(sp_, destOffset));
     }
   }
 
   void storeImmediateToStack(int32_t imm, uint32_t destHeight, Register temp) {
     masm.move32(Imm32(imm), temp);
     masm.store32(temp, Address(sp_, stackOffset(destHeight)));
   }