Reland "[regalloc] Place spill instructions optimally"

This is a reland of f4548e75

Original change's description:
> [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}

Bug: v8:10606
Change-Id: I10fc1ef4b0bebb6c9f55ebdefe33e8c1e5646f0a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2352483
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@{#69427}

BUILD.gn

@@ -1942,6 +1942,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

@@ -2245,16 +2245,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)
 
@@ -3701,15 +3691,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

@@ -546,6 +546,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")