[wasm] Fix interpreter entry for i64 return type

src/compiler/wasm-compiler.cc

Index: src/compiler/wasm-compiler.cc
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
index 92f389261bdd822118fd1df271a2932dd1011ab7..1af084e044daffafa2c1081b9d52de5df09dfd12 100644
--- a/src/compiler/wasm-compiler.cc
+++ b/src/compiler/wasm-compiler.cc
@@ -2973,12 +2973,13 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
   // 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_ =
@@ -3020,26 +3021,23 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
                      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);
   }
 }