Commit 5a716edd authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

[maglev] Make jump gap allocation match move emission

Jump gap moves (for phis and register merges) are emitted as a parallel
move (i.e. treated as a single mapping from registers to registers and
emitted in a way that they don't clobber each other). However, the phi
input allocation was updating the register state as if they were
serialised moves (i.e. a list of moves, one after the other, where each
move could clobber another move's input).

Now the jump phi initialisation doesn't update register state.

Bug: v8:7700
Change-Id: Iecf3211d59d9c416a4449aea22fef633717d92d3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784983Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81934}
parent fd9331eb
......@@ -640,25 +640,15 @@ void StraightForwardRegisterAllocator::InitializeBranchTargetPhis(
DCHECK(!target->is_empty_block());
if (!target->has_phi()) return;
// Phi moves are emitted by resolving all phi moves as a single parallel move,
// which means we shouldn't update register state as we go (as if we were
// emitting a series of serialised moves) but rather take 'old' register
// state as the phi input.
Phi::List* phis = target->phis();
for (Phi* phi : *phis) {
Input& input = phi->input(predecessor_id);
input.InjectLocation(input.node()->allocation());
// Write the node to the phi's register (if any), to make sure
// register state is accurate for MergeRegisterValues later.
if (phi->result().operand().IsAnyRegister()) {
DCHECK(!phi->result().operand().IsDoubleRegister());
Register reg = phi->result().AssignedGeneralRegister();
DCHECK(!general_registers_.is_blocked(reg));
if (!general_registers_.free().has(reg)) {
// Drop the value currently in the register.
DropRegisterValue(general_registers_, reg);
} else {
general_registers_.RemoveFromFree(reg);
}
general_registers_.SetValue(reg, input.node());
}
}
for (Phi* phi : *phis) UpdateUse(&phi->input(predecessor_id));
}
......@@ -722,27 +712,18 @@ void StraightForwardRegisterAllocator::AllocateControlNode(ControlNode* node,
DCHECK(!node->properties().can_eager_deopt());
DCHECK(!node->properties().can_lazy_deopt());
DCHECK(!node->properties().needs_register_snapshot());
// Initialize phis before assigning inputs, in case one of the inputs
// conflicts with a fixed phi.
InitializeBranchTargetPhis(block->predecessor_id(),
unconditional->target());
DCHECK(!node->properties().is_call());
general_registers_.clear_blocked();
double_registers_.clear_blocked();
VerifyRegisterState();
auto predecessor_id = block->predecessor_id();
auto target = unconditional->target();
InitializeBranchTargetPhis(predecessor_id, target);
MergeRegisterValues(unconditional, target, predecessor_id);
if (FLAG_trace_maglev_regalloc) {
printing_visitor_->Process(node,
ProcessingState(compilation_info_, block_it_));
}
// Merge register values. Values only flowing into phis and not being
// independently live will be killed as part of the merge.
MergeRegisterValues(unconditional, unconditional->target(),
block->predecessor_id());
} else {
DCHECK(node->Is<ConditionalControlNode>() || node->Is<Return>());
AssignInputs(node);
......
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