Skip to content
forked from v8/v8

Commit

Permalink
[maglev] Fix interfering move cycles with double spill slots
Browse files Browse the repository at this point in the history
The ParallelMoveResolver is instantiated separately for Registers and
DoubleRegisters. Thus we cannot detect cycles between stack slots used
for spilling Registers and DoubleRegisters at the same time. To avoid
getting into this situation we don't share spill slots between the two.

Bug: v8:14271
Change-Id: I68bb239081eb83cb94d3fc7840c3e5d12598a596
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4804203
Commit-Queue: Olivier Flückiger <[email protected]>
Reviewed-by: Leszek Swirski <[email protected]>
Cr-Commit-Position: refs/heads/main@{#89593}
  • Loading branch information
o- authored and V8 LUCI CQ committed Aug 23, 2023
1 parent af81143 commit ae96bf3
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
31 changes: 26 additions & 5 deletions src/maglev/maglev-regalloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,10 @@ void StraightForwardRegisterAllocator::UpdateUse(
DCHECK_IMPLIES(
slots.free_slots.size() > 0,
slots.free_slots.back().freed_at_position <= node->live_range().end);
slots.free_slots.emplace_back(slot.index(), node->live_range().end);
bool double_slot =
IsDoubleRepresentation(node->properties().value_representation());
slots.free_slots.emplace_back(slot.index(), node->live_range().end,
double_slot);
}
}
}
Expand Down Expand Up @@ -809,7 +812,10 @@ void StraightForwardRegisterAllocator::AllocateNodeResult(ValueNode* node) {
CHECK(node->is_tagged());
CHECK_GE(idx, tagged_.top);
for (int i = tagged_.top; i < idx; ++i) {
tagged_.free_slots.emplace_back(i, node->live_range().start);
bool double_slot =
IsDoubleRepresentation(node->properties().value_representation());
tagged_.free_slots.emplace_back(i, node->live_range().start,
double_slot);
}
tagged_.top = idx + 1;
}
Expand Down Expand Up @@ -1575,8 +1581,10 @@ void StraightForwardRegisterAllocator::AllocateSpillSlot(ValueNode* node) {
bool is_tagged = (node->properties().value_representation() ==
ValueRepresentation::kTagged);
uint32_t slot_size = 1;
bool double_slot =
IsDoubleRepresentation(node->properties().value_representation());
if constexpr (kDoubleSize != kSystemPointerSize) {
if (IsDoubleRepresentation(node->properties().value_representation())) {
if (double_slot) {
slot_size = kDoubleSize / kSystemPointerSize;
}
}
Expand All @@ -1594,10 +1602,23 @@ void StraightForwardRegisterAllocator::AllocateSpillSlot(ValueNode* node) {
start, [](NodeIdT s, const SpillSlotInfo& slot_info) {
return slot_info.freed_at_position >= s;
});
// {it} points to the first invalid slot. Decrement it to get to the last
// valid slot freed before {start}.
if (it != slots.free_slots.begin()) {
// {it} points to the first invalid slot. Decrement it to get to the last
// valid slot freed before {start}.
--it;
}

// TODO(olivf): Currently we cannot mix double and normal stack slots since
// the gap resolver treats them independently and cannot detect cycles via
// shared slots.
while (it != slots.free_slots.begin()) {
if (it->double_slot == double_slot) break;
--it;
}

if (it != slots.free_slots.begin()) {
CHECK_EQ(double_slot, it->double_slot);
CHECK_GT(start, it->freed_at_position);
free_slot = it->slot_index;
slots.free_slots.erase(it);
} else {
Expand Down
8 changes: 6 additions & 2 deletions src/maglev/maglev-regalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,14 @@ class StraightForwardRegisterAllocator {
RegisterFrameState<DoubleRegister> double_registers_;

struct SpillSlotInfo {
SpillSlotInfo(uint32_t slot_index, NodeIdT freed_at_position)
: slot_index(slot_index), freed_at_position(freed_at_position) {}
SpillSlotInfo(uint32_t slot_index, NodeIdT freed_at_position,
bool double_slot)
: slot_index(slot_index),
freed_at_position(freed_at_position),
double_slot(double_slot) {}
uint32_t slot_index;
NodeIdT freed_at_position;
bool double_slot;
};
struct SpillSlots {
int top = 0;
Expand Down

0 comments on commit ae96bf3

Please sign in to comment.