[regalloc] Place spill instructions optimally

Design doc:
https://docs.google.com/document/d/1n9ADWnDI-sw0OvdSmrthf61prmDqbDmQq-NSrQw2MVI/edit?usp=sharing

Most of this change follows directly what is discussed in the design
document. A few other things are also changed:

- PopulateReferenceMapsPhase is moved after ResolveControlFlowPhase so
  that it can make use of the decision regarding whether a value is
  spilled at its definition or later.
- SpillSlotLocator is removed. It was already somewhat confusing,
  because the responsibility for marking blocks as needing frames was
  split: in some cases they were marked by SpillSlotLocator, and in
  other cases they were marked by CommitSpillsInDeferredBlocks. With
  this change, that split responsibility would become yet more
  confusing if we kept SpillSlotLocator for the values that are spilled
  at their definition, so I propose a simpler rule that whatever code
  adds the spill move also marks the block.
- A few class definitions (LiveRangeBound, FindResult,
  LiveRangeBoundArray, and LiveRangeFinder) are moved without
  modification from register-allocator.cc to register-allocator.h so
  that we can refer to them from another cc file.

Bug: v8:10606
Change-Id: I374a3219a5de477a53bc48117e230287eae89e72
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2285390
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69345}

BUILD.gn

@@ -1948,6 +1948,8 @@ v8_compiler_sources = [
   "src/compiler/backend/register-allocator-verifier.h",
   "src/compiler/backend/register-allocator.cc",
   "src/compiler/backend/register-allocator.h",
+  "src/compiler/backend/spill-placer.cc",
+  "src/compiler/backend/spill-placer.h",
   "src/compiler/backend/unwinding-info-writer.h",
   "src/compiler/basic-block-instrumentor.cc",
   "src/compiler/basic-block-instrumentor.h",

src/compiler/backend/OWNERS

@@ -5,5 +5,6 @@ zhin@chromium.org
 # Plus src/compiler owners.
 
 per-file register-allocator*=thibaudm@chromium.org
+per-file spill-placer*=thibaudm@chromium.org
 
 # COMPONENT: Blink>JavaScript>Compiler

src/compiler/backend/register-allocator.h

@@ -706,7 +706,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>;
-  // Bits 28,29 are used by TopLevelLiveRange.
+  // Bits 28-31 are used by TopLevelLiveRange.
 
   // Unique among children and splinters of the same virtual register.
   int relative_id_;
@@ -814,6 +814,7 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
   void set_is_phi(bool value) { bits_ = IsPhiField::update(bits_, value); }
 
   bool is_non_loop_phi() const { return IsNonLoopPhiField::decode(bits_); }
+  bool is_loop_phi() const { return is_phi() && !is_non_loop_phi(); }
   void set_is_non_loop_phi(bool value) {
     bits_ = IsNonLoopPhiField::update(bits_, value);
   }
@@ -865,10 +866,12 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
   // Spill range management.
   void SetSpillRange(SpillRange* spill_range);
 
-  // Encodes whether a range is also available from a memory localtion:
+  // Encodes whether a range is also available from a memory location:
   //   kNoSpillType: not availble in memory location.
   //   kSpillOperand: computed in a memory location at range start.
-  //   kSpillRange: copied (spilled) to memory location at range start.
+  //   kSpillRange: copied (spilled) to memory location at the definition,
+  //                or at the beginning of some later blocks if
+  //                LateSpillingSelected() is true.
   //   kDeferredSpillRange: copied (spilled) to memory location at entry
   //                        to deferred blocks that have a use from memory.
   //
@@ -919,9 +922,13 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
     spill_start_index_ = Min(start, spill_start_index_);
   }
 
+  // Omits any moves from spill_move_insertion_locations_ that can be skipped.
+  void FilterSpillMoves(TopTierRegisterAllocationData* data,
+                        const InstructionOperand& operand);
+
+  // Writes all moves from spill_move_insertion_locations_ to the schedule.
   void CommitSpillMoves(TopTierRegisterAllocationData* data,
-                        const InstructionOperand& operand,
-                        bool might_be_duplicated);
+                        const InstructionOperand& operand);
 
   // If all the children of this range are spilled in deferred blocks, and if
   // for any non-spilled child with a use position requiring a slot, that range
