[wasm-simd][liftoff] Fix i64x2.mul codegen bug

When dst != lhs, we moved lhs to dst, but dst can be == rhs, so we would
overwrite rhs, and end up comparing lhs with itself, always returning
false. We handle the different aliasing cases in the macro-assembler
function I64x2GtS, to simplify the checks in Liftoff a little bit.
TurboFan does not need to change as it will require dst == lhs when AVX
is not supported.

Bug: v8:12237
Change-Id: Icefa6eb79083c003e93dbbd11ccc419aae4b15d3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3169312Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76945}

src/codegen/shared-ia32-x64/macro-assembler-shared-ia32-x64.cc

@@ -787,8 +787,16 @@ void SharedTurboAssembler::I64x2GtS(XMMRegister dst, XMMRegister src0,
     vpcmpgtq(dst, src0, src1);
   } else if (CpuFeatures::IsSupported(SSE4_2)) {
     CpuFeatureScope sse_scope(this, SSE4_2);
-    DCHECK_EQ(dst, src0);
-    pcmpgtq(dst, src1);
+    if (dst == src0) {
+      pcmpgtq(dst, src1);
+    } else if (dst == src1) {
+      movaps(scratch, src0);
+      pcmpgtq(scratch, src1);
+      movaps(dst, scratch);
+    } else {
+      movaps(dst, src0);
+      pcmpgtq(dst, src1);
+    }
   } else {
     CpuFeatureScope sse_scope(this, SSE3);
     DCHECK_NE(dst, src0);

src/wasm/baseline/ia32/liftoff-assembler-ia32.h

@@ -3133,17 +3133,11 @@ void LiftoffAssembler::emit_i64x2_ne(LiftoffRegister dst, LiftoffRegister lhs,
 void LiftoffAssembler::emit_i64x2_gt_s(LiftoffRegister dst, LiftoffRegister lhs,
                                        LiftoffRegister rhs) {
   // Different register alias requirements depending on CpuFeatures supported:
-  if (CpuFeatures::IsSupported(AVX)) {
-    // 1. AVX, no requirements.
+  if (CpuFeatures::IsSupported(AVX) || CpuFeatures::IsSupported(SSE4_2)) {
+    // 1. AVX, or SSE4_2 no requirements (I64x2GtS takes care of aliasing).
     I64x2GtS(dst.fp(), lhs.fp(), rhs.fp(), liftoff::kScratchDoubleReg);
-  } else if (CpuFeatures::IsSupported(SSE4_2)) {
-    // 2. SSE4_2, dst == lhs.
-    if (dst != lhs) {
-      movaps(dst.fp(), lhs.fp());
-    }
-    I64x2GtS(dst.fp(), dst.fp(), rhs.fp(), liftoff::kScratchDoubleReg);
   } else {
-    // 3. Else, dst != lhs && dst != rhs (lhs == rhs is ok).
+    // 2. Else, dst != lhs && dst != rhs (lhs == rhs is ok).
     if (dst == lhs || dst == rhs) {
       LiftoffRegister tmp = GetUnusedRegister(
           RegClass::kFpReg, LiftoffRegList::ForRegs(lhs, rhs));

src/wasm/baseline/x64/liftoff-assembler-x64.h

@@ -2719,17 +2719,11 @@ void LiftoffAssembler::emit_i64x2_ne(LiftoffRegister dst, LiftoffRegister lhs,
 void LiftoffAssembler::emit_i64x2_gt_s(LiftoffRegister dst, LiftoffRegister lhs,
                                        LiftoffRegister rhs) {
   // Different register alias requirements depending on CpuFeatures supported:
-  if (CpuFeatures::IsSupported(AVX)) {
-    // 1. AVX, no requirements.
+  if (CpuFeatures::IsSupported(AVX) || CpuFeatures::IsSupported(SSE4_2)) {
+    // 1. AVX, or SSE4_2 no requirements (I64x2GtS takes care of aliasing).
     I64x2GtS(dst.fp(), lhs.fp(), rhs.fp(), kScratchDoubleReg);
-  } else if (CpuFeatures::IsSupported(SSE4_2)) {
-    // 2. SSE4_2, dst == lhs.
-    if (dst != lhs) {
-      movaps(dst.fp(), lhs.fp());
-    }
-    I64x2GtS(dst.fp(), dst.fp(), rhs.fp(), kScratchDoubleReg);
   } else {
-    // 3. Else, dst != lhs && dst != rhs (lhs == rhs is ok).
+    // 2. Else, dst != lhs && dst != rhs (lhs == rhs is ok).
     if (dst == lhs || dst == rhs) {
       I64x2GtS(liftoff::kScratchDoubleReg2, lhs.fp(), rhs.fp(),
                kScratchDoubleReg);

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

@@ -3705,6 +3705,30 @@ WASM_SIMD_TEST(AddExtAddPairwiseI32LeftUnsigned) {
       kExprI32x4ExtAddPairwiseI16x8U, {1, 2, 3, 4, 5, 6, 7, 8}, {4, 9, 14, 19});
 }
 
+// Regression test from https://crbug.com/v8/12237 to exercise a codegen bug
+// for i64x2.gts which overwrote one of the inputs.
+WASM_SIMD_TEST(Regress_12237) {
+  WasmRunner<int32_t, int64_t> r(execution_tier);
+  int64_t* g = r.builder().AddGlobal<int64_t>(kWasmS128);
+  byte value = 0;
+  byte temp = r.AllocateLocal(kWasmS128);
+  int64_t local = 123;
+  BUILD(r,
+        WASM_LOCAL_SET(temp,
+                       WASM_SIMD_OPN(kExprI64x2Splat, WASM_LOCAL_GET(value))),
+        WASM_GLOBAL_SET(
+            0,
+            WASM_SIMD_BINOP(kExprI64x2GtS, WASM_LOCAL_GET(temp),
+                            WASM_SIMD_BINOP(kExprI64x2Sub, WASM_LOCAL_GET(temp),
+                                            WASM_LOCAL_GET(temp)))),
+        WASM_ONE);
+  r.Call(local);
+  int64_t expected = Greater(local, local - local);
+  for (size_t i = 0; i < kSimd128Size / sizeof(int64_t); i++) {
+    CHECK_EQ(expected, LANE(g, 0));
+  }
+}
+
 #define WASM_EXTRACT_I16x8_TEST(Sign, Type)                                    \
   WASM_SIMD_TEST(I16X8ExtractLane##Sign) {                                     \
     WasmRunner<int32_t, int32_t> r(execution_tier);                            \