[liftoff][mv] Fix merge issue in multi-value loops

thibaudmichaud · Commit Bot · commit 9d063694963c · 2020-05-19T15:43:50.000Z
Registers cannot be used as a merge destination if they have more than one use, otherwise the merge will unexpectedly affect other uses of that register. R=ahaas@chromium.org,clemensb@chromium.org Bug: chromium:1084151 Change-Id: I0d6ad97c585920357a37d95361e0320d32c71f4b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2208851 Reviewed-by: Clemens Backes <clemensb@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Commit-Queue: Thibaud Michaud <thibaudm@chromium.org> Cr-Commit-Position: refs/heads/master@{#67904}
diff --git a/src/wasm/baseline/liftoff-assembler.cc b/src/wasm/baseline/liftoff-assembler.cc
@@ -571,9 +571,22 @@ LiftoffRegister LiftoffAssembler::PeekToRegister(int index,
 void LiftoffAssembler::PrepareLoopArgs(int num) {
   for (int i = 0; i < num; ++i) {
     VarState& slot = cache_state_.stack_state.end()[-1 - i];
-    if (!slot.is_const()) continue;
-    RegClass rc =
-        kNeedI64RegPair && slot.type() == kWasmI64 ? kGpRegPair : kGpReg;
+    if (slot.is_stack()) continue;
+    RegClass rc = reg_class_for(slot.type());
+    if (slot.is_reg()) {
+      if (cache_state_.get_use_count(slot.reg()) > 1) {
+        // If the register is used more than once, we cannot use it for the
+        // merge. Move it to an unused register instead.
+        LiftoffRegList pinned;
+        pinned.set(slot.reg());
+        LiftoffRegister dst_reg = GetUnusedRegister(rc, pinned);
+        Move(dst_reg, slot.reg(), slot.type());
+        cache_state_.dec_used(slot.reg());
+        cache_state_.inc_used(dst_reg);
+        slot.MakeRegister(dst_reg);
+      }
+      continue;
+    }
     LiftoffRegister reg = GetUnusedRegister(rc, {});
     LoadConstant(reg, slot.constant());
     slot.MakeRegister(reg);
diff --git a/test/mjsunit/regress/wasm/regress-1084151.js b/test/mjsunit/regress/wasm/regress-1084151.js
@@ -0,0 +1,31 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --experimental-wasm-mv
+
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+let builder = new WasmModuleBuilder();
+let sig_i_iii = builder.addType(kSig_i_iii);
+
+builder.addFunction("main", sig_i_iii)
+  .addBody([
+    kExprLocalGet, 1,
+    kExprLocalGet, 1,
+    kExprI32Const, 5,
+    kExprLoop, sig_i_iii,
+      kExprLocalGet, 1,
+      kExprBlock, sig_i_iii,
+        kExprLocalGet, 1,
+        kExprLocalGet, 2,
+        kExprBrIf, 1,
+        kExprDrop,
+        kExprDrop,
+        kExprDrop,
+      kExprEnd,
+      kExprDrop,
+    kExprEnd])
+  .exportAs("main");
+
+let module = new WebAssembly.Module(builder.toBuffer());
