Revert "[wasm-simd][arm64] Update f32x4.mul(dup) pattern matching"

This reverts commit d2ce5744.

Reason for revert: We reverted the early canonicalization change, so we need to worry about non-canonicalized shuffles now.

Original change's description:
> [wasm-simd][arm64] Update f32x4.mul(dup) pattern matching
>
> We now canonicalize earlier in the pipeline, and don't need to worry
> about non-canonicalized shuffles.
>
> Bug: v8:11542,v8:11257
> Change-Id: If9f5c44061465be339c98e479fd8c5a437bbd74b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2778673
> Reviewed-by: Bill Budge <bbudge@chromium.org>
> Commit-Queue: Zhi An Ng <zhin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#73645}

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

src/compiler/backend/arm64/instruction-selector-arm64.cc

@@ -3658,23 +3658,29 @@ MulWithDupResult TryMatchMulWithDup(Node* node) {
   ShuffleMatcher left = m.left();
   ShuffleMatcher right = m.right();
 
-  // We don't want CanCover here because in many use cases, the shuffle is
-  // generated early in the function, but the f32x4.mul happens in a loop, which
-  // won't cover the shuffle since they are different basic blocks.
+  // TODO(zhin): We can canonicalize first to avoid checking index < LANES.
+  // e.g. shuffle(x, y, [16, 17, 18, 19...]) => shuffle(y, y, [0, 1, 2,
+  // 3]...). But doing so can mutate the inputs of the shuffle node without
+  // updating the shuffle immediates themselves. Fix that before we
+  // canonicalize here. We don't want CanCover here because in many use cases,
+  // the shuffle is generated early in the function, but the f32x4.mul happens
+  // in a loop, which won't cover the shuffle since they are different basic
+  // blocks.
   if (left.HasResolvedValue() && wasm::SimdShuffle::TryMatchSplat<LANES>(
                                      left.ResolvedValue().data(), &index)) {
-    dup_node = left.node()->InputAt(0);
+    dup_node = left.node()->InputAt(index < LANES ? 0 : 1);
     input = right.node();
   } else if (right.HasResolvedValue() &&
              wasm::SimdShuffle::TryMatchSplat<LANES>(
                  right.ResolvedValue().data(), &index)) {
-    dup_node = right.node()->InputAt(0);
+    dup_node = right.node()->InputAt(index < LANES ? 0 : 1);
     input = left.node();
   }
-
-  DCHECK_LT(index, LANES);
 #endif  // V8_ENABLE_WEBASSEMBLY
 
+  // Canonicalization would get rid of this too.
+  index %= LANES;
+
   return {input, dup_node, index};
 }
 }  // namespace

test/unittests/compiler/arm64/instruction-selector-arm64-unittest.cc

@@ -56,7 +56,7 @@ Node* BuildConstant(InstructionSelectorTest::StreamBuilder* m, MachineType type,
     default:
       UNIMPLEMENTED();
   }
-  return nullptr;
+  return NULL;
 }
 
 // ARM64 logical instructions.
