Skip to content

Commit

Permalink
[regalloc] Missing FP register conflict check
Browse files Browse the repository at this point in the history
Check aliased FP registers when constructing the {to_be_live} set from
multiple predecessors.

[email protected]

Bug: chromium:1029642
Change-Id: I3db7b705ad5689bd8321aebc5e9c5f364951870b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1958054
Reviewed-by: Sigurd Schneider <[email protected]>
Commit-Queue: Thibaud Michaud <[email protected]>
Cr-Commit-Position: refs/heads/master@{#65412}
  • Loading branch information
thibaudmichaud authored and Commit Bot committed Dec 11, 2019
1 parent 2eed6c4 commit 8c050b7
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 7 deletions.
35 changes: 28 additions & 7 deletions src/compiler/backend/register-allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3393,6 +3393,18 @@ RpoNumber LinearScanAllocator::ChooseOneOfTwoPredecessorStates(
: current_block->predecessors()[1];
}

bool LinearScanAllocator::CheckConflict(MachineRepresentation rep, int reg,
RangeWithRegisterSet* to_be_live) {
for (RangeWithRegister range_with_reg : *to_be_live) {
if (data()->config()->AreAliases(range_with_reg.range->representation(),
range_with_reg.expected_register, rep,
reg)) {
return true;
}
}
return false;
}

void LinearScanAllocator::ComputeStateFromManyPredecessors(
InstructionBlock* current_block, RangeWithRegisterSet* to_be_live) {
struct Vote {
Expand Down Expand Up @@ -3440,24 +3452,33 @@ void LinearScanAllocator::ComputeStateFromManyPredecessors(
std::function<bool(TopLevelLiveRange*)> filter,
RangeWithRegisterSet* to_be_live,
bool* taken_registers) {
bool check_aliasing = !kSimpleFPAliasing && check_fp_aliasing();
for (const auto& val : counts) {
if (!filter(val.first)) continue;
if (val.second.count >= majority) {
int register_max = 0;
int reg = kUnassignedRegister;
for (int idx = 0; idx < RegisterConfiguration::kMaxRegisters; idx++) {
bool conflict = false;
int num_regs = num_registers();
int num_codes = num_allocatable_registers();
const int* codes = allocatable_register_codes();
MachineRepresentation rep = val.first->representation();
if (check_aliasing && (rep == MachineRepresentation::kFloat32 ||
rep == MachineRepresentation::kSimd128))
GetFPRegisterSet(rep, &num_regs, &num_codes, &codes);
for (int idx = 0; idx < num_regs; idx++) {
int uses = val.second.used_registers[idx];
if (uses == 0) continue;
if (uses > register_max) {
reg = idx;
register_max = val.second.used_registers[idx];
} else if (taken_registers[reg] && uses == register_max) {
if (uses > register_max || (conflict && uses == register_max)) {
reg = idx;
register_max = uses;
conflict = check_aliasing ? CheckConflict(rep, reg, to_be_live)
: taken_registers[reg];
}
}
if (taken_registers[reg]) {
if (conflict) {
reg = kUnassignedRegister;
} else {
} else if (!check_aliasing) {
taken_registers[reg] = true;
}
to_be_live->emplace(val.first, reg);
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/backend/register-allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,8 @@ class LinearScanAllocator final : public RegisterAllocator {
RpoNumber predecessor);
RpoNumber ChooseOneOfTwoPredecessorStates(InstructionBlock* current_block,
LifetimePosition boundary);
bool CheckConflict(MachineRepresentation rep, int reg,
RangeWithRegisterSet* to_be_live);
void ComputeStateFromManyPredecessors(InstructionBlock* current_block,
RangeWithRegisterSet* to_be_live);

Expand Down
67 changes: 67 additions & 0 deletions test/mjsunit/regress/wasm/regress-1029642.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright 2019 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: --wasm-staging

// Copyright 2016 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.

load('test/mjsunit/wasm/wasm-module-builder.js')

const builder = new WasmModuleBuilder();
builder.addMemory(0, 0, false);
builder.addType(makeSig([kWasmF32, kWasmF32, kWasmF64], [kWasmF64]));
// Generate function 1 (out of 1).
builder.addFunction(undefined, 0 /* sig */)
.addBodyWithEnd([
// signature: d_ffdl
// body:
kExprLoop, kWasmF64, // @15 f64
kExprLoop, kWasmF64, // @17 f64
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprIf, kWasmF64, // @251 f64
kExprLoop, kWasmF64, // @253 f64
kExprI32Const, 0,
kExprIf, kWasmI32, // @267 i32
kExprMemorySize, 0x00,
kExprMemoryGrow, 0x00,
kExprElse, // @273
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32SConvertF32,
kExprEnd, // @282
kExprDrop,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0x00,
kExprBrIf, 0x01, // depth=1
kExprI32SConvertF64,
kExprF64SConvertI32,
kExprEnd, // @311
kExprElse, // @312
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprEnd, // @322
kExprF64Max,
kExprF64Max,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprCallFunction, 0x00, // function #0: d_ffdl
kExprF64Max,
kExprF64Max,
kExprF64Max,
kExprI32Const, 0x00,
kExprF64SConvertI32,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64Max,
kExprF64Max,
kExprEnd, // @1374
kExprEnd, // @1375
kExprEnd, // @1376
]);
builder.addExport('main', 0);
builder.toModule();

0 comments on commit 8c050b7

Please sign in to comment.