Commit 7514db4b authored by Ng Zhi An's avatar Ng Zhi An Committed by Commit Bot

[wasm-simd][liftoff][x64][ia32] Fix i64x2.mul codegen

We are overwriting rhs when dst == rhs && dst != lhs. This is not a
problem on TurboFan because we specify unique registers and dst == lhs
in the instruction-selector.

The fix is to use the helper EmitSimdCommutativeBinOp, which will check
for dst == rhs (pmuludq is commutative).

Bug: v8:11612
Change-Id: I38c3a2b7f3c7bcf2d7e8faec1a67f0814d44ed20
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2798527
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Reviewed-by: 's avatarDeepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73780}
parent ba5fafb0
......@@ -3970,13 +3970,8 @@ void LiftoffAssembler::emit_i64x2_mul(LiftoffRegister dst, LiftoffRegister lhs,
Pmuludq(tmp2.fp(), tmp2.fp(), lhs.fp());
Paddq(tmp2.fp(), tmp2.fp(), tmp1.fp());
Psllq(tmp2.fp(), tmp2.fp(), 32);
if (CpuFeatures::IsSupported(AVX)) {
CpuFeatureScope scope(this, AVX);
vpmuludq(dst.fp(), lhs.fp(), rhs.fp());
} else {
if (dst.fp() != lhs.fp()) movaps(dst.fp(), lhs.fp());
pmuludq(dst.fp(), rhs.fp());
}
liftoff::EmitSimdCommutativeBinOp<&Assembler::vpmuludq, &Assembler::pmuludq>(
this, dst, lhs, rhs);
Paddq(dst.fp(), dst.fp(), tmp2.fp());
}
......
......@@ -3511,13 +3511,8 @@ void LiftoffAssembler::emit_i64x2_mul(LiftoffRegister dst, LiftoffRegister lhs,
Pmuludq(tmp2.fp(), lhs.fp());
Paddq(tmp2.fp(), tmp1.fp());
Psllq(tmp2.fp(), 32);
if (CpuFeatures::IsSupported(AVX)) {
CpuFeatureScope scope(this, AVX);
vpmuludq(dst.fp(), lhs.fp(), rhs.fp());
} else {
if (dst.fp() != lhs.fp()) movaps(dst.fp(), lhs.fp());
pmuludq(dst.fp(), rhs.fp());
}
liftoff::EmitSimdCommutativeBinOp<&Assembler::vpmuludq, &Assembler::pmuludq>(
this, dst, lhs, rhs);
Paddq(dst.fp(), tmp2.fp());
}
......
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --no-enable-avx
load('test/mjsunit/wasm/wasm-module-builder.js');
// Carefully hand-crafted test case to exercie a codegen bug in Liftoff. In
// i64x2.mul, non-AVX case, we will overwrite rhs if dst == rhs. The intention
// is to do dst = lhs * rhs, but if dst == rhs && dst != lhs, we will overwrite
// dst (and hence rhs) with lhs, effectively doing lhs^2.
const builder = new WasmModuleBuilder();
builder.addMemory(16, 32);
builder.addFunction(undefined, kSig_l_v)
.addBody([
kExprI64Const, 0,
kSimdPrefix, kExprI64x2Splat,
kExprI64Const, 1,
kSimdPrefix, kExprI64x2Splat,
kExprI64Const, 2,
kSimdPrefix, kExprI64x2Splat,
kExprCallFunction, 1,
]);
let sig = makeSig([kWasmS128, kWasmS128, kWasmS128], [kWasmI64]);
builder.addFunction(undefined, sig)
.addLocals(kWasmS128, 10)
.addBody([
kExprLocalGet, 2, // This is 2 (lhs).
kExprI64Const, 4, // This is 4 (rhs).
kSimdPrefix, kExprI64x2Splat,
kSimdPrefix, kExprI64x2Mul, 0x01, // The bug will write 2 to rhs.
kSimdPrefix, kExprI64x2ExtractLane, 0,
]);
builder.addExport('main', 0);
const module = builder.instantiate();
// Should be 2 * 4, the buggy codegen will give 2 * 2 instead.
assertEquals(8n, module.exports.main());
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