Commit 1017e8e2 authored by Matthias Liedtke's avatar Matthias Liedtke Committed by V8 LUCI CQ

[x64][codegen] Fix bug reducing right shifts to 32 bit

If a shift right is performed with a negative value <= -32,
it may not be reduced to a 32 bit shift.
The reduction optimization was introduced by commit
2298b35f.

Fixed: v8:13290
Change-Id: Ifb16ed85560ab54d211ebb407690abe2c156e3a2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3905143
Commit-Queue: Matthias Liedtke <mliedtke@chromium.org>
Auto-Submit: Matthias Liedtke <mliedtke@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83324}
parent a7093ce6
......@@ -998,7 +998,8 @@ void VisitWord64Shift(InstructionSelector* selector, Node* node,
if (g.CanBeImmediate(right)) {
if (opcode == kX64Shr && m.left().IsChangeUint32ToUint64() &&
m.right().HasResolvedValue() && m.right().ResolvedValue() < 32) {
m.right().HasResolvedValue() && m.right().ResolvedValue() < 32 &&
m.right().ResolvedValue() >= 0) {
opcode = kX64Shr32;
left = left->InputAt(0);
}
......
// Copyright 2022 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: --allow-natives-syntax
d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
let builder = new WasmModuleBuilder();
builder.addMemory(16, 17, false, false);
let main_func = builder.addFunction('main', kSig_i_v).exportFunc().addBody([
...wasmI32Const(1),
...wasmI32Const(-1),
kExprI32StoreMem16, 1, 0,
...wasmI32Const(0),
kExprI64LoadMem32U, 2, 0,
...wasmI64Const(-32),
kExprI64ShrU,
kExprI32ConvertI64,
]);
let instance = builder.instantiate();
let main = instance.exports.main;
for (let i = 0; i < 20; i++) assertEquals(0, main());
%WasmTierUpFunction(instance, main_func.index);
assertEquals(0, main());
// Copyright 2022 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: --allow-natives-syntax
d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
let builder = new WasmModuleBuilder();
let fcts = [
builder.addFunction('i64_shr_u_dynamic',
makeSig([kWasmI64, kWasmI64], [kWasmI64]))
.exportFunc().addBody([
kExprLocalGet, 0,
kExprLocalGet, 1,
kExprI64ShrU,
]),
builder.addFunction('i64_shr_u_by_negative_48',
makeSig([kWasmI64], [kWasmI64]))
.exportFunc().addBody([
kExprLocalGet, 0,
...wasmI64Const(-48),
kExprI64ShrU,
]),
builder.addFunction('i64_shr_s_dynamic',
makeSig([kWasmI64, kWasmI64], [kWasmI64]))
.exportFunc().addBody([
kExprLocalGet, 0,
kExprLocalGet, 1,
kExprI64ShrS,
]),
builder.addFunction('i64_shr_s_by_negative_48',
makeSig([kWasmI64], [kWasmI64]))
.exportFunc().addBody([
kExprLocalGet, 0,
...wasmI64Const(-48),
kExprI64ShrS,
]),
builder.addFunction('i32_shr_u_dynamic',
makeSig([kWasmI32, kWasmI32], [kWasmI32]))
.exportFunc().addBody([
kExprLocalGet, 0,
kExprLocalGet, 1,
kExprI32ShrU,
]),
builder.addFunction('i32_shr_u_by_negative_22',
makeSig([kWasmI32], [kWasmI32]))
.exportFunc().addBody([
kExprLocalGet, 0,
...wasmI32Const(-22),
kExprI32ShrU,
]),
builder.addFunction('i32_shr_s_dynamic',
makeSig([kWasmI32, kWasmI32], [kWasmI32]))
.exportFunc().addBody([
kExprLocalGet, 0,
kExprLocalGet, 1,
kExprI32ShrS,
]),
builder.addFunction('i32_shr_s_by_negative_22',
makeSig([kWasmI32], [kWasmI32]))
.exportFunc().addBody([
kExprLocalGet, 0,
...wasmI32Const(-22),
kExprI32ShrS,
]),
];
let instance = builder.instantiate();
let wasm = instance.exports;
let testFct = () => {
// i64.shr_u
assertEquals(0n, wasm.i64_shr_u_dynamic(0n, 0n));
assertEquals(1n, wasm.i64_shr_u_dynamic(3n, 1n));
assertEquals(-1n, wasm.i64_shr_u_dynamic(-1n, 64n));
assertEquals(-1n, wasm.i64_shr_u_dynamic(-1n, -64n));
assertEquals(1n, wasm.i64_shr_u_dynamic(-1n, 63n));
assertEquals(1n, wasm.i64_shr_u_dynamic(-1n, -1n));
assertEquals(BigInt(2 ** 32 - 1), wasm.i64_shr_u_dynamic(-1n, -32n));
assertEquals(8n, wasm.i64_shr_u_dynamic(16n, 65n));
assertEquals(8n, wasm.i64_shr_u_dynamic(16n, -127n));
assertEquals(0n, wasm.i64_shr_u_by_negative_48(123n));
assertEquals(BigInt(2 ** 48 - 1), wasm.i64_shr_u_by_negative_48(-1n));
assertEquals(4n, wasm.i64_shr_u_by_negative_48(BigInt(2 ** 18)))
// i64.shr_s
assertEquals(0n, wasm.i64_shr_s_dynamic(0n, 0n));
assertEquals(1n, wasm.i64_shr_s_dynamic(3n, 1n));
assertEquals(-1n, wasm.i64_shr_s_dynamic(-1n, 64n));
assertEquals(-1n, wasm.i64_shr_s_dynamic(-1n, -64n));
assertEquals(-1n, wasm.i64_shr_s_dynamic(-8n, 63n));
assertEquals(-4n, wasm.i64_shr_s_dynamic(-8n, -63n));
assertEquals(-1n, wasm.i64_shr_s_dynamic(-16n, -2n));
assertEquals(0n, wasm.i64_shr_s_by_negative_48(123n));
assertEquals(-BigInt(2 ** 16),
wasm.i64_shr_s_by_negative_48(-BigInt(2 ** 32)));
// i32.shr_u
assertEquals(0, wasm.i32_shr_u_dynamic(0, 0));
assertEquals(1, wasm.i32_shr_u_dynamic(3, 1));
assertEquals(-1, wasm.i32_shr_u_dynamic(-1, 32));
assertEquals(-1, wasm.i32_shr_u_dynamic(-1, -32));
assertEquals((1 << 17) - 1, wasm.i32_shr_u_dynamic(-1, 15));
assertEquals(123, wasm.i32_shr_u_by_negative_22((123 << 10) + 456));
assertEquals((1 << 22) - 123, wasm.i32_shr_u_by_negative_22(-123 << 10));
// i32.shr_s
assertEquals(0, wasm.i32_shr_s_dynamic(0, 0));
assertEquals(1, wasm.i32_shr_s_dynamic(3, 1));
assertEquals(-1, wasm.i32_shr_s_dynamic(-1, 32));
assertEquals(-1, wasm.i32_shr_s_dynamic(-1, -32));
assertEquals(-123, wasm.i32_shr_s_dynamic(-123 << 15, 15));
assertEquals(123, wasm.i32_shr_s_by_negative_22((123 << 10) + 456));
assertEquals(-123, wasm.i32_shr_s_by_negative_22((-123 << 10) + 456));
};
for (let i = 0; i < 20; i++) testFct();
for (let fct of fcts) {
%WasmTierUpFunction(instance, fct.index);
}
testFct();
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