Commit 1495b3db authored by Ng Zhi An's avatar Ng Zhi An Committed by Commit Bot

Reland "[wasm-simd][liftoff] Fix I64x2Mul"

This relands commit 76debfda.

This fix here is to convert the original mjsunit test into a
cctest, where we check for SIMD support, and skip the test.
We don't have lowering for I64x2 yet, so this is the
workaround.

Original change's description:
> [wasm-simd][liftoff] Fix I64x2Mul
>
> The I64x2Mul overwrote the lhs/rhs if they are the same as dst. So when
> deciding if we need temporaries, we should not only check the
> cache_state, but whether they alias dst or not.
>
> Bug: chromium:1088273
> Change-Id: I82efa9b45e0a3d321a06efde60971ce95b21490f
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2225796
> Commit-Queue: Zhi An Ng <zhin@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#68114}

Bug: chromium:1088273
Change-Id: Ia3fd251998159d9beb581a6af3414921fe968e40
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2227068
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68159}
parent 7e89ba7f
......@@ -2500,15 +2500,18 @@ void LiftoffAssembler::emit_i64x2_mul(LiftoffRegister dst, LiftoffRegister lhs,
QwNeonRegister tmp1 = left;
QwNeonRegister tmp2 = right;
if (cache_state()->is_used(lhs) && cache_state()->is_used(rhs)) {
LiftoffRegList used_plus_dst =
cache_state()->used_registers | LiftoffRegList::ForRegs(dst);
if (used_plus_dst.has(lhs) && used_plus_dst.has(rhs)) {
tmp1 = temps.AcquireQ();
// We only have 1 scratch Q register, so acquire another ourselves.
LiftoffRegList pinned = LiftoffRegList::ForRegs(dst);
LiftoffRegister unused_pair = GetUnusedRegister(kFpRegPair, pinned);
tmp2 = liftoff::GetSimd128Register(unused_pair);
} else if (cache_state()->is_used(lhs)) {
} else if (used_plus_dst.has(lhs)) {
tmp1 = temps.AcquireQ();
} else if (cache_state()->is_used(rhs)) {
} else if (used_plus_dst.has(rhs)) {
tmp2 = temps.AcquireQ();
}
......
......@@ -9,6 +9,7 @@
// compiles these functions, in order to verify correctness of SIMD
// implementation in Liftoff.
#include "src/codegen/assembler-inl.h"
#include "test/cctest/cctest.h"
#include "test/cctest/wasm/wasm-run-utils.h"
#include "test/common/wasm/test-signatures.h"
......@@ -84,6 +85,25 @@ WASM_SIMD_LIFTOFF_TEST(S128Return) {
CHECK_EQ(1, r.Call());
}
WASM_SIMD_LIFTOFF_TEST(REGRESS_1088273) {
// TODO(v8:9418): This is a regression test for Liftoff, translated from a
// mjsunit test. We do not have I64x2Mul lowering yet, so this will cause a
// crash on arch that don't support SIMD 128 and require lowering, thus
// explicitly skip them.
if (!CpuFeatures::SupportsWasmSimd128()) return;
WasmRunner<int32_t> r(ExecutionTier::kLiftoff, kNoLowerSimd);
TestSignatures sigs;
WasmFunctionCompiler& simd_func = r.NewFunction(sigs.s_i());
byte temp1 = simd_func.AllocateLocal(kWasmS128);
BUILD(simd_func, WASM_GET_LOCAL(temp1));
BUILD(r, WASM_SIMD_SPLAT(I8x16, WASM_I32V(0x80)),
WASM_SIMD_SPLAT(I8x16, WASM_I32V(0x92)),
WASM_SIMD_I16x8_EXTRACT_LANE_U(0, WASM_SIMD_OP(kExprI64x2Mul)));
CHECK_EQ(18688, r.Call());
}
#undef WASM_SIMD_LIFTOFF_TEST
} // namespace test_run_wasm_simd_liftoff
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment