[Liftoff] Fix register reuse in merge init

When initializing the cache state for a merge, we should never use
registers multiple times. Other code paths leading to the same merge
point might provide different values for the different slots there.

R=ahaas@chromium.org

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

src/wasm/baseline/liftoff-assembler.cc

@@ -136,43 +136,33 @@ void LiftoffAssembler::CacheState::InitMerge(const CacheState& source,
   stack_state.resize(stack_base + arity);
   auto slot = stack_state.begin();
 
-  // Compute list of all registers holding local values.
-  PinnedRegisterScope locals_regs;
-  for (auto local_it = source.stack_state.begin(),
-            local_end = source.stack_state.begin() + num_locals;
-       local_it != local_end; ++local_it) {
-    if (local_it->is_reg()) locals_regs.pin(local_it->reg);
-  }
-
-  RegList used_regs = 0;
-  auto InitStackSlot = [&](const VarState& src, bool needs_unique_reg) {
+  // TODO(clemensh): Avoid using registers which are already in use in source.
+  PinnedRegisterScope used_regs;
+  auto InitStackSlot = [this, &used_regs, &slot](const VarState& src) {
     Register reg = no_reg;
-    if (src.is_reg() &&
-        (!needs_unique_reg || (used_regs & src.reg.bit()) == 0)) {
+    if (src.is_reg() && !used_regs.has(src.reg)) {
       reg = src.reg;
-    } else if (has_unused_register(locals_regs)) {
-      reg = unused_register(locals_regs);
+    } else if (has_unused_register(used_regs)) {
+      reg = unused_register(used_regs);
     } else {
+      // Keep this a stack slot (which is the initial value).
       DCHECK(slot->is_stack());
       return;
     }
     *slot = VarState(reg);
     ++slot;
     inc_used(reg);
-    used_regs |= reg.bit();
+    used_regs.pin(reg);
   };
+
   auto source_slot = source.stack_state.begin();
-  // Ensure that locals do not share the same register.
-  for (uint32_t i = 0; i < num_locals; ++i, ++source_slot) {
-    InitStackSlot(*source_slot, true);
-  }
-  for (uint32_t i = num_locals; i < stack_base; ++i, ++source_slot) {
-    InitStackSlot(*source_slot, false);
+  for (uint32_t i = 0; i < stack_base; ++i, ++source_slot) {
+    InitStackSlot(*source_slot);
   }
   DCHECK_GE(source.stack_height(), stack_base + arity);
   source_slot = source.stack_state.end() - arity;
   for (uint32_t i = 0; i < arity; ++i, ++source_slot) {
-    InitStackSlot(*source_slot, true);
+    InitStackSlot(*source_slot);
   }
   DCHECK_EQ(slot, stack_state.end());
   last_spilled_reg = source.last_spilled_reg;

src/wasm/baseline/liftoff-assembler.h

@@ -57,6 +57,7 @@ class LiftoffAssembler : public TurboAssembler {
     }
 
     RegList pinned_regs() const { return pinned_regs_; }
+    bool has(Register reg) const { return (pinned_regs_ & reg.bit()) != 0; }
 
    private:
     RegList pinned_regs_ = 0;
@@ -102,6 +103,7 @@ class LiftoffAssembler : public TurboAssembler {
   static_assert(IS_TRIVIALLY_COPYABLE(VarState),
                 "VarState should be trivially copyable");
 
+  // TODO(clemensh): Make this a proper class.
   struct CacheState {
     MOVE_ONLY_WITH_DEFAULT_CONSTRUCTORS(CacheState);
 

test/mjsunit/regress/wasm/regress-7035.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, 0x00,  // i32.const 0
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprI32Add,          // i32.add -> 0
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprI32Add,          // i32.add -> 0
+      kExprI32Add,          // i32.add -> 0
+      kExprI32Const, 0x01,  // i32.const 1
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprI32Add,          // i32.add -> 1
+      kExprBlock,    0x7f,  // @39 i32
+      kExprI32Const, 0x00,  // i32.const 0
+      kExprBr,       0x00,  // depth=0
+      kExprEnd,             // @90
+      kExprI32Add,          // i32.add -> 1
+      kExprI32Add,          // i32.add -> 1
+      kExprEnd
+    ])
+    .exportFunc();
+var module = builder.instantiate();
+assertEquals(1, module.exports.test());