[compiler][arm64] Fix Dup and Shuffle recognition

sparker-arm · V8 LUCI CQ · commit b8ff29cf62ff · 2025-10-29T06:08:10.000-07:00
The 'dup and shuffle' optimization worked on the assumption that, for swizzles, we could drop the second argument and replace it with the result of the dup. This would result in the dup producing the eight most significant bytes and the canonical shuffle would produce the other eight. However, because we are dealing with swizzles, it may be that the instruction used to implement the canonical shuffle doesn't read from two registers. In this case the dup is never read and so the top bytes are not what we expect. Bug: 455592080 Change-Id: Ic205b01dec1a82631b1de1fce2d98e82d9af3804 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7095236 Reviewed-by: Darius Mercadier <dmercadier@chromium.org> Commit-Queue: Darius Mercadier <dmercadier@chromium.org> Reviewed-by: Marja Hölttä <marja@chromium.org> Cr-Commit-Position: refs/heads/main@{#103394}
diff --git a/src/compiler/backend/arm64/instruction-selector-arm64.cc b/src/compiler/backend/arm64/instruction-selector-arm64.cc
@@ -5819,19 +5819,14 @@ void InstructionSelector::VisitI8x16Shuffle(OpIndex node) {
     int lane_size = kBitsPerByte * kSimd128Size / lanes;
     Emit(kArm64S128Dup | LaneSizeField::encode(lane_size), dup,
          g.UseRegister(dup_input), g.UseImmediate(dup_index));
-    if (is_swizzle) {
-      Emit(shuffle_op, g.DefineAsRegister(node), g.UseRegister(input0), dup);
-      return;
-    } else {
-      // For non-swizzles, we first need to perform the shuffles with the two
-      // original inputs, into a temp register.
-      InstructionOperand temp = g.TempSimd128Register();
-      Emit(shuffle_op, temp, g.UseRegister(input0), g.UseRegister(input1));
-      // Then we need to move the dup result into the top 8 bytes.
-      Emit(kArm64S128MoveLane | LaneSizeField::encode(64),
-           g.DefineSameAsFirst(node), temp, dup, g.UseImmediate(1),
-           g.UseImmediate(1));
-    }
+    // First need to perform the shuffles with the two original inputs, into a
+    // temp register.
+    InstructionOperand temp = g.TempSimd128Register();
+    Emit(shuffle_op, temp, g.UseRegister(input0), g.UseRegister(input1));
+    // Then we need to move the dup result into the top 8 bytes.
+    Emit(kArm64S128MoveLane | LaneSizeField::encode(64),
+         g.DefineSameAsFirst(node), temp, dup, g.UseImmediate(1),
+         g.UseImmediate(1));
   };
 
   std::array<uint8_t, kSimd128HalfSize> bottom_shuffle;
