Commit 3643b9e7 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[compiler] Fix spilling of SIMD registers on merge

This is similar to the previous SIMD spilling fixes, but this time at
block merges. The logic is similar to the existing cases, but not quite
the same. I did not find a nice way to unify the different locations
where we check for SIMD register overlap.

R=thibaudm@chromium.org

Bug: chromium:1283395, v8:12330
Change-Id: I5ab9b6831368cbce40b8368e4ec7954e985bff96
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3404780Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78720}
parent ca092440
......@@ -1424,7 +1424,8 @@ class SinglePassRegisterAllocator final {
// Spill a register in a previously processed successor block when merging
// state into the current block.
void SpillRegisterAtMerge(RegisterState* reg_state, RegisterIndex reg);
void SpillRegisterAtMerge(RegisterState* reg_state, RegisterIndex reg,
MachineRepresentation rep);
// Introduce a gap move to move |virtual_register| from reg |from| to reg |to|
// on entry to a |successor| block.
......@@ -1815,7 +1816,7 @@ void SinglePassRegisterAllocator::MergeStateFrom(
// then spill this register in the sucessor block to avoid
// invalidating the 1:1 vreg<->reg mapping.
// TODO(rmcilroy): Add a gap move to avoid spilling.
SpillRegisterAtMerge(successor_registers, reg);
SpillRegisterAtMerge(successor_registers, reg, rep);
continue;
}
// Register is free in our current register state, so merge the
......@@ -1841,7 +1842,7 @@ void SinglePassRegisterAllocator::MergeStateFrom(
// Spill the |new_reg| in the successor block to be able to use it
// for this gap move. It would be spilled anyway since it contains
// a different virtual register than the merge block.
SpillRegisterAtMerge(successor_registers, new_reg);
SpillRegisterAtMerge(successor_registers, new_reg, rep);
}
if (new_reg.is_valid()) {
......@@ -1849,7 +1850,7 @@ void SinglePassRegisterAllocator::MergeStateFrom(
successor_registers);
processed_regs.Add(new_reg, rep);
} else {
SpillRegisterAtMerge(successor_registers, reg);
SpillRegisterAtMerge(successor_registers, reg, rep);
}
}
}
......@@ -1869,8 +1870,8 @@ RegisterBitVector SinglePassRegisterAllocator::GetAllocatedRegBitVector(
return allocated_regs;
}
void SinglePassRegisterAllocator::SpillRegisterAtMerge(RegisterState* reg_state,
RegisterIndex reg) {
void SinglePassRegisterAllocator::SpillRegisterAtMerge(
RegisterState* reg_state, RegisterIndex reg, MachineRepresentation rep) {
DCHECK_NE(reg_state, register_state_);
if (reg_state->IsAllocated(reg)) {
int virtual_register = reg_state->VirtualRegisterForRegister(reg);
......@@ -1879,6 +1880,43 @@ void SinglePassRegisterAllocator::SpillRegisterAtMerge(RegisterState* reg_state,
AllocatedOperand allocated = AllocatedOperandForReg(reg, vreg_data.rep());
reg_state->Spill(reg, allocated, current_block_, data_);
}
// Also spill the "simd sibling" register if we want to use {reg} for SIMD.
if (!kSimpleFPAliasing && rep == MachineRepresentation::kSimd128) {
RegisterIndex sibling = simdSibling(reg);
if (reg_state->IsAllocated(sibling)) {
int virtual_register = reg_state->VirtualRegisterForRegister(sibling);
VirtualRegisterData& vreg_data =
data_->VirtualRegisterDataFor(virtual_register);
AllocatedOperand allocated =
AllocatedOperandForReg(sibling, vreg_data.rep());
reg_state->Spill(sibling, allocated, current_block_, data_);
}
}
// Similarly, spill the whole SIMD register if we want to use a part of it.
if (!kSimpleFPAliasing && (rep == MachineRepresentation::kFloat64 ||
rep == MachineRepresentation::kFloat32)) {
int simd_reg_code;
CHECK_EQ(1, data_->config()->GetAliases(rep, ToRegCode(reg, rep),
MachineRepresentation::kSimd128,
&simd_reg_code));
// Sanity check: The SIMD register code should be the shifted {reg_code}.
DCHECK_EQ(simd_reg_code,
ToRegCode(reg, rep) >>
(rep == MachineRepresentation::kFloat64 ? 1 : 2));
RegisterIndex simd_reg =
FromRegCode(simd_reg_code, MachineRepresentation::kSimd128);
DCHECK(simd_reg == reg || simdSibling(simd_reg) == reg);
if (reg_state->IsAllocated(simd_reg)) {
int virtual_register = reg_state->VirtualRegisterForRegister(simd_reg);
VirtualRegisterData& vreg_data =
data_->VirtualRegisterDataFor(virtual_register);
if (vreg_data.rep() == MachineRepresentation::kSimd128) {
AllocatedOperand allocated =
AllocatedOperandForReg(simd_reg, vreg_data.rep());
reg_state->Spill(simd_reg, allocated, current_block_, data_);
}
}
}
}
void SinglePassRegisterAllocator::MoveRegisterOnMerge(
......
......@@ -1547,6 +1547,7 @@
'regress/wasm/regress-1283042': [SKIP],
'regress/wasm/regress-1284980': [SKIP],
'regress/wasm/regress-1286253': [SKIP],
'regress/wasm/regress-1283395': [SKIP],
}], # no_simd_hardware == True
##############################################################################
......
// 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: --no-liftoff --turbo-force-mid-tier-regalloc
d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
const builder = new WasmModuleBuilder();
builder.addMemory(16, 32, false);
// Generate function 1 (out of 3).
builder.addFunction(undefined, makeSig([kWasmI32, kWasmI32, kWasmI32], []))
.addLocals(kWasmS128, 1)
.addBody([
kExprTry, kWasmVoid, // try i32
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // f64.const
kExprI32Const, 0x00, // i32.const
kSimdPrefix, kExprI8x16Splat, // i8x16.splat
kExprLocalTee, 0x03, // local.tee
kExprCallFunction, 2, // call function #2
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // f64.const
kExprLocalGet, 0x03, // local.get
kExprCallFunction, 2, // call function #2
kExprTry, kWasmVoid, // try
kExprLocalGet, 0x03, // local.get
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // f64.const
kExprF32Const, 0x00, 0x00, 0x00, 0x00, // f32.const
kExprCallFunction, 1, // call function #1
kExprCatchAll, // catch-all
kExprEnd, // end
kExprI32Const, 0x00, // i32.const
kExprI32Const, 0x00, // i32.const
kExprI32Const, 0x00, // i32.const
kAtomicPrefix, kExprI32AtomicCompareExchange16U, 0x01, 0x80, 0x80, 0xc0, 0x9b, 0x07, // i32.atomic.cmpxchng16_u
kExprDrop, // drop
kExprCatchAll, // catch-all
kExprEnd, // end
kExprI32Const, 0x00, // i32.const
kSimdPrefix, kExprI8x16Splat, // i8x16.splat
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // f64.const
kExprF32Const, 0x00, 0x00, 0x00, 0x00, // f32.const
kExprCallFunction, 1, // call function #1
]);
// Generate function 2 (out of 3).
builder.addFunction(undefined, makeSig([kWasmS128, kWasmF64, kWasmF32], []))
.addBody([kExprUnreachable]);
// Generate function 3 (out of 3).
builder.addFunction(undefined, makeSig([kWasmF64, kWasmS128], [])).addBody([
kExprUnreachable
]);
builder.toModule();
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