Skip to content

Commit

Permalink
[regalloc] Fix slot requirement for live ranges defined by a const
Browse files Browse the repository at this point in the history
Live ranges defined by a constant operand normally don't require a spill
slot since they can just rematerialize the value from the constant. In
the attached issue however, deoptimization adds an explicit slot
requirement for a range that is defined by a constant operand. This case
is not expected in the register allocator and we eventually hit a
DCHECK.

This fix allocates a new stack slot during the MeetRegisterConstraints
and adds the missing gap move.

Drive-by: remove dead method LiveRange::NextSlotPosition.

[email protected]
CC=​[email protected]

Bug: chromium:1146880
Change-Id: I08fbb890f2f3d9574196989cf3e5ef6232433484
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2563689
Reviewed-by: Sigurd Schneider <[email protected]>
Commit-Queue: Thibaud Michaud <[email protected]>
Cr-Commit-Position: refs/heads/master@{#73510}
  • Loading branch information
thibaudmichaud authored and Commit Bot committed Mar 18, 2021
1 parent 0cfeb2c commit 0ee6f90
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
45 changes: 32 additions & 13 deletions src/compiler/backend/register-allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -476,15 +476,6 @@ UsePosition* LiveRange::NextRegisterPosition(LifetimePosition start) const {
return pos;
}

UsePosition* LiveRange::NextSlotPosition(LifetimePosition start) const {
for (UsePosition* pos = NextUsePosition(start); pos != nullptr;
pos = pos->next()) {
if (pos->type() != UsePositionType::kRequiresSlot) continue;
return pos;
}
return nullptr;
}

bool LiveRange::CanBeSpilled(LifetimePosition pos) const {
// We cannot spill a live range that has a use requiring a register
// at the current or the immediate next position.
Expand Down Expand Up @@ -1325,7 +1316,8 @@ TopTierRegisterAllocationData::TopTierRegisterAllocationData(
spill_state_(code->InstructionBlockCount(), ZoneVector<LiveRange*>(zone),
zone),
flags_(flags),
tick_counter_(tick_counter) {
tick_counter_(tick_counter),
slot_for_const_range_(zone) {
if (!kSimpleFPAliasing) {
fixed_float_live_ranges_.resize(
kNumberOfFixedRangesPerRegister * this->config()->num_float_registers(),
Expand Down Expand Up @@ -1761,6 +1753,28 @@ void ConstraintBuilder::MeetConstraintsBefore(int instr_index) {
continue; // Ignore immediates.
}
UnallocatedOperand* cur_input = UnallocatedOperand::cast(input);
if (cur_input->HasSlotPolicy()) {
TopLevelLiveRange* range =
data()->GetOrCreateLiveRangeFor(cur_input->virtual_register());
if (range->HasSpillOperand() && range->GetSpillOperand()->IsConstant()) {
auto it = data()->slot_for_const_range().find(range);
if (it == data()->slot_for_const_range().end()) {
int width = ByteWidthForStackSlot(range->representation());
int index = data()->frame()->AllocateSpillSlot(width);
auto* slot = AllocatedOperand::New(allocation_zone(),
LocationOperand::STACK_SLOT,
range->representation(), index);
it = data()->slot_for_const_range().emplace(range, slot).first;
}
auto* slot = it->second;
int input_vreg = cur_input->virtual_register();
UnallocatedOperand input_copy(UnallocatedOperand::REGISTER_OR_SLOT,
input_vreg);
// Spill at every use position for simplicity. This case is very rare -
// the only known instance is crbug.com/1146880.
data()->AddGapMove(instr_index, Instruction::END, input_copy, *slot);
}
}
if (cur_input->HasFixedPolicy()) {
int input_vreg = cur_input->virtual_register();
UnallocatedOperand input_copy(UnallocatedOperand::REGISTER_OR_SLOT,
Expand Down Expand Up @@ -4544,9 +4558,14 @@ void OperandAssigner::CommitAssignment() {
if (top_range == nullptr || top_range->IsEmpty()) continue;
InstructionOperand spill_operand;
if (top_range->HasSpillOperand()) {
spill_operand = *top_range->TopLevel()->GetSpillOperand();
} else if (top_range->TopLevel()->HasSpillRange()) {
spill_operand = top_range->TopLevel()->GetSpillRangeOperand();
auto it = data()->slot_for_const_range().find(top_range);
if (it != data()->slot_for_const_range().end()) {
spill_operand = *it->second;
} else {
spill_operand = *top_range->GetSpillOperand();
}
} else if (top_range->HasSpillRange()) {
spill_operand = top_range->GetSpillRangeOperand();
}
if (top_range->is_phi()) {
data()->GetPhiMapValueFor(top_range)->CommitAssignment(
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/backend/register-allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ class TopTierRegisterAllocationData final : public RegisterAllocationData {

TickCounter* tick_counter() { return tick_counter_; }

ZoneMap<TopLevelLiveRange*, AllocatedOperand*>& slot_for_const_range() {
return slot_for_const_range_;
}

private:
int GetNextLiveRangeId();

Expand Down Expand Up @@ -378,6 +382,7 @@ class TopTierRegisterAllocationData final : public RegisterAllocationData {
ZoneVector<ZoneVector<LiveRange*>> spill_state_;
RegisterAllocationFlags flags_;
TickCounter* const tick_counter_;
ZoneMap<TopLevelLiveRange*, AllocatedOperand*> slot_for_const_range_;
};

// Representation of the non-empty interval [start,end[.
Expand Down
26 changes: 26 additions & 0 deletions test/mjsunit/regress/regress-1146880.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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: --allow-natives-syntax --assert-types

function f(a,b) {
let t = a >= b;
while (t != 0) {
a = a | (b - a);
let unused = a >= b;
t = a < b;
}
}
function test() {
f(Infinity,1);
f(undefined, undefined);
}

// Trigger TurboFan compilation
%PrepareFunctionForOptimization(test);
%PrepareFunctionForOptimization(f);
test();
test();
%OptimizeFunctionOnNextCall(test);
test();

0 comments on commit 0ee6f90

Please sign in to comment.