@@ -2146,9 +2146,9 @@ namespace {
 
 struct SIMDMulDPInst {
   const char* mul_constructor_name;
-  const Operator* (MachineOperatorBuilder::*mul_operator)();
-  const Operator* (MachineOperatorBuilder::*add_operator)();
-  const Operator* (MachineOperatorBuilder::*sub_operator)();
+  const Operator* (MachineOperatorBuilder::*mul_operator)(void);
+  const Operator* (MachineOperatorBuilder::*add_operator)(void);
+  const Operator* (MachineOperatorBuilder::*sub_operator)(void);
   ArchOpcode multiply_add_arch_opcode;
   ArchOpcode multiply_sub_arch_opcode;
   MachineType machine_type;
@@ -2221,24 +2221,49 @@ INSTANTIATE_TEST_SUITE_P(InstructionSelectorTest,
 struct SIMDMulDupInst {
   const uint8_t shuffle[16];
   int32_t lane;
+  int shuffle_input_index;
 };
 
 const SIMDMulDupInst kSIMDF32x4MulDuplInstructions[] = {
     {
         {0, 1, 2, 3, 0, 1, 2, 3, 0, 1, 2, 3, 0, 1, 2, 3},
         0,
+        0,
     },
     {
         {4, 5, 6, 7, 4, 5, 6, 7, 4, 5, 6, 7, 4, 5, 6, 7},
         1,
+        0,
     },
     {
         {8, 9, 10, 11, 8, 9, 10, 11, 8, 9, 10, 11, 8, 9, 10, 11},
         2,
+        0,
     },
     {
         {12, 13, 14, 15, 12, 13, 14, 15, 12, 13, 14, 15, 12, 13, 14, 15},
         3,
+        0,
+    },
+    {
+        {16, 17, 18, 19, 16, 17, 18, 19, 16, 17, 18, 19, 16, 17, 18, 19},
+        0,
+        1,
+    },
+    {
+        {20, 21, 22, 23, 20, 21, 22, 23, 20, 21, 22, 23, 20, 21, 22, 23},
+        1,
+        1,
+    },
+    {
+        {24, 25, 26, 27, 24, 25, 26, 27, 24, 25, 26, 27, 24, 25, 26, 27},
+        2,
+        1,
+    },
+    {
+        {28, 29, 30, 31, 28, 29, 30, 31, 28, 29, 30, 31, 28, 29, 30, 31},
+        3,
+        1,
     },
 };
 
@@ -2259,7 +2284,8 @@ TEST_P(InstructionSelectorSimdF32x4MulWithDupTest, MulWithDup) {
     EXPECT_EQ(3U, s[0]->InputCount());
     EXPECT_EQ(param.lane, s.ToInt32(s[0]->InputAt(2)));
     EXPECT_EQ(1U, s[0]->OutputCount());
-    EXPECT_EQ(s.ToVreg(m.Parameter(0)), s.ToVreg(s[0]->InputAt(1)));
+    EXPECT_EQ(s.ToVreg(m.Parameter(param.shuffle_input_index)),
+              s.ToVreg(s[0]->InputAt(1)));
   }
 
   // Multiplication operator should be commutative, so test shuffle op as lhs.
@@ -2274,7 +2300,8 @@ TEST_P(InstructionSelectorSimdF32x4MulWithDupTest, MulWithDup) {
     EXPECT_EQ(3U, s[0]->InputCount());
     EXPECT_EQ(param.lane, s.ToInt32(s[0]->InputAt(2)));
     EXPECT_EQ(1U, s[0]->OutputCount());
-    EXPECT_EQ(s.ToVreg(m.Parameter(0)), s.ToVreg(s[0]->InputAt(1)));
+    EXPECT_EQ(s.ToVreg(m.Parameter(param.shuffle_input_index)),
+              s.ToVreg(s[0]->InputAt(1)));
   }
 }
 
@@ -2307,10 +2334,22 @@ const SIMDMulDupInst kSIMDF64x2MulDuplInstructions[] = {
     {
         {0, 1, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 4, 5, 6, 7},
         0,
+        0,
     },
     {
         {8, 9, 10, 11, 12, 13, 14, 15, 8, 9, 10, 11, 12, 13, 14, 15},
         1,
+        0,
+    },
+    {
+        {16, 17, 18, 19, 20, 21, 22, 23, 16, 17, 18, 19, 20, 21, 22, 23},
+        0,
+        1,
+    },
+    {
+        {24, 25, 26, 27, 28, 29, 30, 31, 24, 25, 26, 27, 28, 29, 30, 31},
+        1,
+        1,
     },
 };
 
@@ -2331,7 +2370,8 @@ TEST_P(InstructionSelectorSimdF64x2MulWithDupTest, MulWithDup) {
     EXPECT_EQ(3U, s[0]->InputCount());
     EXPECT_EQ(param.lane, s.ToInt32(s[0]->InputAt(2)));
     EXPECT_EQ(1U, s[0]->OutputCount());
-    EXPECT_EQ(s.ToVreg(m.Parameter(0)), s.ToVreg(s[0]->InputAt(1)));
+    EXPECT_EQ(s.ToVreg(m.Parameter(param.shuffle_input_index)),
+              s.ToVreg(s[0]->InputAt(1)));
   }
 
   // Multiplication operator should be commutative, so test shuffle op as lhs.
@@ -2346,7 +2386,8 @@ TEST_P(InstructionSelectorSimdF64x2MulWithDupTest, MulWithDup) {
     EXPECT_EQ(3U, s[0]->InputCount());
     EXPECT_EQ(param.lane, s.ToInt32(s[0]->InputAt(2)));
     EXPECT_EQ(1U, s[0]->OutputCount());
-    EXPECT_EQ(s.ToVreg(m.Parameter(0)), s.ToVreg(s[0]->InputAt(1)));
+    EXPECT_EQ(s.ToVreg(m.Parameter(param.shuffle_input_index)),
+              s.ToVreg(s[0]->InputAt(1)));
   }
 }