Commit 10bbbf13 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[compiler] Fix mixed same-as-input and unique registers

The mid-tier register allocator could not handle the case that the same
virtual register was used for
- the input corresponding to the 'same-as-input' output, and
- another 'unique register' input.

In this case, it cannot choose the already assigned register for the
'unique' register. Instead, it needs to allocate a new register and
introduce a gap move to duplicate the input value in two different
registers.

FYI, the instruction where the current logic failed was:
  (v5(0), v6(R)) = IA32AddPair v7(R) v7(*) v8(R) v7(R)
(where the last input was marked 'unique').

R=leszeks@chromium.org
CC=thibaudm@chromium.org

Bug: v8:12330, chromium:1272204
Change-Id: Ie4843aa9f5e027afe503e0481a4acdfa325dfe0e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3347821Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78411}
parent b9b4da5a
......@@ -1999,11 +1999,14 @@ RegisterIndex SinglePassRegisterAllocator::ChooseRegisterFor(
pos != UsePosition::kStart) {
// If we are trying to allocate a register that was used as a
// same_input_output operand, then we can't use it for an input that expands
// past UsePosition::kStart. This should only happen for REGISTER_OR_SLOT
// operands that are used for the deopt state, so we can just use a spill
// slot.
CHECK(!must_use_register);
return RegisterIndex::Invalid();
// past UsePosition::kStart.
if (must_use_register) {
// Use a new register instead.
reg = ChooseRegisterFor(rep, pos, must_use_register);
} else {
// Use a spill slot.
reg = RegisterIndex::Invalid();
}
}
return reg;
}
......@@ -2304,18 +2307,27 @@ void SinglePassRegisterAllocator::AllocateInput(
RegisterIndex reg = ChooseRegisterFor(virtual_register, instr_index, pos,
must_use_register);
if (reg.is_valid()) {
if (must_use_register) {
AllocateUse(reg, virtual_register, operand, instr_index, pos);
} else {
AllocatePendingUse(reg, virtual_register, operand,
operand->HasRegisterOrSlotOrConstantPolicy(),
instr_index);
}
} else {
if (!reg.is_valid()) {
// The register will have been spilled at this use.
virtual_register.SpillOperand(
operand, instr_index, operand->HasRegisterOrSlotOrConstantPolicy(),
data());
} else if (!must_use_register) {
// We might later dedice to spill this register; allocate a pending use.
AllocatePendingUse(reg, virtual_register, operand,
operand->HasRegisterOrSlotOrConstantPolicy(),
instr_index);
} else if (VirtualRegisterIsUnallocatedOrInReg(virtual_register.vreg(),
reg)) {
// The register is directly usable.
AllocateUse(reg, virtual_register, operand, instr_index, pos);
} else {
// We assigned another register to the vreg before. {ChooseRegisterFor}
// chose a different one (e.g. to fulfill a "unique register" constraint
// for a vreg that was previously used for the input corresponding to the
// "same as input" output), so add a gap move to copy the input value to
// that new register.
AllocateUseWithMove(reg, virtual_register, operand, instr_index, pos);
}
}
}
......
// 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-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);
builder.addFunction(undefined, kSig_i_iii).addBody([
kExprI64Const, 0x7a, // i64.const
kExprI64Const, 0x7f, // i64.const
kExprI64Const, 0x7e, // i64.const
kExprI64Add, // i64.add
kExprI64DivS, // i64.div_s
kExprUnreachable, // unreachable
]);
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