[regalloc] Avoid duplicate moves for slot constraint

We potentially emitted the same gap move multiple times to satisfy slot
constraint of live ranges defined by a constant. Avoid this by keeping
track of already spilled ranges for a given instruction.

This is not expected to cause any regression because this case is rare.
If it does, a better approach to save allocations would be to re-use the
same vector by storing it somewhere that survives the function calls,
e.g. in the ConstraintBuilder.

Drive-by: Remove unused functions.

R=sigurds@chromium.org
CC=nicohartmann@chromium.org

Bug: chromium:1204748
Change-Id: I75a838a8b27775ecdeddb4c60cf72c56d5f1c2a4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2871462Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74375}

src/compiler/backend/register-allocator.cc

@@ -1373,21 +1373,6 @@ TopLevelLiveRange* TopTierRegisterAllocationData::NewLiveRange(
   return allocation_zone()->New<TopLevelLiveRange>(index, rep);
 }
 
-int TopTierRegisterAllocationData::GetNextLiveRangeId() {
-  int vreg = virtual_register_count_++;
-  if (vreg >= static_cast<int>(live_ranges().size())) {
-    live_ranges().resize(vreg + 1, nullptr);
-  }
-  return vreg;
-}
-
-TopLevelLiveRange* TopTierRegisterAllocationData::NextLiveRange(
-    MachineRepresentation rep) {
-  int vreg = GetNextLiveRangeId();
-  TopLevelLiveRange* ret = NewLiveRange(vreg, rep);
-  return ret;
-}
-
 TopTierRegisterAllocationData::PhiMapValue*
 TopTierRegisterAllocationData::InitializePhiMap(const InstructionBlock* block,
                                                 PhiInstruction* phi) {
@@ -1747,6 +1732,7 @@ void ConstraintBuilder::MeetConstraintsAfter(int instr_index) {
 void ConstraintBuilder::MeetConstraintsBefore(int instr_index) {
   Instruction* second = code()->InstructionAt(instr_index);
   // Handle fixed input operands of second instruction.
+  ZoneVector<TopLevelLiveRange*>* spilled_consts = nullptr;
   for (size_t i = 0; i < second->InputCount(); i++) {
     InstructionOperand* input = second->InputAt(i);
     if (input->IsImmediate()) {
@@ -1757,8 +1743,19 @@ void ConstraintBuilder::MeetConstraintsBefore(int instr_index) {
       TopLevelLiveRange* range =
           data()->GetOrCreateLiveRangeFor(cur_input->virtual_register());
       if (range->HasSpillOperand() && range->GetSpillOperand()->IsConstant()) {
+        bool already_spilled = false;
+        if (spilled_consts == nullptr) {
+          spilled_consts =
+              allocation_zone()->New<ZoneVector<TopLevelLiveRange*>>(
+                  allocation_zone());
+        } else {
+          auto it =
+              std::find(spilled_consts->begin(), spilled_consts->end(), range);
+          already_spilled = it != spilled_consts->end();
+        }
         auto it = data()->slot_for_const_range().find(range);
         if (it == data()->slot_for_const_range().end()) {
+          DCHECK(!already_spilled);
           int width = ByteWidthForStackSlot(range->representation());
           int index = data()->frame()->AllocateSpillSlot(width);
           auto* slot = AllocatedOperand::New(allocation_zone(),
@@ -1766,13 +1763,15 @@ void ConstraintBuilder::MeetConstraintsBefore(int instr_index) {
                                              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 (!already_spilled) {
+          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.
+          data()->AddGapMove(instr_index, Instruction::END, input_copy, *slot);
+          spilled_consts->push_back(range);
+        }
       }
     }
     if (cur_input->HasFixedPolicy()) {

src/compiler/backend/register-allocator.h

@@ -305,7 +305,6 @@ class TopTierRegisterAllocationData final : public RegisterAllocationData {
   TopLevelLiveRange* GetOrCreateLiveRangeFor(int index);
   // Creates a new live range.
   TopLevelLiveRange* NewLiveRange(int index, MachineRepresentation rep);
-  TopLevelLiveRange* NextLiveRange(MachineRepresentation rep);
 
   SpillRange* AssignSpillRangeToLiveRange(TopLevelLiveRange* range,
                                           SpillMode spill_mode);
@@ -356,8 +355,6 @@ class TopTierRegisterAllocationData final : public RegisterAllocationData {
   }
 
  private:
-  int GetNextLiveRangeId();
-
   Zone* const allocation_zone_;
   Frame* const frame_;
   InstructionSequence* const code_;

test/mjsunit/regress/regress-1204748.js

+// 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_2(__v_4, __v_5) {
+  let __v_6 = __v_4 >= __v_5;
+    while (__v_6 != 0) {
+        __v_4 = __v_4 | __v_5 - __v_4;
+      let __v_7 = __v_4 >= __v_5;
+        new Int32Array(__v_4);
+        __v_6 = __v_4 < __v_5;
+    }
+}
+function __f_3() {
+  __f_2(Infinity, 1);
+    __f_2();
+}
+  %PrepareFunctionForOptimization(__f_3);
+  %PrepareFunctionForOptimization(__f_2);
+  __f_3();
+  %OptimizeFunctionOnNextCall(__f_3);
+  __f_3();