[wasm simd] Rework CanonicalizeShuffle for testing

- Refactors most of the logic into a helper CanonicalizeShuffle
  overload that is more easily tested.
- Reorders these methods to be in the order they're used.
- Adds unit tests for this helper.

Bug: v8:6020
Change-Id: Ia7e08bd2ff3ae62b13c9283c6de04e0e1e85086b
Reviewed-on: https://chromium-review.googlesource.com/1118706Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54303}

src/compiler/instruction-selector.cc

@@ -2892,6 +2892,76 @@ FrameStateDescriptor* InstructionSelector::GetFrameStateDescriptor(
       state_info.shared_info(), outer_state);
 }
 
+// static
+void InstructionSelector::CanonicalizeShuffle(bool inputs_equal,
+                                              uint8_t* shuffle,
+                                              bool* needs_swap,
+                                              bool* is_swizzle) {
+  *needs_swap = false;
+  // Inputs equal, then it's a swizzle.
+  if (inputs_equal) {
+    *is_swizzle = true;
+  } else {
+    // Inputs are distinct; check that both are required.
+    bool src0_is_used = false;
+    bool src1_is_used = false;
+    for (int i = 0; i < kSimd128Size; ++i) {
+      if (shuffle[i] < kSimd128Size) {
+        src0_is_used = true;
+      } else {
+        src1_is_used = true;
+      }
+    }
+    if (src0_is_used && !src1_is_used) {
+      *is_swizzle = true;
+    } else if (src1_is_used && !src0_is_used) {
+      *needs_swap = true;
+      *is_swizzle = true;
+    } else {
+      *is_swizzle = false;
+      // Canonicalize general 2 input shuffles so that the first input lanes are
+      // encountered first. This makes architectural shuffle pattern matching
+      // easier, since we only need to consider 1 input ordering instead of 2.
+      if (shuffle[0] >= kSimd128Size) {
+        // The second operand is used first. Swap inputs and adjust the shuffle.
+        *needs_swap = true;
+        for (int i = 0; i < kSimd128Size; ++i) {
+          shuffle[i] ^= kSimd128Size;
+        }
+      }
+    }
+  }
+  if (*is_swizzle) {
+    for (int i = 0; i < kSimd128Size; ++i) shuffle[i] &= kSimd128Size - 1;
+  }
+}
+
+void InstructionSelector::CanonicalizeShuffle(Node* node, uint8_t* shuffle,
+                                              bool* is_swizzle) {
+  // Get raw shuffle indices.
+  memcpy(shuffle, OpParameter<uint8_t*>(node->op()), kSimd128Size);
+  bool needs_swap;
+  bool inputs_equal = GetVirtualRegister(node->InputAt(0)) ==
+                      GetVirtualRegister(node->InputAt(1));
+  CanonicalizeShuffle(inputs_equal, shuffle, &needs_swap, is_swizzle);
+  if (needs_swap) {
+    SwapShuffleInputs(node);
+  }
+  // Duplicate the first input; for some shuffles on some architectures, it's
+  // easiest to implement a swizzle as a shuffle so it might be used.
+  if (*is_swizzle) {
+    node->ReplaceInput(1, node->InputAt(0));
+  }
+}
+
+// static
+void InstructionSelector::SwapShuffleInputs(Node* node) {
+  Node* input0 = node->InputAt(0);
+  Node* input1 = node->InputAt(1);
+  node->ReplaceInput(0, input1);
+  node->ReplaceInput(1, input0);
+}
+
 // static
 bool InstructionSelector::TryMatchIdentity(const uint8_t* shuffle) {
   for (int i = 0; i < kSimd128Size; ++i) {
@@ -2953,51 +3023,6 @@ bool InstructionSelector::TryMatchBlend(const uint8_t* shuffle) {
   return true;
 }
 
-void InstructionSelector::CanonicalizeShuffle(Node* node, uint8_t* shuffle,
-                                              bool* is_swizzle) {
-  // Get raw shuffle indices.
-  memcpy(shuffle, OpParameter<uint8_t*>(node->op()), kSimd128Size);
-
-  // Detect shuffles that only operate on one input.
-  if (GetVirtualRegister(node->InputAt(0)) ==
-      GetVirtualRegister(node->InputAt(1))) {
-    *is_swizzle = true;
-  } else {
-    // Inputs are distinct; check that both are required.
-    bool src0_is_used = false;
-    bool src1_is_used = false;
-    for (int i = 0; i < kSimd128Size; ++i) {
-      if (shuffle[i] < kSimd128Size) {
-        src0_is_used = true;
-      } else {
-        src1_is_used = true;
-      }
-    }
-    if (src0_is_used && !src1_is_used) {
-      node->ReplaceInput(1, node->InputAt(0));
-      *is_swizzle = true;
-    } else if (src1_is_used && !src0_is_used) {
-      node->ReplaceInput(0, node->InputAt(1));
-      *is_swizzle = true;
-    } else {
-      *is_swizzle = false;
-      // Canonicalize general 2 input shuffles so that the first input lanes are
-      // encountered first. This makes architectural shuffle pattern matching
-      // easier, since we only need to consider 1 input ordering instead of 2.
-      if (shuffle[0] >= kSimd128Size) {
-        // The second operand is used first. Swap inputs and adjust the shuffle.
-        SwapShuffleInputs(node);
-        for (int i = 0; i < kSimd128Size; ++i) {
-          shuffle[i] ^= kSimd128Size;
-        }
-      }
-    }
-  }
-  if (*is_swizzle) {
-    for (int i = 0; i < kSimd128Size; ++i) shuffle[i] &= kSimd128Size - 1;
-  }
-}
-
 // static
 int32_t InstructionSelector::Pack4Lanes(const uint8_t* shuffle) {
   int32_t result = 0;
@@ -3008,14 +3033,6 @@ int32_t InstructionSelector::Pack4Lanes(const uint8_t* shuffle) {
   return result;
 }
 
-// static
-void InstructionSelector::SwapShuffleInputs(Node* node) {
-  Node* input0 = node->InputAt(0);
-  Node* input1 = node->InputAt(1);
-  node->ReplaceInput(0, input1);
-  node->ReplaceInput(1, input0);
-}
-
 bool InstructionSelector::NeedsPoisoning(IsSafetyCheck safety_check) const {
   switch (poisoning_level_) {
     case PoisoningMitigationLevel::kDontPoison:

src/compiler/instruction-selector.h

@@ -449,6 +449,12 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
   }
 
   // Expose these SIMD helper functions for testing.
+  static void CanonicalizeShuffleForTesting(bool inputs_equal, uint8_t* shuffle,
+                                            bool* needs_swap,
+                                            bool* is_swizzle) {
+    CanonicalizeShuffle(inputs_equal, shuffle, needs_swap, is_swizzle);
+  }
+
   static bool TryMatchIdentityForTesting(const uint8_t* shuffle) {
     return TryMatchIdentity(shuffle);
   }
@@ -631,6 +637,22 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
   // ============= Vector instruction (SIMD) helper fns. =======================
   // ===========================================================================
 
+  // Converts a shuffle into canonical form, meaning that the first lane index
+  // is in the range [0 .. 15]. Set |inputs_equal| true if this is an explicit
+  // swizzle. Returns canonicalized |shuffle|, |needs_swap|, and |is_swizzle|.
+  // If |needs_swap| is true, inputs must be swapped. If |is_swizzle| is true,
+  // the second input can be ignored.
+  static void CanonicalizeShuffle(bool inputs_equal, uint8_t* shuffle,
+                                  bool* needs_swap, bool* is_swizzle);
+
+  // Canonicalize shuffles to make pattern matching simpler. Returns the shuffle
+  // indices, and a boolean indicating if the shuffle is a swizzle (one input).
+  void CanonicalizeShuffle(Node* node, uint8_t* shuffle, bool* is_swizzle);
+
+  // Swaps the two first input operands of the node, to help match shuffles
+  // to specific architectural instructions.
+  void SwapShuffleInputs(Node* node);
+
   // Tries to match an 8x16 byte shuffle to the identity shuffle, which is
   // [0 1 ... 15]. This should be called after canonicalizing the shuffle, so
   // the second identity shuffle, [16 17 .. 31] is converted to the first one.
@@ -685,14 +707,6 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
   // Packs 4 bytes of shuffle into a 32 bit immediate.
   static int32_t Pack4Lanes(const uint8_t* shuffle);
 
-  // Canonicalize shuffles to make pattern matching simpler. Returns the shuffle
-  // indices, and a boolean indicating if the shuffle is a swizzle (one input).
-  void CanonicalizeShuffle(Node* node, uint8_t* shuffle, bool* is_swizzle);
-
-  // Swaps the two first input operands of the node, to help match shuffles
-  // to specific architectural instructions.
-  void SwapShuffleInputs(Node* node);
-
   // ===========================================================================
 
   Schedule* schedule() const { return schedule_; }

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

@@ -618,42 +618,130 @@ TARGET_TEST_F(InstructionSelectorTest, CallStubWithDeoptRecursiveFrameState) {
   EXPECT_EQ(index, s.size());
 }
 
-// Helper to make calls to private InstructionSelector::TryMatch* functions.
+// Helper to make calls to private InstructionSelector shuffle functions.
 class InstructionSelectorShuffleTest : public ::testing::Test {
  public:
   using Shuffle = std::array<uint8_t, kSimd128Size>;
-  // Call private members
+
+  struct TestShuffle {
+    Shuffle non_canonical;
+    Shuffle canonical;
+    bool needs_swap;
+    bool is_swizzle;
+  };
+
+  // Call testing members in InstructionSelector.
+  static void CanonicalizeShuffle(bool inputs_equal, Shuffle* shuffle,
+                                  bool* needs_swap, bool* is_swizzle) {
+    InstructionSelector::CanonicalizeShuffleForTesting(
+        inputs_equal, &(*shuffle)[0], needs_swap, is_swizzle);
+  }
+
   static bool TryMatchIdentity(const Shuffle& shuffle) {
     return InstructionSelector::TryMatchIdentityForTesting(&shuffle[0]);
   }
-
   template <int LANES>
   static bool TryMatchDup(const Shuffle& shuffle, int* index) {
     return InstructionSelector::TryMatchDupForTesting<LANES>(&shuffle[0],
                                                              index);
   }
-
   static bool TryMatch32x4Shuffle(const Shuffle& shuffle,
                                   uint8_t* shuffle32x4) {
     return InstructionSelector::TryMatch32x4ShuffleForTesting(&shuffle[0],
                                                               shuffle32x4);
   }
-
   static bool TryMatch16x8Shuffle(const Shuffle& shuffle,
                                   uint8_t* shuffle16x8) {
     return InstructionSelector::TryMatch16x8ShuffleForTesting(&shuffle[0],
                                                               shuffle16x8);
   }
-
   static bool TryMatchConcat(const Shuffle& shuffle, uint8_t* offset) {
     return InstructionSelector::TryMatchConcatForTesting(&shuffle[0], offset);
   }
-
   static bool TryMatchBlend(const Shuffle& shuffle) {
     return InstructionSelector::TryMatchBlendForTesting(&shuffle[0]);
   }
 };
 
+bool operator==(const InstructionSelectorShuffleTest::Shuffle& a,
+                const InstructionSelectorShuffleTest::Shuffle& b) {
+  for (int i = 0; i < kSimd128Size; ++i) {
+    if (a[i] != b[i]) return false;
+  }
+  return true;
+}
+
+TEST_F(InstructionSelectorShuffleTest, CanonicalizeShuffle) {
+  const bool kInputsEqual = true;
+  const bool kNeedsSwap = true;
+  const bool kIsSwizzle = true;
+
+  bool needs_swap;
+  bool is_swizzle;
+
+  // Test canonicalization driven by input shuffle.
+  TestShuffle test_shuffles[] = {
+      // Identity is canonical.
+      {{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}},
+       {{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}},
+       !kNeedsSwap,
+       kIsSwizzle},
+      // Non-canonical identity requires a swap.
+      {{{16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}},
+       {{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}},
+       kNeedsSwap,
+       kIsSwizzle},
+      // General shuffle, canonical is unchanged.
+      {{{0, 16, 1, 17, 2, 18, 3, 19, 4, 20, 5, 21, 6, 22, 7, 23}},
+       {{0, 16, 1, 17, 2, 18, 3, 19, 4, 20, 5, 21, 6, 22, 7, 23}},
+       !kNeedsSwap,
+       !kIsSwizzle},
+      // Non-canonical shuffle requires a swap.
+      {{{16, 0, 17, 1, 18, 2, 19, 3, 20, 4, 21, 5, 22, 6, 23, 7}},
+       {{0, 16, 1, 17, 2, 18, 3, 19, 4, 20, 5, 21, 6, 22, 7, 23}},
+       kNeedsSwap,
+       !kIsSwizzle},
+  };
+  for (size_t i = 0; i < arraysize(test_shuffles); ++i) {
+    Shuffle shuffle = test_shuffles[i].non_canonical;
+    CanonicalizeShuffle(!kInputsEqual, &shuffle, &needs_swap, &is_swizzle);
+    EXPECT_EQ(shuffle, test_shuffles[i].canonical);
+    EXPECT_EQ(needs_swap, test_shuffles[i].needs_swap);
+    EXPECT_EQ(is_swizzle, test_shuffles[i].is_swizzle);
+  }
+
+  // Test canonicalization when inputs are equal (explicit swizzle).
+  TestShuffle test_swizzles[] = {
+      // Identity is canonical.
+      {{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}},
+       {{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}},
+       !kNeedsSwap,
+       kIsSwizzle},
+      // Non-canonical identity requires a swap.
+      {{{16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}},
+       {{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}},
+       !kNeedsSwap,
+       kIsSwizzle},
+      // Canonicalized to swizzle.
+      {{{0, 16, 1, 17, 2, 18, 3, 19, 4, 20, 5, 21, 6, 22, 7, 23}},
+       {{0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7}},
+       !kNeedsSwap,
+       kIsSwizzle},
+      // Canonicalized to swizzle.
+      {{{16, 0, 17, 1, 18, 2, 19, 3, 20, 4, 21, 5, 22, 6, 23, 7}},
+       {{0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7}},
+       !kNeedsSwap,
+       kIsSwizzle},
+  };
+  for (size_t i = 0; i < arraysize(test_swizzles); ++i) {
+    Shuffle shuffle = test_swizzles[i].non_canonical;
+    CanonicalizeShuffle(kInputsEqual, &shuffle, &needs_swap, &is_swizzle);
+    EXPECT_EQ(shuffle, test_swizzles[i].canonical);
+    EXPECT_EQ(needs_swap, test_swizzles[i].needs_swap);
+    EXPECT_EQ(is_swizzle, test_swizzles[i].is_swizzle);
+  }
+}
+
 TEST_F(InstructionSelectorShuffleTest, TryMatchIdentity) {
   // Match shuffle that returns first source operand.
   EXPECT_TRUE(TryMatchIdentity(