[wasm] Fix interpreter entry for i64 return type

src/compiler/wasm-compiler.cc

 // Copyright 2015 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.
 
 #include "src/compiler/wasm-compiler.h"
 
 #include <memory>
 
 #include "src/assembler-inl.h"
 #include "src/base/platform/elapsed-timer.h"
@@ skipping 2955 matching lines @@
 
   // Get a stack slot for the arguments.
   Node* arg_buffer = args_size_bytes == 0 && return_size_bytes == 0
                          ? jsgraph()->IntPtrConstant(0)
                          : graph()->NewNode(jsgraph()->machine()->StackSlot(
                                std::max(args_size_bytes, return_size_bytes)));
 
   // Now store all our arguments to the buffer.
   int param_index = 0;
   int offset = 0;
+  auto is_i64_as_two_params = [&](wasm::ValueType t) {

[titzer, 2017/03/07 10:21:55]

Can you extract this to a utility function in Int64Lowering?

[Clemens Hammacher, 2017/03/07 10:30:03]

Int64Lowering current does the lowering unconditionally. Adding the
machine()->Is32() check there would not fit the rest of this class.

[titzer, 2017/03/07 11:51:52]

What I meant is just a static function that takes a MachineOperatorBuilder and a
ValueType and returns true/false.

It might be useful elsewhere and it's probably best to put it in Int64Lowering.

+    return jsgraph()->machine()->Is32() && t == wasm::kWasmI64;
+  };
+
   for (int i = 0; i < wasm_count; i++) {
     Node* param = Param(param_index++);
-    bool is_i64_as_two_params =
-        jsgraph()->machine()->Is32() && sig->GetParam(i) == wasm::kWasmI64;
-
-    if (is_i64_as_two_params) {
+    if (is_i64_as_two_params(sig->GetParam(i))) {
       StoreRepresentation store_rep(wasm::kWasmI32,
                                     WriteBarrierKind::kNoWriteBarrier);
       *effect_ =
           graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
                            Int32Constant(offset + kInt64LowerHalfMemoryOffset),
                            param, *effect_, *control_);
 
       param = Param(param_index++);
       *effect_ =
           graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
@@ skipping 21 matching lines @@
   // call Smi::value on it, but just cast it to a byte pointer.
   Node* parameters[] = {
       jsgraph()->HeapConstant(instance),       // wasm instance
       jsgraph()->SmiConstant(function_index),  // function index
       arg_buffer,                              // argument buffer
   };
   BuildCallToRuntime(Runtime::kWasmRunInterpreter, jsgraph(), parameters,
                      arraysize(parameters), effect_, *control_);
 
   // Read back the return value.
-  if (jsgraph()->machine()->Is32() && sig->return_count() > 0 &&
-      sig->GetReturn() == wasm::kWasmI64) {
+  if (sig->return_count() == 0) {
+    Return(Int32Constant(0));
+  } else if (is_i64_as_two_params(sig->GetReturn())) {
     MachineType load_rep = wasm::WasmOpcodes::MachineTypeFor(wasm::kWasmI32);
     Node* lower =
         graph()->NewNode(jsgraph()->machine()->Load(load_rep), arg_buffer,
                          Int32Constant(0), *effect_, *control_);
     Node* upper =
         graph()->NewNode(jsgraph()->machine()->Load(load_rep), arg_buffer,
-                         Int32Constant(sizeof(int32_t)), *effect_, *control_);
-    Return(upper, lower);
+                         Int32Constant(4), lower, *control_);

[titzer, 2017/03/07 10:21:55]

Can you keep the sizeof(int32_t) for documentation here?

[Clemens Hammacher, 2017/03/07 10:30:03]

Done.

+    *effect_ = upper;
+    Return(lower, upper);
   } else {
-    Node* val;
-    if (sig->return_count() == 0) {
-      val = Int32Constant(0);
-    } else {
-      MachineType load_rep =
-          wasm::WasmOpcodes::MachineTypeFor(sig->GetReturn());
-      val = graph()->NewNode(jsgraph()->machine()->Load(load_rep), arg_buffer,
-                             Int32Constant(0), *effect_, *control_);
-    }
+    MachineType load_rep = wasm::WasmOpcodes::MachineTypeFor(sig->GetReturn());
+    Node* val =
+        graph()->NewNode(jsgraph()->machine()->Load(load_rep), arg_buffer,
+                         Int32Constant(0), *effect_, *control_);
     Return(val);
   }
 }
 
 Node* WasmGraphBuilder::MemBuffer(uint32_t offset) {
   DCHECK(module_ && module_->instance);
   if (offset == 0) {
     if (!mem_buffer_) {
       mem_buffer_ = jsgraph()->RelocatableIntPtrConstant(
           reinterpret_cast<uintptr_t>(module_->instance->mem_start),
@@ skipping 1184 matching lines @@
     wasm::ErrorThrower* thrower, Isolate* isolate,
     wasm::ModuleBytesEnv* module_env, const wasm::WasmFunction* function) {
   WasmCompilationUnit unit(isolate, module_env, function);
   unit.ExecuteCompilation();
   return unit.FinishCompilation(thrower);
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8