@@ -1015,6 +1022,26 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
   void MarkHasPreassignedSlot() { has_preassigned_slot_ = true; }
   bool has_preassigned_slot() const { return has_preassigned_slot_; }
 
+  // Late spilling refers to spilling at places after the definition. These
+  // spills are guaranteed to cover at least all of the sub-ranges where the
+  // register allocator chose to evict the value from a register.
+  void SetLateSpillingSelected(bool late_spilling_selected) {
+    DCHECK(spill_type() == SpillType::kSpillRange);
+    SpillRangeMode new_mode = late_spilling_selected
+                                  ? SpillRangeMode::kSpillLater
+                                  : SpillRangeMode::kSpillAtDefinition;
+    // A single TopLevelLiveRange should never be used in both modes.
+    DCHECK(SpillRangeModeField::decode(bits_) == SpillRangeMode::kNotSet ||
+           SpillRangeModeField::decode(bits_) == new_mode);
+    bits_ = SpillRangeModeField::update(bits_, new_mode);
+  }
+  bool LateSpillingSelected() const {
+    // Nobody should be reading this value until it's been decided.
+    DCHECK_IMPLIES(HasGeneralSpillRange(), SpillRangeModeField::decode(bits_) !=
+                                               SpillRangeMode::kNotSet);
+    return SpillRangeModeField::decode(bits_) == SpillRangeMode::kSpillLater;
+  }
+
   void AddBlockRequiringSpillOperand(
       RpoNumber block_id, const TopTierRegisterAllocationData* data) {
     DCHECK(IsSpilledOnlyInDeferredBlocks(data));
@@ -1031,12 +1058,21 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
   friend class LiveRange;
   void SetSplinteredFrom(TopLevelLiveRange* splinter_parent);
 
+  // If spill type is kSpillRange, then this value indicates whether we've
+  // chosen to spill at the definition or at some later points.
+  enum class SpillRangeMode : uint8_t {
+    kNotSet,
+    kSpillAtDefinition,
+    kSpillLater,
+  };
+
   using HasSlotUseField = base::BitField<SlotUseKind, 1, 2>;
   using IsPhiField = base::BitField<bool, 3, 1>;
   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>;
+  using SpillRangeModeField = base::BitField<SpillRangeMode, 30, 2>;
 
   int vreg_;
   int last_child_id_;
@@ -1055,11 +1091,12 @@ class V8_EXPORT_PRIVATE TopLevelLiveRange final : public LiveRange {
   // TODO(mtrofin): generalize spilling after definition, currently specialized
   // just for spill in a single deferred block.
   bool spilled_in_deferred_blocks_;
+  bool has_preassigned_slot_;
+
   int spill_start_index_;
   UsePosition* last_pos_;
   LiveRange* last_child_covers_;
   TopLevelLiveRange* splinter_;
-  bool has_preassigned_slot_;
 
   DISALLOW_COPY_AND_ASSIGN(TopLevelLiveRange);
 };
@@ -1114,6 +1151,65 @@ class SpillRange final : public ZoneObject {
   DISALLOW_COPY_AND_ASSIGN(SpillRange);
 };
 
+class LiveRangeBound {
+ public:
+  explicit LiveRangeBound(LiveRange* range, bool skip)
+      : range_(range), start_(range->Start()), end_(range->End()), skip_(skip) {
+    DCHECK(!range->IsEmpty());
+  }
+
+  bool CanCover(LifetimePosition position) {
+    return start_ <= position && position < end_;
+  }
+
+  LiveRange* const range_;
+  const LifetimePosition start_;
+  const LifetimePosition end_;
+  const bool skip_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LiveRangeBound);
+};
+
+struct FindResult {
+  LiveRange* cur_cover_;
+  LiveRange* pred_cover_;
+};
+
+class LiveRangeBoundArray {
+ public:
+  LiveRangeBoundArray() : length_(0), start_(nullptr) {}
+  bool ShouldInitialize() { return start_ == nullptr; }
+  void Initialize(Zone* zone, TopLevelLiveRange* range);
+  LiveRangeBound* Find(const LifetimePosition position) const;
+  LiveRangeBound* FindPred(const InstructionBlock* pred);
+  LiveRangeBound* FindSucc(const InstructionBlock* succ);
+  bool FindConnectableSubranges(const InstructionBlock* block,
+                                const InstructionBlock* pred,
+                                FindResult* result) const;
+
+ private:
+  size_t length_;
+  LiveRangeBound* start_;
+
+  DISALLOW_COPY_AND_ASSIGN(LiveRangeBoundArray);
+};
+
+class LiveRangeFinder {
+ public:
+  explicit LiveRangeFinder(const TopTierRegisterAllocationData* data,
+                           Zone* zone);
+  LiveRangeBoundArray* ArrayFor(int operand_index);
+
+ private:
+  const TopTierRegisterAllocationData* const data_;
+  const int bounds_length_;
+  LiveRangeBoundArray* const bounds_;
+  Zone* const zone_;
+
+  DISALLOW_COPY_AND_ASSIGN(LiveRangeFinder);
+};
+
 class ConstraintBuilder final : public ZoneObject {
  public:
   explicit ConstraintBuilder(TopTierRegisterAllocationData* data);
@@ -1469,20 +1565,6 @@ class LinearScanAllocator final : public RegisterAllocator {
   DISALLOW_COPY_AND_ASSIGN(LinearScanAllocator);
 };
 
-class SpillSlotLocator final : public ZoneObject {
- public:
-  explicit SpillSlotLocator(TopTierRegisterAllocationData* data);
-
-  void LocateSpillSlots();
-
- private:
-  TopTierRegisterAllocationData* data() const { return data_; }
-
-  TopTierRegisterAllocationData* const data_;
-
-  DISALLOW_COPY_AND_ASSIGN(SpillSlotLocator);
-};
-
 class OperandAssigner final : public ZoneObject {
  public:
   explicit OperandAssigner(TopTierRegisterAllocationData* data);
@@ -1508,7 +1590,7 @@ class ReferenceMapPopulator final : public ZoneObject {
  public:
   explicit ReferenceMapPopulator(TopTierRegisterAllocationData* data);
 
-  // Phase 8: compute values for pointer maps.
+  // Phase 10: compute values for pointer maps.
   void PopulateReferenceMaps();
 
  private:
@@ -1533,13 +1615,14 @@ class LiveRangeConnector final : public ZoneObject {
  public:
   explicit LiveRangeConnector(TopTierRegisterAllocationData* data);
 
-  // Phase 9: reconnect split ranges with moves, when the control flow
+  // Phase 8: reconnect split ranges with moves, when the control flow
   // between the ranges is trivial (no branches).
   void ConnectRanges(Zone* local_zone);
 
-  // Phase 10: insert moves to connect ranges across basic blocks, when the
+  // Phase 9: insert moves to connect ranges across basic blocks, when the
   // control flow between them cannot be trivially resolved, such as joining
-  // branches.
+  // branches. Also determines whether to spill at the definition or later, and
+  // adds spill moves to the gaps in the schedule.
   void ResolveControlFlow(Zone* local_zone);
 
  private:

src/compiler/backend/spill-placer.h

+// Copyright 2020 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.
+
+#ifndef V8_COMPILER_BACKEND_SPILL_PLACER_H_
+#define V8_COMPILER_BACKEND_SPILL_PLACER_H_
+
+#include "src/compiler/backend/instruction.h"
+
+namespace v8 {
+namespace internal {
+
+namespace compiler {
+
+class LiveRangeFinder;
+class TopLevelLiveRange;
+class TopTierRegisterAllocationData;
+
+// SpillPlacer is an implementation of an algorithm to find optimal spill
+// insertion positions, where optimal is defined as:
+//
+// 1. Spills needed by deferred code don't affect non-deferred code.
+// 2. No control-flow path spills the same value more than once in non-deferred
+//    blocks.
+// 3. Where possible based on #2, control-flow paths through non-deferred code
+//    that don't need the value to be on the stack don't execute any spills.
+// 4. The fewest number of spill instructions is written to meet these rules.
+// 5. Spill instructions are placed as early as possible.
+//
+// These rules are an attempt to make code paths that don't need to spill faster
+// while not increasing code size too much.
+//
+// Considering just one value at a time for now, the steps are:
+//
+// 1. If the value is defined in a deferred block, or needs its value to be on
+//    the stack during the definition block, emit a move right after the
+//    definition and exit.
+// 2. Build an array representing the state at each block, where the state can
+//    be any of the following:
+//    - unmarked (default/initial state)
+//    - definition
+//    - spill required
+//    - spill required in non-deferred successor
+//    - spill required in deferred successor
+// 3. Mark the block containing the definition.
+// 4. Mark as "spill required" all blocks that contain any part of a spilled
+//    LiveRange, or any use that requires the value to be on the stack.
+// 5. Walk the block list backward, setting the "spill required in successor"
+//    values where appropriate. If both deferred and non-deferred successors
+//    require a spill, then the result should be "spill required in non-deferred
+//    successor".
+// 6. Walk the block list forward, updating marked blocks to "spill required" if
+//    all of their predecessors agree that a spill is required. Furthermore, if
+//    a block is marked as "spill required in non-deferred successor" and any
+//    non-deferred predecessor is marked as "spill required", then the current
+//    block is updated to "spill required". We must mark these merge points as
+//    "spill required" to obey rule #2 above: if we didn't, then there would
+//    exist a control-flow path through two different spilled regions.
+// 7. Walk the block list backward again, updating blocks to "spill required" if
+//    all of their successors agree that a spill is required, or if the current
+//    block is deferred and any of its successors require spills. If only some
+//    successors of a non-deferred block require spills, then insert spill moves
+//    at the beginning of those successors. If we manage to smear the "spill
+//    required" value all the way to the definition block, then insert a spill
+//    move at the definition instead. (Spilling at the definition implies that
+//    we didn't emit any other spill moves, and there is a DCHECK mechanism to
+//    ensure that invariant.)
+//
+// Loop back-edges can be safely ignored in every step. Anything that the loop
+// header needs on-stack will be spilled either in the loop header itself or
+// sometime before entering the loop, so its back-edge predecessors don't need
+// to contain any data about the loop header.
+//
+// The operations described in those steps are simple Boolean logic, so we can
+// easily process a batch of values at the same time as an optimization.
+class SpillPlacer {
+ public:
+  SpillPlacer(LiveRangeFinder* finder, TopTierRegisterAllocationData* data,
+              Zone* zone);
+
+  ~SpillPlacer();
+
+  // Adds the given TopLevelLiveRange to the SpillPlacer's state. Will
+  // eventually commit spill moves for that range and mark the range to indicate
+  // whether its value is spilled at the definition or some later point, so that
+  // subsequent phases can know whether to assume the value is always on-stack.
+  // However, those steps may happen during a later call to Add or during the
+  // destructor.
+  void Add(TopLevelLiveRange* range);
+
+ private:
+  TopTierRegisterAllocationData* data() const { return data_; }
+
+  // While initializing data for a range, returns the index within each Entry
+  // where data about that range should be stored. May cause data about previous
+  // ranges to be committed to make room if the table is full.
+  int GetOrCreateIndexForLatestVreg(int vreg);
+
+  bool IsLatestVreg(int vreg) const {
+    return assigned_indices_ > 0 &&
+           vreg_numbers_[assigned_indices_ - 1] == vreg;
+  }
+
+  // Processes all of the ranges which have been added, inserts spill moves for
+  // them to the instruction sequence, and marks the ranges with whether they
+  // are spilled at the definition or later.
+  void CommitSpills();
+
+  void ClearData();
+
+  // Updates the iteration bounds first_block_ and last_block_ so that they
+  // include the new value.
+  void ExpandBoundsToInclude(RpoNumber block);
+
+  void SetSpillRequired(InstructionBlock* block, int vreg,
+                        RpoNumber top_start_block);
+
+  void SetDefinition(RpoNumber block, int vreg);
+
+  // The first backward pass is responsible for marking blocks which do not
+  // themselves need the value to be on the stack, but which do have successors
+  // requiring the value to be on the stack.
+  void FirstBackwardPass();
+
+  // The forward pass is responsible for selecting merge points that should
+  // require the value to be on the stack.
+  void ForwardPass();
+
+  // The second backward pass is responsible for propagating the spill
+  // requirements to the earliest block where all successors can agree a spill
+  // is required. It also emits the actual spill instructions.
+  void SecondBackwardPass();
+
+  void CommitSpill(int vreg, InstructionBlock* predecessor,
+                   InstructionBlock* successor);
+
+  // Each Entry represents the state for 64 values at a block, so that we can
+  // compute a batch of values in parallel.
+  class Entry;
+  static constexpr int kValueIndicesPerEntry = 64;
+
+  // Objects provided to the constructor, which all outlive this SpillPlacer.
+  LiveRangeFinder* finder_;
+  TopTierRegisterAllocationData* data_;
+  Zone* zone_;
+
+  // An array of one Entry per block, where blocks are in reverse post-order.
+  Entry* entries_ = nullptr;
+
+  // An array representing which TopLevelLiveRange is in each bit.
+  int* vreg_numbers_ = nullptr;
+
+  // The number of vreg_numbers_ that have been assigned.
+  int assigned_indices_ = 0;
+
+  // The first and last block that have any definitions or uses in the current
+  // batch of values. In large functions, tracking these bounds can help prevent
+  // additional work.
+  RpoNumber first_block_ = RpoNumber::Invalid();
+  RpoNumber last_block_ = RpoNumber::Invalid();
+
+  DISALLOW_COPY_AND_ASSIGN(SpillPlacer);
+};
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_COMPILER_BACKEND_SPILL_PLACER_H_

src/compiler/pipeline.cc

@@ -2246,16 +2246,6 @@ struct MergeSplintersPhase {
   }
 };
 
-
-struct LocateSpillSlotsPhase {
-  DECL_PIPELINE_PHASE_CONSTANTS(LocateSpillSlots)
-
-  void Run(PipelineData* data, Zone* temp_zone) {
-    SpillSlotLocator locator(data->top_tier_register_allocation_data());
-    locator.LocateSpillSlots();
-  }
-};
-
 struct DecideSpillingModePhase {
   DECL_PIPELINE_PHASE_CONSTANTS(DecideSpillingMode)
 
@@ -3678,15 +3668,16 @@ void PipelineImpl::AllocateRegistersForTopTier(
     verifier->VerifyAssignment("Immediately after CommitAssignmentPhase.");
   }
 
-  Run<PopulateReferenceMapsPhase>();
 
   Run<ConnectRangesPhase>();
 
   Run<ResolveControlFlowPhase>();
+
+  Run<PopulateReferenceMapsPhase>();
+
   if (FLAG_turbo_move_optimization) {
     Run<OptimizeMovesPhase>();
   }
-  Run<LocateSpillSlotsPhase>();
 
   TraceSequence(info(), data, "after register allocation");
 

src/flags/flag-definitions.h

@@ -547,6 +547,9 @@ DEFINE_BOOL(turbo_sp_frame_access, false,
             "use stack pointer-relative access to frame wherever possible")
 DEFINE_BOOL(turbo_control_flow_aware_allocation, true,
             "consider control flow while allocating registers")
+DEFINE_BOOL(
+    stress_turbo_late_spilling, false,
+    "optimize placement of all spill instructions, not just loop-top phis")
 
 DEFINE_STRING(turbo_filter, "*", "optimization filter for TurboFan compiler")
 DEFINE_BOOL(trace_turbo, false, "trace generated TurboFan IR")