[liftoff] Handle unordered register pairs

backes · Commit Bot · commit b429b8f92488 · 2020-07-09T11:05:08.000Z
For 64-bit binary operations, Liftoff on arm made the assumption that register pairs are always ordered, i.e. the register code for the low word is lower than the register code for the high word. Ensuring this was only implemented in {GetUnusedRegister} in https://crrev.com/c/2168875. Other cases were missing though, e.g. return values, but also different places were we construct register pairs internally. Thus, this CL removes this constraint again and instead handles unordered register pairs in 64-bit binary operations on arm. R=thibaudm@chromium.org Bug: chromium:1101304 Change-Id: I4cd9fb1577f82ab06d34c9dde6533cf04a2cade7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2287870 Reviewed-by: Thibaud Michaud <thibaudm@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#68752}
diff --git a/src/wasm/baseline/arm/liftoff-assembler-arm.h b/src/wasm/baseline/arm/liftoff-assembler-arm.h
@@ -120,11 +120,16 @@ template <void (Assembler::*op)(Register, Register, Register, SBit, Condition),
                                            SBit, Condition)>
 inline void I64Binop(LiftoffAssembler* assm, LiftoffRegister dst,
                      LiftoffRegister lhs, LiftoffRegister rhs) {
-  DCHECK_NE(dst.low_gp(), lhs.high_gp());
-  DCHECK_NE(dst.low_gp(), rhs.high_gp());
-  (assm->*op)(dst.low_gp(), lhs.low_gp(), rhs.low_gp(), SetCC, al);
+  Register dst_low = dst.low_gp();
+  if (dst_low == lhs.high_gp() || dst_low == rhs.high_gp()) {
+    dst_low = assm->GetUnusedRegister(
+                      kGpReg, LiftoffRegList::ForRegs(lhs, rhs, dst.high_gp()))
+                  .gp();
+  }
+  (assm->*op)(dst_low, lhs.low_gp(), rhs.low_gp(), SetCC, al);
   (assm->*op_with_carry)(dst.high_gp(), lhs.high_gp(), Operand(rhs.high_gp()),
                          LeaveCC, al);
+  if (dst_low != dst.low_gp()) assm->mov(dst.low_gp(), dst_low);
 }
 
 template <void (Assembler::*op)(Register, Register, const Operand&, SBit,
diff --git a/src/wasm/baseline/liftoff-assembler.h b/src/wasm/baseline/liftoff-assembler.h
@@ -371,15 +371,6 @@ class LiftoffAssembler : public TurboAssembler {
       LiftoffRegList candidates = kGpCacheRegList;
       Register low = pinned.set(GetUnusedRegister(candidates, pinned)).gp();
       Register high = GetUnusedRegister(candidates, pinned).gp();
-      if (low.code() > high.code()) {
-        // Establish the invariant that the register of the low word always has
-        // a lower code than the register of the high word. This guarantees that
-        // if a register pair of an input is reused for the result, the low
-        // word and high word registers are not swapped, i.e. the low word
-        // register of the result is not the high word register of the input,
-        // and vice versa.
-        std::swap(low, high);
-      }
       return LiftoffRegister::ForPair(low, high);
     } else if (kNeedS128RegPair && rc == kFpRegPair) {
       // kFpRegPair specific logic here because we need adjacent registers, not
diff --git a/test/mjsunit/regress/wasm/regress-1101304.js b/test/mjsunit/regress/wasm/regress-1101304.js
@@ -0,0 +1,29 @@
+// 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.
+
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+builder.addType(makeSig(
+    [kWasmI64, kWasmI32, kWasmF64, kWasmI64, kWasmI64, kWasmI32, kWasmI64],
+    []));
+builder.addFunction(undefined, 0 /* sig */).addBody([
+  kExprI32Const,    0,          // i32.const
+  kExprIf,          kWasmStmt,  // if @3
+  kExprI32Const,    1,          // i32.const
+  kExprIf,          kWasmStmt,  // if @7
+  kExprNop,                     // nop
+  kExprElse,                    // else @10
+  kExprUnreachable,             // unreachable
+  kExprEnd,                     // end @12
+  kExprLocalGet,    0x06,       // local.get
+  kExprLocalSet,    0x00,       // local.set
+  kExprLocalGet,    0x03,       // local.get
+  kExprLocalGet,    0x00,       // local.get
+  kExprI64Sub,                  // i64.sub
+  kExprDrop,                    // drop
+  kExprUnreachable,             // unreachable
+  kExprEnd                      // end @24
+]);
+builder.toModule();