diff --git a/test/mjsunit/wasm/half-dup-shuffles.js b/test/mjsunit/wasm/half-dup-shuffles.js
@@ -54,12 +54,14 @@ function Test(config) {
 
 (function SplatAndShuffleTest(config) {
   const dup_byte_0 = [0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 ]
+  const dup_half_0 = [0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01 ]
   const dup_half_8 = [0x10, 0x11, 0x10, 0x11, 0x10, 0x11, 0x10, 0x11 ]
   const dup_word_1 = [0x04, 0x05, 0x06, 0x07, 0x04, 0x05, 0x06, 0x07 ]
 
   const even_bytes = [0x00, 0x02, 0x04, 0x06, 0x08, 0x0a, 0x0c, 0x0e ]
   const interleave_high_halves = [0x08, 0x09, 0x18, 0x19, 0x0a, 0x0b, 0x1a, 0x1b ]
   const transpose_odd_words = [0x04, 0x05, 0x06, 0x07, 0x14, 0x15, 0x16, 0x17 ]
+  const reverse_16x2 = [0x02, 0x03, 0x00, 0x01, 0x06, 0x07, 0x04, 0x05 ]
 
   const splat_and_shuffle_tests =[
     {
@@ -77,6 +79,11 @@ function Test(config) {
       dup_shuffle: dup_word_1,
       shuffle: transpose_odd_words,
     },
+    {
+      name: "dup and reverse 16x2",
+      dup_shuffle: dup_half_0,
+      shuffle: reverse_16x2,
+    },
   ];
 
   for (const config of splat_and_shuffle_tests) {
diff --git a/test/unittests/compiler/arm64/turboshaft-instruction-selector-arm64-unittest.cc b/test/unittests/compiler/arm64/turboshaft-instruction-selector-arm64-unittest.cc
@@ -3627,23 +3627,23 @@ std::ostream& operator<<(std::ostream& os, const DupAndShuffleInst& inst) {
 const DupAndShuffleInst kDupAndShuffles[] = {
     {"Dup 0 and UnzipLeft",
      kArm64S128UnzipLeft,
-     2,
+     3,
      0,
      16,
      0,
      true,
      {{0, 1, 4, 5, 8, 9, 12, 13, 0, 1, 0, 1, 0, 1, 0, 1}}},
     {"Dup 1 and UnzipLeft",
      kArm64S128UnzipLeft,
-     2,
+     3,
      0,
      16,
      1,
      true,
      {{0, 1, 4, 5, 8, 9, 12, 13, 2, 3, 2, 3, 2, 3, 2, 3}}},
     {"Dup 0 and UnzipRight",
      kArm64S128UnzipRight,
-     2,
+     3,
      0,
      16,
      0,
@@ -3738,28 +3738,29 @@ TEST_P(TurboshaftInstructionSelectorDupAndShuffleTest, DupAndShuffle) {
   Stream s = m.Build();
   EXPECT_EQ(inst.expected_num_insts, s.size());
 
-  if (inst.expected_num_insts > 1) {
+  if (inst.expected_num_insts == 3) {
+    // The dup
     EXPECT_EQ(kArm64S128Dup, s[0]->arch_opcode());
+    EXPECT_EQ(inst.lane_size, LaneSizeField::decode(s[0]->opcode()));
     EXPECT_EQ(s.ToVreg(s[0]->InputAt(0)),
               s.ToVreg(m.Parameter(inst.expected_param_index)));
     EXPECT_EQ(s.ToInt32(s[0]->InputAt(1)), inst.index);
 
-    EXPECT_EQ(inst.lane_size, LaneSizeField::decode(s[0]->opcode()));
+    // The shuffle
     EXPECT_EQ(inst.arch_opcode, s[1]->arch_opcode());
     EXPECT_EQ(inst.lane_size, LaneSizeField::decode(s[1]->opcode()));
-
-    if (inst.expected_num_insts == 3) {
-      EXPECT_EQ(s.ToVreg(s[1]->InputAt(0)), s.ToVreg(m.Parameter(0)));
-      EXPECT_EQ(s.ToVreg(s[1]->InputAt(1)), s.ToVreg(m.Parameter(1)));
-      EXPECT_EQ(kArm64S128MoveLane, s[2]->arch_opcode());
-      EXPECT_EQ(1U, s[2]->OutputCount());
+    EXPECT_EQ(s.ToVreg(s[1]->InputAt(0)), s.ToVreg(m.Parameter(0)));
+    if (inst.is_swizzle) {
+      EXPECT_EQ(s.ToVreg(s[1]->InputAt(1)), s.ToVreg(m.Parameter(0)));
     } else {
-      EXPECT_EQ(s.ToVreg(s[1]->InputAt(0)),
-                s.ToVreg(m.Parameter(inst.expected_param_index)));
-      EXPECT_EQ(s.ToVreg(s[1]->InputAt(1)), s.ToVreg(s[0]->Output()));
-      EXPECT_EQ(1U, s[1]->OutputCount());
+      EXPECT_EQ(s.ToVreg(s[1]->InputAt(1)), s.ToVreg(m.Parameter(1)));
     }
+
+    // Copy the top half of the dup into the result register.
+    EXPECT_EQ(kArm64S128MoveLane, s[2]->arch_opcode());
+    EXPECT_EQ(1U, s[2]->OutputCount());
   } else {
+    DCHECK_EQ(inst.expected_num_insts, 1);
     EXPECT_EQ(inst.arch_opcode, s[0]->arch_opcode());
   }
 }
