Commit 8bfd278c authored by Yolanda Chen's avatar Yolanda Chen Committed by Commit Bot

[regalloc] Fix a regression when enable FindOptimalSpillingPos for phis

This patch is to avoid spilling the phi at the loop header if there is a back-edge with an input for the phi that interferes with the phi's value, because in case that input gets spilled it might introduce a stack-to-stack move at the back-edge.

Bug: chromium:1063831
Change-Id: Ie7129f10fb573cc799c588e6639b5ad486ea520d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2152002
Commit-Queue: Yolanda Chen <yolanda.chen@intel.com>
Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67299}
parent d587f67a
......@@ -2769,6 +2769,7 @@ void BundleBuilder::BuildBundles() {
}
TRACE("Processing phi for v%d with %d:%d\n", phi->virtual_register(),
out_range->TopLevel()->vreg(), out_range->relative_id());
bool phi_interferes_with_backedge_input = false;
for (auto input : phi->operands()) {
LiveRange* input_range = data()->GetOrCreateLiveRangeFor(input);
TRACE("Input value v%d with range %d:%d\n", input,
......@@ -2776,16 +2777,32 @@ void BundleBuilder::BuildBundles() {
LiveRangeBundle* input_bundle = input_range->get_bundle();
if (input_bundle != nullptr) {
TRACE("Merge\n");
if (out->TryMerge(input_bundle, data()->is_trace_alloc()))
if (out->TryMerge(input_bundle, data()->is_trace_alloc())) {
TRACE("Merged %d and %d to %d\n", phi->virtual_register(), input,
out->id());
} else if (input_range->Start() > out_range->Start()) {
// We are only interested in values defined after the phi, because
// those are values that will go over a back-edge.
phi_interferes_with_backedge_input = true;
}
} else {
TRACE("Add\n");
if (out->TryAddRange(input_range))
if (out->TryAddRange(input_range)) {
TRACE("Added %d and %d to %d\n", phi->virtual_register(), input,
out->id());
} else if (input_range->Start() > out_range->Start()) {
// We are only interested in values defined after the phi, because
// those are values that will go over a back-edge.
phi_interferes_with_backedge_input = true;
}
}
}
// Spilling the phi at the loop header is not beneficial if there is
// a back-edge with an input for the phi that interferes with the phi's
// value, because in case that input gets spilled it might introduce
// a stack-to-stack move at the back-edge.
if (phi_interferes_with_backedge_input)
out_range->TopLevel()->set_spilling_at_loop_header_not_beneficial();
}
TRACE("Done block B%d\n", block_id);
}
......@@ -3006,6 +3023,12 @@ LifetimePosition RegisterAllocator::FindOptimalSpillingPos(
// This will reduce number of memory moves on the back edge.
LifetimePosition loop_start = LifetimePosition::GapFromInstructionIndex(
loop_header->first_instruction_index());
// Stop if we moved to a loop header before the value is defined or
// at the define position that is not beneficial to spill.
if (range->TopLevel()->Start() > loop_start ||
(range->TopLevel()->Start() == loop_start &&
range->TopLevel()->SpillAtLoopHeaderNotBeneficial()))
return pos;
auto& loop_header_state =
data()->GetSpillState(loop_header->rpo_number());
for (LiveRange* live_at_header : loop_header_state) {
......
......@@ -675,7 +675,7 @@ class V8_EXPORT_PRIVATE LiveRange : public NON_EXPORTED_BASE(ZoneObject) {
using RepresentationField = base::BitField<MachineRepresentation, 13, 8>;
using RecombineField = base::BitField<bool, 21, 1>;
using ControlFlowRegisterHint = base::BitField<uint8_t, 22, 6>;
// Bit 28 is used by TopLevelLiveRange.
// Bits 28,29 are used by TopLevelLiveRange.
// Unique among children and splinters of the same virtual register.
int relative_id_;
......@@ -785,6 +785,12 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
void set_is_non_loop_phi(bool value) {
bits_ = IsNonLoopPhiField::update(bits_, value);
}
bool SpillAtLoopHeaderNotBeneficial() const {
return SpillAtLoopHeaderNotBeneficialField::decode(bits_);
}
void set_spilling_at_loop_header_not_beneficial() {
bits_ = SpillAtLoopHeaderNotBeneficialField::update(bits_, true);
}
enum SlotUseKind { kNoSlotUse, kDeferredSlotUse, kGeneralSlotUse };
......@@ -991,6 +997,7 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
using IsNonLoopPhiField = base::BitField<bool, 4, 1>;
using SpillTypeField = base::BitField<SpillType, 5, 2>;
using DeferredFixedField = base::BitField<bool, 28, 1>;
using SpillAtLoopHeaderNotBeneficialField = base::BitField<bool, 29, 1>;
int vreg_;
int last_child_id_;
......
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