[wasm-simd][scalar-lowering] Fix more lowering of returns

Fix lowering of functions that returns the result of an
i8x16 or i16x8 operation.

Bug: v8:10507
Change-Id: Ia3b29e69cff7771f85dc5160937cbaf2bbc12b55
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2399049
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69832}

src/compiler/simd-scalar-lowering.cc

@@ -1308,23 +1308,39 @@ void SimdScalarLowering::LowerNode(Node* node) {
     }
     case IrOpcode::kReturn: {
       // V128 return types are lowered to i32x4, so if the inputs to kReturn are
-      // not Word32, we need to cast them.
+      // not Word32, we need to convert them.
       int return_arity = static_cast<int>(signature()->return_count());
       for (int i = 0; i < return_arity; i++) {
         if (signature()->GetReturn(i) != MachineRepresentation::kSimd128) {
           continue;
         }
-        // Input 1 of kReturn is not used.
+
+        // Return nodes have a hidden input at value 0.
         Node* input = node->InputAt(i + 1);
-        if (HasReplacement(0, input)) {
-          // f64x2 isn't handled anywhere yet, so we ignore it here for now.
-          Replacement rep = replacements_[input->id()];
-          if (rep.type == SimdType::kFloat32x4) {
-            Node** rep = GetReplacementsWithType(input, rep_type);
-            ReplaceNode(input, rep, NumLanes(rep_type));
+        if (!HasReplacement(0, input)) {
+          continue;
+        }
+
+        switch (ReplacementType(input)) {
+          case SimdType::kInt8x16:
+          case SimdType::kInt16x8:
+          case SimdType::kFloat32x4: {
+            Node** reps = GetReplacementsWithType(input, rep_type);
+            ReplaceNode(input, reps, NumLanes(rep_type));
+            break;
+          }
+          case SimdType::kInt32x4: {
+            // No action needed.
+            break;
+          }
+          default: {
+            // i64x2 and f64x2 aren't handled anywhere yet, ignore it here.
+            break;
+            UNIMPLEMENTED();
           }
         }
       }
+
       DefaultLowering(node);
       int new_return_count = GetReturnCountAfterLoweringSimd128(signature());
       if (static_cast<int>(signature()->return_count()) != new_return_count) {
@@ -1340,6 +1356,16 @@ void SimdScalarLowering::LowerNode(Node* node) {
           GetReturnCountAfterLoweringSimd128(call_descriptor) !=
           static_cast<int>(call_descriptor->ReturnCount());
 
+      // All call arguments are lowered to i32x4 in the call descriptor, so the
+      // arguments need to be converted to i32x4 as well.
+      for (int i = NodeProperties::PastValueIndex(node) - 1; i >= 0; i--) {
+        Node* input = node->InputAt(i);
+        if (HasReplacement(0, input)) {
+          Node** reps = GetReplacementsWithType(input, SimdType::kInt32x4);
+          ReplaceNode(input, reps, NumLanes(SimdType::kInt32x4));
+        }
+      }
+
       if (DefaultLowering(node) || returns_require_lowering) {
         // We have to adjust the call descriptor.
         const Operator* op = common()->Call(

test/cctest/wasm/test-run-wasm-simd-scalar-lowering.cc

@@ -43,13 +43,13 @@ WASM_SIMD_TEST(I8x16ToF32x4) {
   CHECK_EQ(expected, actual);
 }
 
-WASM_SIMD_TEST(F32x4) {
+WASM_SIMD_TEST(F32x4_Call_Return) {
   // Check that functions that return F32x4 are correctly lowered into 4 int32
   // nodes. The signature of such functions are always lowered to 4 Word32, and
   // if the last operation before the return was a f32x4, it will need to be
   // bitcasted from float to int.
   TestSignatures sigs;
-  WasmRunner<uint32_t, uint32_t> r(execution_tier, lower_simd);
+  WasmRunner<float, float> r(execution_tier, lower_simd);
 
   // A simple function that just calls f32x4.neg on the param.
   WasmFunctionCompiler& fn = r.NewFunction(sigs.s_s());
@@ -59,10 +59,48 @@ WASM_SIMD_TEST(F32x4) {
   // Use i32x4 splat since scalar lowering has a problem with f32x4 as a param
   // to a function call, the lowering is not correct yet.
   BUILD(r,
-        WASM_SIMD_I32x4_EXTRACT_LANE(
+        WASM_SIMD_F32x4_EXTRACT_LANE(
+            0, WASM_CALL_FUNCTION(fn.function_index(),
+                                  WASM_SIMD_F32x4_SPLAT(WASM_GET_LOCAL(0)))));
+  CHECK_EQ(-1.0, r.Call(1));
+}
+
+WASM_SIMD_TEST(I8x16_Call_Return) {
+  // Check that calling a function with i8x16 arguments, and returns i8x16, is
+  // correctly lowered. The signature of the functions are always lowered to 4
+  // Word32, so each i8x16 needs to be correctly converted.
+  TestSignatures sigs;
+  WasmRunner<uint32_t, uint32_t> r(execution_tier, lower_simd);
+
+  WasmFunctionCompiler& fn = r.NewFunction(sigs.s_ss());
+  BUILD(fn,
+        WASM_SIMD_BINOP(kExprI8x16Add, WASM_GET_LOCAL(0), WASM_GET_LOCAL(1)));
+
+  BUILD(r,
+        WASM_SIMD_I8x16_EXTRACT_LANE(
+            0, WASM_CALL_FUNCTION(fn.function_index(),
+                                  WASM_SIMD_I8x16_SPLAT(WASM_GET_LOCAL(0)),
+                                  WASM_SIMD_I8x16_SPLAT(WASM_GET_LOCAL(0)))));
+  CHECK_EQ(2, r.Call(1));
+}
+
+WASM_SIMD_TEST(I16x8_Call_Return) {
+  // Check that calling a function with i16x8 arguments, and returns i16x8, is
+  // correctly lowered. The signature of the functions are always lowered to 4
+  // Word32, so each i16x8 needs to be correctly converted.
+  TestSignatures sigs;
+  WasmRunner<uint32_t, uint32_t> r(execution_tier, lower_simd);
+
+  WasmFunctionCompiler& fn = r.NewFunction(sigs.s_ss());
+  BUILD(fn,
+        WASM_SIMD_BINOP(kExprI16x8Add, WASM_GET_LOCAL(0), WASM_GET_LOCAL(1)));
+
+  BUILD(r,
+        WASM_SIMD_I16x8_EXTRACT_LANE(
             0, WASM_CALL_FUNCTION(fn.function_index(),
-                                  WASM_SIMD_I32x4_SPLAT(WASM_GET_LOCAL(0)))));
-  CHECK_EQ(0x80000001, r.Call(1));
+                                  WASM_SIMD_I16x8_SPLAT(WASM_GET_LOCAL(0)),
+                                  WASM_SIMD_I16x8_SPLAT(WASM_GET_LOCAL(0)))));
+  CHECK_EQ(2, r.Call(1));
 }
 
 WASM_SIMD_TEST(I8x16Eq_ToTest_S128Const) {

test/common/wasm/test-signatures.h

@@ -49,6 +49,7 @@ class TestSignatures {
         sig_v_c(0, 1, kFuncTypes4),
         sig_s_i(1, 1, kSimd128IntTypes4),
         sig_s_s(1, 1, kSimd128Types4),
+        sig_s_ss(1, 2, kSimd128Types4),
         sig_ii_v(2, 0, kIntTypes4),
         sig_iii_v(3, 0, kIntTypes4) {
     // I used C++ and you won't believe what happened next....
@@ -112,6 +113,7 @@ class TestSignatures {
   FunctionSig* v_c() { return &sig_v_c; }
   FunctionSig* s_i() { return &sig_s_i; }
   FunctionSig* s_s() { return &sig_s_s; }
+  FunctionSig* s_ss() { return &sig_s_ss; }
 
   FunctionSig* ii_v() { return &sig_ii_v; }
   FunctionSig* iii_v() { return &sig_iii_v; }
@@ -178,6 +180,7 @@ class TestSignatures {
   FunctionSig sig_v_c;
   FunctionSig sig_s_i;
   FunctionSig sig_s_s;
+  FunctionSig sig_s_ss;
 
   FunctionSig sig_ii_v;
   FunctionSig sig_iii_v;