[gasm] Automatically mark all loop exits as loop peeling candidates

The loop peeling optimization requires all loop exits to be marked
with {LoopExit,LoopExitEffect,LoopExitValue} nodes in order to peel
the first loop iteration. Previously, the graph assembler only marked
the default loop exit (taken once the loop condition evaluates to
false).

This CL adds more general support, such that all exits taken inside
the loop body passed to a ForBuilder are automatically marked. We do
this by tracking the current loop nesting level and a stack of loop
headers inside the graph assembler, and creating marker nodes as needed
inside MergeState.

Bug: v8:9972,chromium:1038297
Change-Id: I1d0196ead55d6678880f8330c7cc7b8d4f2cea06
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2000740
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65861}

src/compiler/graph-assembler.cc

@@ -336,16 +336,18 @@ BasicBlock* GraphAssembler::BasicBlockUpdater::Finalize(BasicBlock* original) {
 }
 
 GraphAssembler::GraphAssembler(MachineGraph* mcgraph, Zone* zone,
-                               Schedule* schedule)
+                               Schedule* schedule, bool mark_loop_exits)
     : temp_zone_(zone),
       mcgraph_(mcgraph),
       effect_(nullptr),
       control_(nullptr),
       block_updater_(schedule != nullptr ? new BasicBlockUpdater(
                                                schedule, mcgraph->graph(), zone)
-                                         : nullptr) {}
+                                         : nullptr),
+      loop_headers_(zone),
+      mark_loop_exits_(mark_loop_exits) {}
 
