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

[compiler] Fix mid-tier register allocator issue

If an output operand has "same as input" policy, we cannot assume that
it's input 0. Instead we should look at the {input_index}.

The bug manifests on Wasm select instructions, where the input index is
actually 2 and not 0.

In order to test this better, we introduce the a new
--turbo-force-mid-tier-regalloc flag, which always uses the mid-tier
register allocator. Otherwise the bug would only manifest on huge
functions.

R=mslekova@chromium.org
CC=​thibaudm@chromium.org

Bug: v8:12330
Change-Id: I6a005a48bbd2aba354dc99fed587bffce24c8839
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3234722Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77495}
parent 2e955523
......@@ -2875,8 +2875,8 @@ void MidTierRegisterAllocator::AllocateRegisters(
if (unallocated_output->HasSameAsInputPolicy()) {
DCHECK_EQ(i, 0);
UnallocatedOperand* unallocated_input =
UnallocatedOperand::cast(instr->InputAt(0));
UnallocatedOperand* unallocated_input = UnallocatedOperand::cast(
instr->InputAt(unallocated_output->input_index()));
VirtualRegisterData& input_vreg_data =
VirtualRegisterDataFor(unallocated_input->virtual_register());
DCHECK_EQ(AllocatorFor(output_vreg_data.rep()).kind(),
......@@ -3001,9 +3001,11 @@ void MidTierRegisterAllocator::ReserveFixedRegisters(int instr_index) {
const UnallocatedOperand* operand =
UnallocatedOperand::cast(instr->OutputAt(i));
if (operand->HasSameAsInputPolicy()) {
DCHECK_EQ(i, 0);
// Input operand has the register constraints, use it here to reserve the
// register for the output (it will be reserved for input below).
operand = UnallocatedOperand::cast(instr->InputAt(i));
operand =
UnallocatedOperand::cast(instr->InputAt(operand->input_index()));
}
if (IsFixedRegisterPolicy(operand)) {
VirtualRegisterData& vreg_data =
......
......@@ -3562,7 +3562,8 @@ bool PipelineImpl::SelectInstructions(Linkage* linkage) {
const RegisterConfiguration* config = RegisterConfiguration::Default();
std::unique_ptr<const RegisterConfiguration> restricted_config;
bool use_mid_tier_register_allocator =
(data->info()->IsTurboprop() && FLAG_turboprop_mid_tier_reg_alloc) ||
FLAG_turbo_force_mid_tier_regalloc ||
(FLAG_turboprop_mid_tier_reg_alloc && data->info()->IsTurboprop()) ||
(FLAG_turbo_use_mid_tier_regalloc_for_huge_functions &&
data->sequence()->VirtualRegisterCount() >
kTopTierVirtualRegistersLimit);
......
......@@ -925,6 +925,8 @@ DEFINE_BOOL(turbo_inline_js_wasm_calls, true, "inline JS->Wasm calls")
DEFINE_BOOL(turbo_use_mid_tier_regalloc_for_huge_functions, false,
"fall back to the mid-tier register allocator for huge functions "
"(experimental)")
DEFINE_BOOL(turbo_force_mid_tier_regalloc, false,
"always use the mid-tier register allocator (for testing)")
DEFINE_BOOL(turbo_optimize_apply, true, "optimize Function.prototype.apply")
......
// 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: --turbo-force-mid-tier-regalloc
d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
const builder = new WasmModuleBuilder();
builder.addType(makeSig([], [kWasmF32]));
builder.addFunction(undefined, 0 /* sig */)
.addLocals(kWasmI32, 1)
.addBodyWithEnd([
// signature: f_v
// body:
kExprLoop, 0x7d, // loop @3 f32
kExprI32Const, 0x9a, 0x7f, // i32.const
kExprI32Const, 0x01, // i32.const
kExprLocalGet, 0x00, // local.get
kExprSelect, // select
kExprLocalTee, 0x00, // local.tee
kExprBrIf, 0x00, // br_if depth=0
kExprF32Const, 0x4e, 0x03, 0xf1, 0xff, // f32.const
kExprEnd, // end @22
kExprEnd, // end @23
]);
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