Commit 7f542d25 authored by Thibaud Michaud's avatar Thibaud Michaud Committed by Commit Bot

[regalloc] Ensure ranges are split at gap positions

Splitting a range at an instruction position can lead to incorrect code
generation. See the attached bug for a concrete example of that, in
particular comment 6.

The issue is when we add a gap move to connect the split ranges during
the ConnectLiveRanges phase. If the split position is a gap position,
the move coincides with the start of the range. But if the split
position is an instruction position, the move is inserted in the last
gap position, which is outside of the range. This violates assumptions
made during the main register allocation phase and can invalidate the
use of that register in a different range.

The fix proposed here works by moving the split position backwards to
the previous gap position. This ensures that the connecting gap move is
always at the start of the range that it defines.

R=sigurds@chromium.org

Bug: chromium:1182985
Change-Id: Ic4a9f56d5551f01cc91bece087d5ab3afd9b04fd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2735396Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73333}
parent 81b91591
......@@ -4078,8 +4078,14 @@ bool LinearScanAllocator::TryAllocateFreeReg(
if (pos < current->End()) {
// Register reg is available at the range start but becomes blocked before
// the range end. Split current at position where it becomes blocked.
LiveRange* tail = SplitRangeAt(current, pos);
// the range end. Split current before the position where it becomes
// blocked. Shift the split position to the last gap position. This is to
// ensure that if a connecting move is needed, that move coincides with the
// start of the range that it defines. See crbug.com/1182985.
LifetimePosition gap_pos =
pos.IsGapPosition() ? pos : pos.FullStart().End();
if (gap_pos <= current->Start()) return false;
LiveRange* tail = SplitRangeAt(current, gap_pos);
AddToUnhandled(tail);
// Try to allocate preferred register once more.
......@@ -4088,7 +4094,7 @@ bool LinearScanAllocator::TryAllocateFreeReg(
// Register reg is available at the range start and is free until the range
// end.
DCHECK(pos >= current->End());
DCHECK_GE(pos, current->End());
TRACE("Assigning free reg %s to live range %d:%d\n", RegisterName(reg),
current->TopLevel()->vreg(), current->relative_id());
SetLiveRangeAssignedRegister(current, reg);
......
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