-GraphAssembler::~GraphAssembler() = default;
+GraphAssembler::~GraphAssembler() { DCHECK_EQ(loop_nesting_level_, 0); }
 
 Node* GraphAssembler::IntPtrConstant(intptr_t value) {
   return AddClonedNode(mcgraph()->IntPtrConstant(value));

src/compiler/graph-assembler.h

@@ -181,27 +181,13 @@ class GraphAssemblerLabel {
     return TNode<T>::UncheckedCast(PhiAt(index));
   }
 
-  template <typename... Reps>
-  explicit GraphAssemblerLabel(GraphAssemblerLabelType type,
-                               BasicBlock* basic_block, Reps... reps)
-      : type_(type), basic_block_(basic_block) {
-    STATIC_ASSERT(VarCount == sizeof...(reps));
-    MachineRepresentation reps_array[] = {MachineRepresentation::kNone,
-                                          reps...};
-    for (size_t i = 0; i < VarCount; i++) {
-      representations_[i] = reps_array[i + 1];
-    }
-  }
-
-  // Multiple merge variables with the same representation.
-  explicit GraphAssemblerLabel(GraphAssemblerLabelType type,
-                               BasicBlock* basic_block,
-                               MachineRepresentation rep)
-      : type_(type), basic_block_(basic_block) {
-    for (size_t i = 0; i < VarCount; i++) {
-      representations_[i] = rep;
-    }
-  }
+  GraphAssemblerLabel(GraphAssemblerLabelType type, BasicBlock* basic_block,
+                      int loop_nesting_level,
+                      const std::array<MachineRepresentation, VarCount>& reps)
+      : type_(type),
+        basic_block_(basic_block),
+        loop_nesting_level_(loop_nesting_level),
+        representations_(reps) {}
 
   ~GraphAssemblerLabel() { DCHECK(IsBound() || merged_count_ == 0); }
 
@@ -220,13 +206,14 @@ class GraphAssemblerLabel {
   BasicBlock* basic_block() { return basic_block_; }
 
   bool is_bound_ = false;
-  GraphAssemblerLabelType const type_;
-  BasicBlock* basic_block_;
+  const GraphAssemblerLabelType type_;
+  BasicBlock* const basic_block_;
+  const int loop_nesting_level_;
   size_t merged_count_ = 0;
   Node* effect_;
   Node* control_;
   std::array<Node*, VarCount> bindings_;
-  std::array<MachineRepresentation, VarCount> representations_;
+  const std::array<MachineRepresentation, VarCount> representations_;
 };
 
 class V8_EXPORT_PRIVATE GraphAssembler {
@@ -234,7 +221,7 @@ class V8_EXPORT_PRIVATE GraphAssembler {
   // Constructs a GraphAssembler. If {schedule} is not null, the graph assembler
   // will maintain the schedule as it updates blocks.
   GraphAssembler(MachineGraph* jsgraph, Zone* zone,
-                 Schedule* schedule = nullptr);
+                 Schedule* schedule = nullptr, bool mark_loop_exits = false);
   virtual ~GraphAssembler();
 
   void Reset(BasicBlock* block);
@@ -244,9 +231,18 @@ class V8_EXPORT_PRIVATE GraphAssembler {
   template <typename... Reps>
   GraphAssemblerLabel<sizeof...(Reps)> MakeLabelFor(
       GraphAssemblerLabelType type, Reps... reps) {
-    return GraphAssemblerLabel<sizeof...(Reps)>(
+    std::array<MachineRepresentation, sizeof...(Reps)> reps_array = {reps...};
+    return MakeLabel<sizeof...(Reps)>(reps_array, type);
+  }
+
+  // As above, but with an std::array of machine representations.
+  template <int VarCount>
+  GraphAssemblerLabel<VarCount> MakeLabel(
+      std::array<MachineRepresentation, VarCount> reps_array,
+      GraphAssemblerLabelType type) {
+    return GraphAssemblerLabel<VarCount>(
         type, NewBasicBlock(type == GraphAssemblerLabelType::kDeferred),
-        reps...);
+        loop_nesting_level_, reps_array);
   }
 
   // Convenience wrapper for creating non-deferred labels.
@@ -420,6 +416,79 @@ class V8_EXPORT_PRIVATE GraphAssembler {
   CommonOperatorBuilder* common() const { return mcgraph()->common(); }
   MachineOperatorBuilder* machine() const { return mcgraph()->machine(); }
 
+  // Updates machinery for creating {LoopExit,LoopExitEffect,LoopExitValue}
+  // nodes on loop exits (which are necessary for loop peeling).
+  //
+  // All labels created while a LoopScope is live are considered to be inside
+  // the loop.
+  template <MachineRepresentation... Reps>
+  class LoopScope final {
+   private:
+    // The internal scope is only here to increment the graph assembler's
+    // nesting level prior to `loop_header_label` creation below.
+    class LoopScopeInternal {
+     public:
+      explicit LoopScopeInternal(GraphAssembler* gasm)
+          : previous_loop_nesting_level_(gasm->loop_nesting_level_),
+            gasm_(gasm) {
+        gasm->loop_nesting_level_++;
+      }
+
+      ~LoopScopeInternal() {
+        gasm_->loop_nesting_level_--;
+        DCHECK_EQ(gasm_->loop_nesting_level_, previous_loop_nesting_level_);
+      }
+
+     private:
+      const int previous_loop_nesting_level_;
+      GraphAssembler* const gasm_;
+    };
+
+   public:
+    explicit LoopScope(GraphAssembler* gasm)
+        : internal_scope_(gasm),
+          gasm_(gasm),
+          loop_header_label_(gasm->MakeLoopLabel(Reps...)) {
+      // This feature may only be used if it has been enabled.
+      DCHECK(gasm_->mark_loop_exits_);
+      gasm->loop_headers_.push_back(&loop_header_label_.control_);
+      DCHECK_EQ(static_cast<int>(gasm_->loop_headers_.size()),
+                gasm_->loop_nesting_level_);
+    }
+
+    ~LoopScope() {
+      DCHECK_EQ(static_cast<int>(gasm_->loop_headers_.size()),
+                gasm_->loop_nesting_level_);
+      gasm_->loop_headers_.pop_back();
+    }
+
+    GraphAssemblerLabel<sizeof...(Reps)>* loop_header_label() {
+      return &loop_header_label_;
+    }
+
+   private:
+    const LoopScopeInternal internal_scope_;
+    GraphAssembler* const gasm_;
+    GraphAssemblerLabel<sizeof...(Reps)> loop_header_label_;
+  };
+
+  // Upon destruction, restores effect and control to the state at construction.
+  class RestoreEffectControlScope {
+   public:
+    explicit RestoreEffectControlScope(GraphAssembler* gasm)
+        : gasm_(gasm), effect_(gasm->effect()), control_(gasm->control()) {}
+
+    ~RestoreEffectControlScope() {
+      gasm_->effect_ = effect_;
+      gasm_->control_ = control_;
+    }
+
+   private:
+    GraphAssembler* const gasm_;
+    const Effect effect_;
+    const Control control_;
+  };
+
  private:
   template <typename... Vars>
   void BranchImpl(Node* condition,
@@ -436,6 +505,16 @@ class V8_EXPORT_PRIVATE GraphAssembler {
   Node* effect_;
   Node* control_;
   std::unique_ptr<BasicBlockUpdater> block_updater_;
+
+  // Track loop information in order to properly mark loop exits with
+  // {LoopExit,LoopExitEffect,LoopExitValue} nodes. The outermost level has
+  // a nesting level of 0. See also GraphAssembler::LoopScope.
+  int loop_nesting_level_ = 0;
+  ZoneVector<Node**> loop_headers_;
+
+  // Feature configuration. As more features are added, this should be turned
+  // into a bitfield.
+  const bool mark_loop_exits_;
 };
 
 template <size_t VarCount>
@@ -448,9 +527,32 @@ Node* GraphAssemblerLabel<VarCount>::PhiAt(size_t index) {
 template <typename... Vars>
 void GraphAssembler::MergeState(GraphAssemblerLabel<sizeof...(Vars)>* label,
                                 Vars... vars) {
-  int merged_count = static_cast<int>(label->merged_count_);
+  RestoreEffectControlScope restore_effect_control_scope(this);
+
+  const int merged_count = static_cast<int>(label->merged_count_);
   static constexpr int kVarCount = sizeof...(vars);
   std::array<Node*, kVarCount> var_array = {vars...};
+
+  const bool is_loop_exit = label->loop_nesting_level_ != loop_nesting_level_;
+  if (is_loop_exit) {
+    // This feature may only be used if it has been enabled.
+    USE(mark_loop_exits_);
+    DCHECK(mark_loop_exits_);
+    // Jumping from loops to loops not supported.
+    DCHECK(!label->IsLoop());
+    // Currently only the simple case of jumping one level is supported.
+    DCHECK_EQ(label->loop_nesting_level_, loop_nesting_level_ - 1);
+    DCHECK(!loop_headers_.empty());
+    DCHECK_NOT_NULL(*loop_headers_.back());
+
+    // Mark this exit to enable loop peeling.
+    LoopExit(Control{*loop_headers_.back()});
+    LoopExitEffect();
+    for (size_t i = 0; i < kVarCount; i++) {
+      var_array[i] = LoopExitValue(var_array[i]);
+    }
+  }
+
   if (label->IsLoop()) {
     if (merged_count == 0) {
       DCHECK(!label->IsBound());
@@ -476,10 +578,10 @@ void GraphAssembler::MergeState(GraphAssemblerLabel<sizeof...(Vars)>* label,
       }
     }
   } else {
+    DCHECK(!label->IsLoop());
     DCHECK(!label->IsBound());
     if (merged_count == 0) {
       // Just set the control, effect and variables directly.
-      DCHECK(!label->IsBound());
       label->control_ = control();
       label->effect_ = effect();
       for (size_t i = 0; i < kVarCount; i++) {
@@ -527,6 +629,7 @@ void GraphAssembler::Bind(GraphAssemblerLabel<VarCount>* label) {
   DCHECK_NULL(control());
   DCHECK_NULL(effect());
   DCHECK_LT(0, label->merged_count_);
+  DCHECK_EQ(label->loop_nesting_level_, loop_nesting_level_);
 
   control_ = label->control_;
   effect_ = label->effect_;
@@ -659,8 +762,10 @@ class V8_EXPORT_PRIVATE JSGraphAssembler : public GraphAssembler {
  public:
   // Constructs a JSGraphAssembler. If {schedule} is not null, the graph
   // assembler will maintain the schedule as it updates blocks.
-  JSGraphAssembler(JSGraph* jsgraph, Zone* zone, Schedule* schedule = nullptr)
-      : GraphAssembler(jsgraph, zone, schedule), jsgraph_(jsgraph) {}
+  JSGraphAssembler(JSGraph* jsgraph, Zone* zone, Schedule* schedule = nullptr,
+                   bool mark_loop_exits = false)
+      : GraphAssembler(jsgraph, zone, schedule, mark_loop_exits),
+        jsgraph_(jsgraph) {}
 
   Node* SmiConstant(int32_t value);
   TNode<HeapObject> HeapConstant(Handle<HeapObject> object);

src/compiler/js-call-reducer.cc

@@ -42,9 +42,12 @@ namespace compiler {
 #define _ [&]()  // NOLINT(whitespace/braces)
 
 class JSCallReducerAssembler : public JSGraphAssembler {
+ private:
+  static constexpr bool kMarkLoopExits = true;
+
  public:
   JSCallReducerAssembler(JSGraph* jsgraph, Zone* zone, Node* node)
-      : JSGraphAssembler(jsgraph, zone),
+      : JSGraphAssembler(jsgraph, zone, nullptr, kMarkLoopExits),
         node_(node),
         if_exception_nodes_(zone) {
     InitializeEffectControl(NodeProperties::GetEffectInput(node),
@@ -301,27 +304,28 @@ class JSCallReducerAssembler : public JSGraphAssembler {
           step_(step) {}
 
     void Do(const For0BodyFunction& body) {
-      auto loop_header = gasm_->MakeLoopLabel(kPhiRepresentation);
-      auto loop_body = gasm_->MakeLabel();
       auto loop_exit = gasm_->MakeLabel();
 
-      gasm_->Goto(&loop_header, initial_value_);
+      {
+        GraphAssembler::LoopScope<kPhiRepresentation> loop_scope(gasm_);
+
+        auto loop_header = loop_scope.loop_header_label();
+        auto loop_body = gasm_->MakeLabel();
+
+        gasm_->Goto(loop_header, initial_value_);
 
-      gasm_->Bind(&loop_header);
-      Control loop_header_control = gasm_->control();  // For LoopExit below.
-      TNode<Number> i = loop_header.PhiAt<Number>(0);
+        gasm_->Bind(loop_header);
+        TNode<Number> i = loop_header->PhiAt<Number>(0);
 
-      gasm_->BranchWithHint(cond_(i), &loop_body, &loop_exit,
-                            BranchHint::kTrue);
+        gasm_->BranchWithHint(cond_(i), &loop_body, &loop_exit,
+                              BranchHint::kTrue);
 
-      gasm_->Bind(&loop_body);
-      body(i);
-      gasm_->Goto(&loop_header, step_(i));
+        gasm_->Bind(&loop_body);
+        body(i);
+        gasm_->Goto(loop_header, step_(i));
+      }
 
       gasm_->Bind(&loop_exit);
-      // Mark {loop_header} as a candidate for loop peeling (crbug.com/v8/8273).
-      gasm_->LoopExit(loop_header_control);
-      gasm_->LoopExitEffect();
     }
 
    private:
@@ -369,29 +373,30 @@ class JSCallReducerAssembler : public JSGraphAssembler {
       DCHECK(body_);
       TNode<Object> arg0 = initial_arg0_;
 
-      auto loop_header =
-          gasm_->MakeLoopLabel(kPhiRepresentation, kPhiRepresentation);
-      auto loop_body = gasm_->MakeDeferredLabel(kPhiRepresentation);
       auto loop_exit = gasm_->MakeDeferredLabel(kPhiRepresentation);
 
-      gasm_->Goto(&loop_header, initial_value_, initial_arg0_);
+      {
+        GraphAssembler::LoopScope<kPhiRepresentation, kPhiRepresentation>
+            loop_scope(gasm_);
+
+        auto loop_header = loop_scope.loop_header_label();
+        auto loop_body = gasm_->MakeDeferredLabel(kPhiRepresentation);
 
-      gasm_->Bind(&loop_header);
-      Control loop_header_control = gasm_->control();  // For LoopExit below.
-      TNode<Number> i = loop_header.PhiAt<Number>(0);
-      arg0 = loop_header.PhiAt<Object>(1);
+        gasm_->Goto(loop_header, initial_value_, initial_arg0_);
 
-      gasm_->BranchWithHint(cond_(i), &loop_body, &loop_exit, BranchHint::kTrue,
-                            arg0);
+        gasm_->Bind(loop_header);
+        TNode<Number> i = loop_header->PhiAt<Number>(0);
+        arg0 = loop_header->PhiAt<Object>(1);
 
-      gasm_->Bind(&loop_body);
-      body_(i, &arg0);
-      gasm_->Goto(&loop_header, step_(i), arg0);
+        gasm_->BranchWithHint(cond_(i), &loop_body, &loop_exit,
+                              BranchHint::kTrue, arg0);
+
+        gasm_->Bind(&loop_body);
+        body_(i, &arg0);
+        gasm_->Goto(loop_header, step_(i), arg0);
+      }
 
       gasm_->Bind(&loop_exit);
-      // Mark {loop_header} as a candidate for loop peeling (crbug.com/v8/8273).
-      gasm_->LoopExit(loop_header_control);
-      gasm_->LoopExitEffect();
       return TNode<Object>::UncheckedCast(
           gasm_->LoopExitValue(loop_exit.PhiAt<Object>(0)));
     }
@@ -562,14 +567,14 @@ class IteratingArrayBuiltinReducerAssembler : public JSCallReducerAssembler {
   template <typename... Vars>
   TNode<Object> MaybeSkipHole(
       TNode<Object> o, ElementsKind kind,
-      GraphAssemblerLabel<sizeof...(Vars)>* continue_label, Vars... vars) {
+      GraphAssemblerLabel<sizeof...(Vars)>* continue_label,
+      TNode<Vars>... vars) {
     if (!IsHoleyElementsKind(kind)) return o;
 
-    // TODO(jgruber): Currently we just assume all merge variables to have a
-    // tagged representation.
-    auto if_not_hole = GraphAssemblerLabel<sizeof...(Vars)>(
-        GraphAssemblerLabelType::kNonDeferred, NewBasicBlock(false),
-        MachineRepresentation::kTagged);
+    std::array<MachineRepresentation, sizeof...(Vars)> reps = {
+        MachineRepresentationOf<Vars>::value...};
+    auto if_not_hole =
+        MakeLabel<sizeof...(Vars)>(reps, GraphAssemblerLabelType::kNonDeferred);
     BranchWithHint(HoleCheck(kind, o), continue_label, &if_not_hole,
                    BranchHint::kFalse, vars...);
 
@@ -1074,7 +1079,11 @@ TNode<Object> IteratingArrayBuiltinReducerAssembler::ReduceArrayPrototypeReduce(
       TNode<Object> element;
       std::tie(k, element) = SafeLoadElement(kind, receiver, k);
 
-      GotoIfNot(HoleCheck(kind, element), &found_initial_element, k, element);
+      auto continue_label = MakeLabel();
+      GotoIf(HoleCheck(kind, element), &continue_label);
+      Goto(&found_initial_element, k, TypeGuardNonInternal(element));
+
+      Bind(&continue_label);
     });
     Unreachable();  // The loop is exited either by deopt or a jump to below.
     InitializeEffectControl(nullptr, nullptr);
@@ -1084,7 +1093,6 @@ TNode<Object> IteratingArrayBuiltinReducerAssembler::ReduceArrayPrototypeReduce(
     Bind(&found_initial_element);
     k = step(found_initial_element.PhiAt<Number>(0));
     accumulator = found_initial_element.PhiAt<Object>(1);
-    accumulator = TypeGuardNonInternal(accumulator);
   }
 
   TNode<Object> result =