[wasm] Fix interpreter entry for i64 return type

Fix two issues in the interpreter entry for 64 bit return values on
32 bit platforms. First, the effect chain was slightly incorrect, second
the order of the returned values was wrong.

Also add a test case for this.
Tested on x64, ia32 and s390.

Plus drive-by fix in Int64Lowering to reuse global constants for
big-endian/little-endian disambiguation.

R=titzer@chromium.org
BUG=v8:5822

Review-Url: https://codereview.chromium.org/2731713002
Cr-Commit-Position: refs/heads/master@{#43654}

src/compiler/int64-lowering.cc

@@ -74,6 +74,8 @@ void Int64Lowering::LowerGraph() {
   }
 }
 
+namespace {
+
 static int GetParameterIndexAfterLowering(
     Signature<MachineRepresentation>* signature, int old_index) {
   int result = old_index;
@@ -85,6 +87,19 @@ static int GetParameterIndexAfterLowering(
   return result;
 }
 
+int GetReturnCountAfterLowering(Signature<MachineRepresentation>* signature) {
+  int result = static_cast<int>(signature->return_count());
+  for (int i = 0; i < static_cast<int>(signature->return_count()); i++) {
+    if (signature->GetReturn(i) == MachineRepresentation::kWord64) {
+      result++;
+    }
+  }
+  return result;
+}
+
+}  // namespace
+
+// static
 int Int64Lowering::GetParameterCountAfterLowering(
     Signature<MachineRepresentation>* signature) {
   // GetParameterIndexAfterLowering(parameter_count) returns the parameter count
@@ -93,15 +108,10 @@ int Int64Lowering::GetParameterCountAfterLowering(
       signature, static_cast<int>(signature->parameter_count()));
 }
 
-static int GetReturnCountAfterLowering(
-    Signature<MachineRepresentation>* signature) {
-  int result = static_cast<int>(signature->return_count());
-  for (int i = 0; i < static_cast<int>(signature->return_count()); i++) {
-    if (signature->GetReturn(i) == MachineRepresentation::kWord64) {
-      result++;
-    }
-  }
-  return result;
+// static
+bool Int64Lowering::IsI64AsTwoParameters(MachineOperatorBuilder* machine,
+                                         MachineRepresentation type) {
+  return machine->Is32() && type == MachineRepresentation::kWord64;
 }
 
 void Int64Lowering::GetIndexNodes(Node* index, Node*& index_low,
@@ -120,14 +130,6 @@ void Int64Lowering::GetIndexNodes(Node* index, Node*& index_low,
 #endif
 }
 
-#if defined(V8_TARGET_LITTLE_ENDIAN)
-const int Int64Lowering::kLowerWordOffset = 0;
-const int Int64Lowering::kHigherWordOffset = 4;
-#elif defined(V8_TARGET_BIG_ENDIAN)
-const int Int64Lowering::kLowerWordOffset = 4;
-const int Int64Lowering::kHigherWordOffset = 0;
-#endif
-
 void Int64Lowering::LowerNode(Node* node) {
   switch (node->opcode()) {
     case IrOpcode::kInt64Constant: {
@@ -561,7 +563,8 @@ void Int64Lowering::LowerNode(Node* node) {
               StoreRepresentation(MachineRepresentation::kWord32,
                                   WriteBarrierKind::kNoWriteBarrier)),
           stack_slot,
-          graph()->NewNode(common()->Int32Constant(kHigherWordOffset)),
+          graph()->NewNode(
+              common()->Int32Constant(kInt64UpperHalfMemoryOffset)),
           GetReplacementHigh(input), graph()->start(), graph()->start());
 
       Node* store_low_word = graph()->NewNode(
@@ -569,7 +572,8 @@ void Int64Lowering::LowerNode(Node* node) {
               StoreRepresentation(MachineRepresentation::kWord32,
                                   WriteBarrierKind::kNoWriteBarrier)),
           stack_slot,
-          graph()->NewNode(common()->Int32Constant(kLowerWordOffset)),
+          graph()->NewNode(
+              common()->Int32Constant(kInt64LowerHalfMemoryOffset)),
           GetReplacementLow(input), store_high_word, graph()->start());
 
       Node* load =
@@ -597,13 +601,15 @@ void Int64Lowering::LowerNode(Node* node) {
 
       Node* high_node = graph()->NewNode(
           machine()->Load(MachineType::Int32()), stack_slot,
-          graph()->NewNode(common()->Int32Constant(kHigherWordOffset)), store,
-          graph()->start());
+          graph()->NewNode(
+              common()->Int32Constant(kInt64UpperHalfMemoryOffset)),
+          store, graph()->start());
 
       Node* low_node = graph()->NewNode(
           machine()->Load(MachineType::Int32()), stack_slot,
-          graph()->NewNode(common()->Int32Constant(kLowerWordOffset)), store,
-          graph()->start());
+          graph()->NewNode(
+              common()->Int32Constant(kInt64LowerHalfMemoryOffset)),
+          store, graph()->start());
       ReplaceNode(node, low_node, high_node);
       break;
     }

src/compiler/int64-lowering.h

@@ -27,8 +27,10 @@ class V8_EXPORT_PRIVATE Int64Lowering {
   static int GetParameterCountAfterLowering(
       Signature<MachineRepresentation>* signature);
 
-  static const int kLowerWordOffset;
-  static const int kHigherWordOffset;
+  // Determine whether the given type is i64 and has to be passed via two
+  // parameters on the given machine.
+  static bool IsI64AsTwoParameters(MachineOperatorBuilder* machine,
+                                   MachineRepresentation type);
 
  private:
   enum class State : uint8_t { kUnvisited, kOnStack, kVisited };

src/compiler/wasm-compiler.cc

@@ -2971,12 +2971,11 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
   // Now store all our arguments to the buffer.
   int param_index = 0;
   int offset = 0;
+
   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 (Int64Lowering::IsI64AsTwoParameters(jsgraph()->machine(),
+                                            sig->GetParam(i))) {
       StoreRepresentation store_rep(wasm::kWasmI32,
                                     WriteBarrierKind::kNoWriteBarrier);
       *effect_ =
@@ -3018,26 +3017,24 @@ 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 (Int64Lowering::IsI64AsTwoParameters(jsgraph()->machine(),
+                                                 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(sizeof(int32_t)), lower, *control_);
+    *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);
   }
 }

test/cctest/wasm/test-wasm-interpreter-entry.cc

@@ -87,6 +87,7 @@ static ArgPassingHelper<T> GetHelper(
 
 }  // namespace
 
+// Pass int32_t, return int32_t.
 TEST(TestArgumentPassing_int32) {
   WasmRunner<int32_t, int32_t> runner(kExecuteCompiled);
   WasmFunctionCompiler& f2 = runner.NewFunction<int32_t, int32_t>();
@@ -103,7 +104,8 @@ TEST(TestArgumentPassing_int32) {
   for (int32_t v : test_values) helper.CheckCall(v);
 }
 
-TEST(TestArgumentPassing_int64) {
+// Pass int64_t, return double.
+TEST(TestArgumentPassing_double_int64) {
   WasmRunner<double, int32_t, int32_t> runner(kExecuteCompiled);
   WasmFunctionCompiler& f2 = runner.NewFunction<double, int64_t>();
 
@@ -138,6 +140,27 @@ TEST(TestArgumentPassing_int64) {
   }
 }
 
+// Pass double, return int64_t.
+TEST(TestArgumentPassing_int64_double) {
+  // Outer function still returns double.
+  WasmRunner<double, double> runner(kExecuteCompiled);
+  WasmFunctionCompiler& f2 = runner.NewFunction<int64_t, double>();
+
+  auto helper = GetHelper(
+      runner, f2,
+      {// Return (int64_t)<0>.
+       WASM_I64_SCONVERT_F64(WASM_GET_LOCAL(0))},
+      {// Call f2 with param <0>, convert returned value back to double.
+       WASM_F64_SCONVERT_I64(WASM_SEQ(
+           WASM_GET_LOCAL(0), WASM_CALL_FUNCTION0(f2.function_index())))},
+      [](double d) { return d; });
+
+  for (int64_t i : compiler::ValueHelper::int64_vector()) {
+    helper.CheckCall(i);
+  }
+}
+
+// Pass float, return double.
 TEST(TestArgumentPassing_float_double) {
   WasmRunner<double, float> runner(kExecuteCompiled);
   WasmFunctionCompiler& f2 = runner.NewFunction<double, float>();
@@ -156,6 +179,7 @@ TEST(TestArgumentPassing_float_double) {
   for (float f : test_values) helper.CheckCall(f);
 }
 
+// Pass two doubles, return double.
 TEST(TestArgumentPassing_double_double) {
   WasmRunner<double, double, double> runner(kExecuteCompiled);
   WasmFunctionCompiler& f2 = runner.NewFunction<double, double, double>();
@@ -176,11 +200,13 @@ TEST(TestArgumentPassing_double_double) {
   }
 }
 
+// Pass int32_t, int64_t, float and double, return double.
 TEST(TestArgumentPassing_AllTypes) {
   // The second and third argument will be combined to an i64.
-  WasmRunner<double, int, int, int, float, double> runner(kExecuteCompiled);
+  WasmRunner<double, int32_t, int32_t, int32_t, float, double> runner(
+      kExecuteCompiled);
   WasmFunctionCompiler& f2 =
-      runner.NewFunction<double, int, int64_t, float, double>();
+      runner.NewFunction<double, int32_t, int64_t, float, double>();
 
   auto helper = GetHelper(
       runner, f2,

test/unittests/compiler/int64-lowering-unittest.cc

@@ -610,12 +610,12 @@ TEST_F(Int64LoweringTest, F64ReinterpretI64) {
       IsStore(StoreRepresentation(MachineRepresentation::kWord32,
                                   WriteBarrierKind::kNoWriteBarrier),
               AllOf(CaptureEq(&stack_slot_capture), stack_slot_matcher),
-              IsInt32Constant(Int64Lowering::kLowerWordOffset),
+              IsInt32Constant(kInt64LowerHalfMemoryOffset),
               IsInt32Constant(low_word_value(0)),
               IsStore(StoreRepresentation(MachineRepresentation::kWord32,
                                           WriteBarrierKind::kNoWriteBarrier),
                       AllOf(CaptureEq(&stack_slot_capture), stack_slot_matcher),
-                      IsInt32Constant(Int64Lowering::kHigherWordOffset),
+                      IsInt32Constant(kInt64UpperHalfMemoryOffset),
                       IsInt32Constant(high_word_value(0)), start(), start()),
               start());
 
@@ -647,11 +647,11 @@ TEST_F(Int64LoweringTest, I64ReinterpretF64) {
       graph()->end()->InputAt(1),
       IsReturn2(IsLoad(MachineType::Int32(),
                        AllOf(CaptureEq(&stack_slot), stack_slot_matcher),
-                       IsInt32Constant(Int64Lowering::kLowerWordOffset),
+                       IsInt32Constant(kInt64LowerHalfMemoryOffset),
                        AllOf(CaptureEq(&store), store_matcher), start()),
                 IsLoad(MachineType::Int32(),
                        AllOf(CaptureEq(&stack_slot), stack_slot_matcher),
-                       IsInt32Constant(Int64Lowering::kHigherWordOffset),
+                       IsInt32Constant(kInt64UpperHalfMemoryOffset),
                        AllOf(CaptureEq(&store), store_matcher), start()),
                 start(), start()));
 }