[Liftoff] Fix caller frame slots generated from stack values

Because of missing parentheses, the computation of the "half index" was
wrong, and always produced 0 or 1.
Also, for non-pairs, we were still passing kHighWord for the
RegPairHalf.

R=ahaas@chromium.org

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

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

@@ -536,7 +536,8 @@ void LiftoffAssembler::PushCallerFrameSlot(const VarState& src,
   switch (src.loc()) {
     case VarState::kStack:
       DCHECK_NE(kWasmF64, src.type());  // TODO(clemensh): Implement this.
-      push(liftoff::GetHalfStackSlot(2 * src_index + half == kLowWord ? 0 : 1));
+      push(liftoff::GetHalfStackSlot(2 * src_index +
+                                     (half == kLowWord ? 0 : 1)));
       break;
     case VarState::kRegister:
       PushCallerFrameSlot(

src/wasm/baseline/liftoff-assembler.cc

@@ -481,33 +481,33 @@ void LiftoffAssembler::PrepareCall(wasm::FunctionSig* sig,
   uint32_t param_base = cache_state_.stack_height() - num_params;
   uint32_t call_desc_input_idx = static_cast<uint32_t>(call_desc->InputCount());
   for (uint32_t i = num_params; i > 0; --i) {
-    uint32_t param = i - 1;
+    const uint32_t param = i - 1;
     ValueType type = sig->GetParam(param);
-    const int num_lowered_params = kNeedI64RegPair && type == kWasmI64 ? 2 : 1;
-    uint32_t stack_idx = param_base + param;
+    const bool is_pair = kNeedI64RegPair && type == kWasmI64;
+    const int num_lowered_params = is_pair ? 2 : 1;
+    const uint32_t stack_idx = param_base + param;
     const VarState& slot = cache_state_.stack_state[stack_idx];
     // Process both halfs of register pair separately, because they are passed
     // as separate parameters. One or both of them could end up on the stack.
     for (int lowered_idx = 0; lowered_idx < num_lowered_params; ++lowered_idx) {
+      const RegPairHalf half =
+          is_pair && lowered_idx == 0 ? kHighWord : kLowWord;
       --call_desc_input_idx;
       compiler::LinkageLocation loc =
           call_desc->GetInputLocation(call_desc_input_idx);
       if (loc.IsRegister()) {
         DCHECK(!loc.IsAnyRegister());
-        int reg_code = loc.AsRegister();
-        RegClass rc = num_lowered_params == 2 ? kGpReg : reg_class_for(type);
-        LiftoffRegister reg = LiftoffRegister::from_code(rc, reg_code);
+        RegClass rc = is_pair ? kGpReg : reg_class_for(type);
+        LiftoffRegister reg = LiftoffRegister::from_code(rc, loc.AsRegister());
         param_regs.set(reg);
-        if (num_lowered_params == 1) {
-          stack_transfers.LoadIntoRegister(reg, slot, stack_idx);
+        if (is_pair) {
+          stack_transfers.LoadI64HalfIntoRegister(reg, slot, stack_idx, half);
         } else {
-          stack_transfers.LoadI64HalfIntoRegister(
-              reg, slot, stack_idx, lowered_idx == 0 ? kHighWord : kLowWord);
+          stack_transfers.LoadIntoRegister(reg, slot, stack_idx);
         }
       } else {
         DCHECK(loc.IsCallerFrameSlot());
-        PushCallerFrameSlot(slot, stack_idx,
-                            lowered_idx == 0 ? kHighWord : kLowWord);
+        PushCallerFrameSlot(slot, stack_idx, half);
       }
     }
   }

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

+// Copyright 2018 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();
+sig = makeSig([kWasmI32, kWasmI32, kWasmI32, kWasmI32, kWasmI32], [kWasmI32]);
+builder.addFunction(undefined, sig).addBody([kExprGetLocal, 4]);
+builder.addMemory(16, 32);
+builder.addFunction('main', sig)
+    .addBody([
+      kExprI32Const, 0, kExprSetLocal, 0,
+      // Compute five arguments to the function call.
+      kExprI32Const, 0, kExprI32Const, 0, kExprI32Const, 0, kExprI32Const, 0,
+      kExprGetLocal, 4, kExprI32Const, 1, kExprI32Add,
+      // Now some intermediate computation to force the arguments to be spilled
+      // to the stack:
+      kExprGetLocal, 0, kExprI32Const, 1, kExprI32Add, kExprGetLocal, 1,
+      kExprGetLocal, 1, kExprI32Add, kExprI32Add, kExprDrop,
+      // Now call the function.
+      kExprCallFunction, 0
+    ])
+    .exportFunc();
+var instance = builder.instantiate();
+assertEquals(11, instance.exports.main(2, 4, 6, 8, 10));