[Liftoff] Fix binop code generation bug

If the destination register of a binop is the same register as the
right hand side, we would first move the left hand side into that
register (overwriting the value of the rhs), and then use the rhs.
This CL fixes this issue and adds a regression test.

R=ahaas@chromium.org

Bug: v8:6600, v8:7033
Change-Id: Ief90b5bcffc65823037bc57fb00741b2448e6375
Reviewed-on: https://chromium-review.googlesource.com/753462
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49137}

src/wasm/baseline/ia32/liftoff-assembler-ia32.h

@@ -42,10 +42,6 @@ void LiftoffAssembler::LoadConstant(Register reg, WasmValue value) {
   }
 }
 
-void LiftoffAssembler::MoveToReturnRegister(Register reg) {
-  if (reg != eax) mov(eax, reg);
-}
-
 void LiftoffAssembler::Load(Register dst, Address addr,
                             RelocInfo::Mode reloc_mode) {
   mov(dst, Operand(reinterpret_cast<uint32_t>(addr), reloc_mode));
@@ -76,6 +72,10 @@ void LiftoffAssembler::MoveStackValue(uint32_t dst_index, uint32_t src_index,
   }
 }
 
+void LiftoffAssembler::MoveToReturnRegister(Register reg) {
+  if (reg != eax) mov(eax, reg);
+}
+
 void LiftoffAssembler::Spill(uint32_t index, Register reg) {
   // TODO(clemensh): Handle different types here.
   mov(liftoff::GetStackSlot(index), reg);
@@ -92,28 +92,39 @@ void LiftoffAssembler::Fill(Register reg, uint32_t index) {
 }
 
 void LiftoffAssembler::emit_i32_add(Register dst, Register lhs, Register rhs) {
-  if (lhs.code() != dst.code()) {
+  if (lhs != dst) {
     lea(dst, Operand(lhs, rhs, times_1, 0));
   } else {
     add(dst, rhs);
   }
 }
 
-#define DEFAULT_I32_BINOP(name, internal_name)                       \
+void LiftoffAssembler::emit_i32_sub(Register dst, Register lhs, Register rhs) {
+  if (dst == rhs) {
+    neg(dst);
+    add(dst, lhs);
+  } else {
+    if (dst != lhs) mov(dst, lhs);
+    sub(dst, rhs);
+  }
+}
+
+#define COMMUTATIVE_I32_BINOP(name, instruction)                     \
   void LiftoffAssembler::emit_i32_##name(Register dst, Register lhs, \
                                          Register rhs) {             \
-    if (lhs.code() != dst.code()) {                                  \
-      mov(dst, lhs);                                                 \
+    if (dst == rhs) {                                                \
+      instruction(dst, lhs);                                         \
+    } else {                                                         \
+      if (dst != lhs) mov(dst, lhs);                                 \
+      instruction(dst, rhs);                                         \
     }                                                                \
-    internal_name(dst, rhs);                                         \
   }
 
 // clang-format off
-DEFAULT_I32_BINOP(sub, sub)
-DEFAULT_I32_BINOP(mul, imul)
-DEFAULT_I32_BINOP(and, and_)
-DEFAULT_I32_BINOP(or, or_)
-DEFAULT_I32_BINOP(xor, xor_)
+COMMUTATIVE_I32_BINOP(mul, imul)
+COMMUTATIVE_I32_BINOP(and, and_)
+COMMUTATIVE_I32_BINOP(or, or_)
+COMMUTATIVE_I32_BINOP(xor, xor_)
 // clang-format on
 
 #undef DEFAULT_I32_BINOP

src/wasm/baseline/liftoff-compiler.cc

@@ -239,7 +239,7 @@ class LiftoffCompiler {
     Register rhs_reg = pinned_regs.pin(__ PopToRegister(kWasmI32, pinned_regs));
     Register lhs_reg = __ PopToRegister(kWasmI32, pinned_regs);
     (asm_->*emit_fn)(target_reg, lhs_reg, rhs_reg);
-    __ PushRegister(std::move(target_reg));
+    __ PushRegister(target_reg);
   }
 
   void I32Const(Decoder* decoder, Value* result, int32_t value) {

src/wasm/baseline/x64/liftoff-assembler-x64.h

@@ -98,28 +98,39 @@ void LiftoffAssembler::Fill(Register reg, uint32_t index) {
 }
 
 void LiftoffAssembler::emit_i32_add(Register dst, Register lhs, Register rhs) {
-  if (lhs.code() != dst.code()) {
+  if (lhs != dst) {
     leal(dst, Operand(lhs, rhs, times_1, 0));
   } else {
     addl(dst, rhs);
   }
 }
 
-#define DEFAULT_I32_BINOP(name, internal_name)                       \
+void LiftoffAssembler::emit_i32_sub(Register dst, Register lhs, Register rhs) {
+  if (dst == rhs) {
+    negl(dst);
+    addl(dst, lhs);
+  } else {
+    if (dst != lhs) movl(dst, lhs);
+    subl(dst, rhs);
+  }
+}
+
+#define COMMUTATIVE_I32_BINOP(name, instruction)                     \
   void LiftoffAssembler::emit_i32_##name(Register dst, Register lhs, \
                                          Register rhs) {             \
-    if (lhs.code() != dst.code()) {                                  \
-      movl(dst, lhs);                                                \
+    if (dst == rhs) {                                                \
+      instruction##l(dst, lhs);                                      \
+    } else {                                                         \
+      if (dst != lhs) movl(dst, lhs);                                \
+      instruction##l(dst, rhs);                                      \
     }                                                                \
-    internal_name##l(dst, rhs);                                      \
   }
 
 // clang-format off
-DEFAULT_I32_BINOP(sub, sub)
-DEFAULT_I32_BINOP(mul, imul)
-DEFAULT_I32_BINOP(and, and)
-DEFAULT_I32_BINOP(or, or)
-DEFAULT_I32_BINOP(xor, xor)
+COMMUTATIVE_I32_BINOP(mul, imul)
+COMMUTATIVE_I32_BINOP(and, and)
+COMMUTATIVE_I32_BINOP(or, or)
+COMMUTATIVE_I32_BINOP(xor, xor)
 // clang-format on
 
 #undef DEFAULT_I32_BINOP

test/mjsunit/regress/wasm/regress-7033.js

+// Copyright 2017 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-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+var builder = new WasmModuleBuilder();
+builder.addFunction('test', kSig_i_iii)
+    .addBodyWithEnd([
+      kExprI32Const, 0x07,  // i32.const 7
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprI32And,          // i32.and
+      kExprI32And,          // i32.and
+      kExprEnd,             // -
+    ])
+    .exportFunc();
+var module = builder.instantiate();
+assertEquals(0, module.exports.test());