[interpreter] Merge nested loops that share the same header offset

This CL merges nested loops that share the same header offset with its
parent loop, by not emitting JumpLoop bytecode for these inner loops.
Instead, we generate a Jump to its parent's JumpToHeader (which in
turn can be a JumpLoop or another Jump to its parent's JumpToHeader).

Originally, every loop had a unique first Bytecode to jump to. Since
IterationBody StackChecks are going to become implicit this will no
longer be the case.

As a note, this CL just sets the foundation that the follow-up CLs
will build on top of. Since we have explicit StackChecks, and they
are at the beginning of loops we do not have nested loops as of now.

Bug: v8:10149, v8:9960
Change-Id: I6daee4d2c6d6216f022228c87c4aa74e163997b2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2062390
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66626}

src/interpreter/bytecode-generator.cc

@@ -411,10 +411,7 @@ class BytecodeGenerator::ControlScopeForIteration final
                            LoopBuilder* loop_builder)
       : ControlScope(generator),
         statement_(statement),
-        loop_builder_(loop_builder) {
-    generator->loop_depth_++;
-  }
-  ~ControlScopeForIteration() override { generator()->loop_depth_--; }
+        loop_builder_(loop_builder) {}
 
  protected:
   bool Execute(Command command, Statement* statement,
@@ -953,6 +950,36 @@ class BytecodeGenerator::OptionalChainNullLabelScope final {
   BytecodeLabels* prev_;
 };
 
+// LoopScope delimits the scope of {loop}, from its header to its final jump.
+// It should be constructed iff a (conceptual) back edge should be produced. In
+// the case of creating a LoopBuilder but never emitting the loop, it is valid
+// to skip the creation of LoopScope.
+class BytecodeGenerator::LoopScope final {
+ public:
+  explicit LoopScope(BytecodeGenerator* bytecode_generator, LoopBuilder* loop)
+      : bytecode_generator_(bytecode_generator),
+        parent_loop_scope_(bytecode_generator_->current_loop_scope()),
+        loop_builder_(loop) {
+    loop_builder_->LoopHeader();
+    bytecode_generator_->set_current_loop_scope(this);
+    bytecode_generator_->loop_depth_++;
+  }
+
+  ~LoopScope() {
+    bytecode_generator_->loop_depth_--;
+    bytecode_generator_->set_current_loop_scope(parent_loop_scope_);
+    DCHECK_GE(bytecode_generator_->loop_depth_, 0);
+    loop_builder_->JumpToHeader(
+        bytecode_generator_->loop_depth_,
+        parent_loop_scope_ ? parent_loop_scope_->loop_builder_ : nullptr);
+  }
+
+ private:
+  BytecodeGenerator* const bytecode_generator_;
+  LoopScope* const parent_loop_scope_;
+  LoopBuilder* const loop_builder_;
+};
+
 namespace {
 
 template <typename PropertyT>
@@ -1043,6 +1070,7 @@ BytecodeGenerator::BytecodeGenerator(
       generator_jump_table_(nullptr),
       suspend_count_(0),
       loop_depth_(0),
+      current_loop_scope_(nullptr),
       catch_prediction_(HandlerTable::UNCAUGHT) {
   DCHECK_EQ(closure_scope(), closure_scope()->GetClosureScope());
   if (info->has_source_range_map()) {
@@ -1840,20 +1868,22 @@ void BytecodeGenerator::VisitIterationBody(IterationStatement* stmt,
 void BytecodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) {
   LoopBuilder loop_builder(builder(), block_coverage_builder_, stmt);
   if (stmt->cond()->ToBooleanIsFalse()) {
+    // Since we know that the condition is false, we don't create a loop.
+    // Therefore, we don't create a LoopScope (and thus we don't create a header
+    // and a JumpToHeader). However, we still need to iterate once through the
+    // body.
     VisitIterationBody(stmt, &loop_builder);
   } else if (stmt->cond()->ToBooleanIsTrue()) {
-    loop_builder.LoopHeader();
+    LoopScope loop_scope(this, &loop_builder);
     VisitIterationBody(stmt, &loop_builder);
-    loop_builder.JumpToHeader(loop_depth_);
   } else {
-    loop_builder.LoopHeader();
+    LoopScope loop_scope(this, &loop_builder);
     VisitIterationBody(stmt, &loop_builder);
     builder()->SetExpressionAsStatementPosition(stmt->cond());
     BytecodeLabels loop_backbranch(zone());
     VisitForTest(stmt->cond(), &loop_backbranch, loop_builder.break_labels(),
                  TestFallthrough::kThen);
     loop_backbranch.Bind(builder());
-    loop_builder.JumpToHeader(loop_depth_);
   }
 }
 
@@ -1865,7 +1895,7 @@ void BytecodeGenerator::VisitWhileStatement(WhileStatement* stmt) {
     return;
   }
 
-  loop_builder.LoopHeader();
+  LoopScope loop_scope(this, &loop_builder);
   if (!stmt->cond()->ToBooleanIsTrue()) {
     builder()->SetExpressionAsStatementPosition(stmt->cond());
     BytecodeLabels loop_body(zone());
@@ -1874,22 +1904,21 @@ void BytecodeGenerator::VisitWhileStatement(WhileStatement* stmt) {
     loop_body.Bind(builder());
   }
   VisitIterationBody(stmt, &loop_builder);
-  loop_builder.JumpToHeader(loop_depth_);
 }
 
 void BytecodeGenerator::VisitForStatement(ForStatement* stmt) {
-  LoopBuilder loop_builder(builder(), block_coverage_builder_, stmt);
-
   if (stmt->init() != nullptr) {
     Visit(stmt->init());
   }
+
+  LoopBuilder loop_builder(builder(), block_coverage_builder_, stmt);
   if (stmt->cond() && stmt->cond()->ToBooleanIsFalse()) {
     // If the condition is known to be false there is no need to generate
     // body, next or condition blocks. Init block should be generated.
     return;
   }
 
-  loop_builder.LoopHeader();
+  LoopScope loop_scope(this, &loop_builder);
   if (stmt->cond() && !stmt->cond()->ToBooleanIsTrue()) {
     builder()->SetExpressionAsStatementPosition(stmt->cond());
     BytecodeLabels loop_body(zone());
@@ -1902,7 +1931,6 @@ void BytecodeGenerator::VisitForStatement(ForStatement* stmt) {
     builder()->SetStatementPosition(stmt->next());
     Visit(stmt->next());
   }
-  loop_builder.JumpToHeader(loop_depth_);
 }
 
 void BytecodeGenerator::VisitForInStatement(ForInStatement* stmt) {
@@ -1936,7 +1964,7 @@ void BytecodeGenerator::VisitForInStatement(ForInStatement* stmt) {
   // The loop
   {
     LoopBuilder loop_builder(builder(), block_coverage_builder_, stmt);
-    loop_builder.LoopHeader();
+    LoopScope loop_scope(this, &loop_builder);
     builder()->SetExpressionAsStatementPosition(stmt->each());
     builder()->ForInContinue(index, cache_length);
     loop_builder.BreakIfFalse(ToBooleanMode::kAlreadyBoolean);
@@ -1958,7 +1986,6 @@ void BytecodeGenerator::VisitForInStatement(ForInStatement* stmt) {
     VisitIterationBody(stmt, &loop_builder);
     builder()->ForInStep(index);
     builder()->StoreAccumulatorInRegister(index);
-    loop_builder.JumpToHeader(loop_depth_);
   }
   builder()->Bind(&subject_undefined_label);
 }
@@ -2010,7 +2037,7 @@ void BytecodeGenerator::VisitForOfStatement(ForOfStatement* stmt) {
         Register next_result = register_allocator()->NewRegister();
 
         LoopBuilder loop_builder(builder(), block_coverage_builder_, stmt);
-        loop_builder.LoopHeader();
+        LoopScope loop_scope(this, &loop_builder);
 
         builder()->LoadTrue().StoreAccumulatorInRegister(done);
 
@@ -2042,8 +2069,6 @@ void BytecodeGenerator::VisitForOfStatement(ForOfStatement* stmt) {
         BuildAssignment(lhs_data, Token::ASSIGN, LookupHoistingMode::kNormal);
 
         VisitIterationBody(stmt, &loop_builder);
-
-        loop_builder.JumpToHeader(loop_depth_);
       },
       // Finally block.
       [&](Register iteration_continuation_token) {
@@ -2857,7 +2882,7 @@ void BytecodeGenerator::BuildFillArrayWithIterator(
   DCHECK(value.is_valid());
 
   LoopBuilder loop_builder(builder(), nullptr, nullptr);
-  loop_builder.LoopHeader();
+  LoopScope loop_scope(this, &loop_builder);
 
   // Call the iterator's .next() method. Break from the loop if the `done`
   // property is truthy, otherwise load the value from the iterator result and
@@ -2880,7 +2905,6 @@ void BytecodeGenerator::BuildFillArrayWithIterator(
       .UnaryOperation(Token::INC, feedback_index(index_slot))
       .StoreAccumulatorInRegister(index);
   loop_builder.BindContinueTarget();
-  loop_builder.JumpToHeader(loop_depth_);
 }
 
 void BytecodeGenerator::BuildCreateArrayLiteral(
@@ -4344,8 +4368,8 @@ void BytecodeGenerator::VisitYieldStar(YieldStar* expr) {
       //   - One for awaiting the iterator result yielded by the delegated
       //     iterator
 
-      LoopBuilder loop(builder(), nullptr, nullptr);
-      loop.LoopHeader();
+      LoopBuilder loop_builder(builder(), nullptr, nullptr);
+      LoopScope loop_scope(this, &loop_builder);
 
       {
         BytecodeLabels after_switch(zone());
@@ -4426,7 +4450,7 @@ void BytecodeGenerator::VisitYieldStar(YieldStar* expr) {
           output, ast_string_constants()->done_string(),
           feedback_index(feedback_spec()->AddLoadICSlot()));
 
-      loop.BreakIfTrue(ToBooleanMode::kConvertToBoolean);
+      loop_builder.BreakIfTrue(ToBooleanMode::kConvertToBoolean);
 
       // Suspend the current generator.
       if (iterator_type == IteratorType::kNormal) {
@@ -4457,8 +4481,7 @@ void BytecodeGenerator::VisitYieldStar(YieldStar* expr) {
                         generator_object())
           .StoreAccumulatorInRegister(resume_mode);
 
-      loop.BindContinueTarget();
-      loop.JumpToHeader(loop_depth_);
+      loop_builder.BindContinueTarget();
     }
   }
 

src/interpreter/bytecode-generator.h

@@ -58,6 +58,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   void VisitStatements(const ZonePtrList<Statement>* statments);
 
  private:
+  class AccumulatorPreservingScope;
   class ContextScope;
   class ControlScope;
   class ControlScopeForBreakable;
@@ -66,17 +67,17 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   class ControlScopeForTryCatch;
   class ControlScopeForTryFinally;
   class CurrentScope;
-  class ExpressionResultScope;
   class EffectResultScope;
+  class ExpressionResultScope;
   class FeedbackSlotCache;
-  class TopLevelDeclarationsBuilder;
   class IteratorRecord;
+  class LoopScope;
   class NaryCodeCoverageSlots;
+  class OptionalChainNullLabelScope;
   class RegisterAllocationScope;
-  class AccumulatorPreservingScope;
   class TestResultScope;
+  class TopLevelDeclarationsBuilder;
   class ValueResultScope;
-  class OptionalChainNullLabelScope;
 
   using ToBooleanMode = BytecodeArrayBuilder::ToBooleanMode;
 
@@ -488,6 +489,11 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
     catch_prediction_ = value;
   }
 
+  LoopScope* current_loop_scope() const { return current_loop_scope_; }
+  void set_current_loop_scope(LoopScope* loop_scope) {
+    current_loop_scope_ = loop_scope;
+  }
+
   Zone* zone_;
   BytecodeArrayBuilder builder_;
   UnoptimizedCompilationInfo* info_;
@@ -524,8 +530,11 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
 
   BytecodeJumpTable* generator_jump_table_;
   int suspend_count_;
+  // TODO(solanes): assess if we can move loop_depth_ into LoopScope.
   int loop_depth_;
 
+  LoopScope* current_loop_scope_;
+
   HandlerTable::CatchPrediction catch_prediction_;
 };
 

src/interpreter/control-flow-builders.cc

@@ -47,6 +47,7 @@ void BreakableControlFlowBuilder::EmitJumpIfNull(BytecodeLabels* sites) {
 
 LoopBuilder::~LoopBuilder() {
   DCHECK(continue_labels_.empty() || continue_labels_.is_bound());
+  DCHECK(end_labels_.empty() || end_labels_.is_bound());
 }
 
 void LoopBuilder::LoopHeader() {
@@ -54,7 +55,8 @@ void LoopBuilder::LoopHeader() {
   // requirements of bytecode basic blocks. The only entry into a loop
   // must be the loop header. Surely breaks is okay? Not if nested
   // and misplaced between the headers.
-  DCHECK(break_labels_.empty() && continue_labels_.empty());
+  DCHECK(break_labels_.empty() && continue_labels_.empty() &&
+         end_labels_.empty());
   builder()->Bind(&loop_header_);
 }
 
@@ -64,17 +66,30 @@ void LoopBuilder::LoopBody() {
   }
 }
 
-void LoopBuilder::JumpToHeader(int loop_depth) {
-  // Pass the proper loop nesting level to the backwards branch, to trigger
-  // on-stack replacement when armed for the given loop nesting depth.
-  int level = Min(loop_depth, AbstractCode::kMaxLoopNestingMarker - 1);
-  // Loop must have closed form, i.e. all loop elements are within the loop,
-  // the loop header precedes the body and next elements in the loop.
-  builder()->JumpLoop(&loop_header_, level);
+void LoopBuilder::JumpToHeader(int loop_depth, LoopBuilder* const parent_loop) {
+  BindLoopEnd();
+  if (parent_loop &&
+      loop_header_.offset() == parent_loop->loop_header_.offset()) {
+    // TurboFan can't cope with multiple loops that have the same loop header
+    // bytecode offset. If we have an inner loop with the same header offset
+    // than its parent loop, we do not create a JumpLoop bytecode. Instead, we
+    // Jump to our parent's JumpToHeader which in turn can be a JumpLoop or, iff
+    // they are a nested inner loop too, a Jump to its parent's JumpToHeader.
+    parent_loop->JumpToLoopEnd();
+  } else {
+    // Pass the proper loop nesting level to the backwards branch, to trigger
+    // on-stack replacement when armed for the given loop nesting depth.
+    int level = Min(loop_depth, AbstractCode::kMaxLoopNestingMarker - 1);
+    // Loop must have closed form, i.e. all loop elements are within the loop,
+    // the loop header precedes the body and next elements in the loop.
+    builder()->JumpLoop(&loop_header_, level);
+  }
 }
 
 void LoopBuilder::BindContinueTarget() { continue_labels_.Bind(builder()); }
 
+void LoopBuilder::BindLoopEnd() { end_labels_.Bind(builder()); }
+
 SwitchBuilder::~SwitchBuilder() {
 #ifdef DEBUG
   for (auto site : case_sites_) {

src/interpreter/control-flow-builders.h

@@ -9,6 +9,7 @@
 
 #include "src/ast/ast-source-ranges.h"
 #include "src/interpreter/block-coverage-builder.h"
+#include "src/interpreter/bytecode-generator.h"
 #include "src/interpreter/bytecode-label.h"
 #include "src/zone/zone-containers.h"
 
@@ -79,7 +80,6 @@ class V8_EXPORT_PRIVATE BreakableControlFlowBuilder
   BlockCoverageBuilder* block_coverage_builder_;
 };
 
-
 // Class to track control flow for block statements (which can break in JS).
 class V8_EXPORT_PRIVATE BlockBuilder final
     : public BreakableControlFlowBuilder {
@@ -91,7 +91,6 @@ class V8_EXPORT_PRIVATE BlockBuilder final
                                     statement) {}
 };
 
-
 // A class to help with co-ordinating break and continue statements with
 // their loop.
 class V8_EXPORT_PRIVATE LoopBuilder final : public BreakableControlFlowBuilder {
@@ -99,7 +98,8 @@ class V8_EXPORT_PRIVATE LoopBuilder final : public BreakableControlFlowBuilder {
   LoopBuilder(BytecodeArrayBuilder* builder,
               BlockCoverageBuilder* block_coverage_builder, AstNode* node)
       : BreakableControlFlowBuilder(builder, block_coverage_builder, node),
-        continue_labels_(builder->zone()) {
+        continue_labels_(builder->zone()),
+        end_labels_(builder->zone()) {
     if (block_coverage_builder_ != nullptr) {
       block_coverage_body_slot_ =
           block_coverage_builder_->AllocateBlockCoverageSlot(
@@ -110,7 +110,7 @@ class V8_EXPORT_PRIVATE LoopBuilder final : public BreakableControlFlowBuilder {
 
   void LoopHeader();
   void LoopBody();
-  void JumpToHeader(int loop_depth);
+  void JumpToHeader(int loop_depth, LoopBuilder* const parent_loop);
   void BindContinueTarget();
 
   // This method is called when visiting continue statements in the AST.
@@ -121,16 +121,27 @@ class V8_EXPORT_PRIVATE LoopBuilder final : public BreakableControlFlowBuilder {
   void ContinueIfNull() { EmitJumpIfNull(&continue_labels_); }
 
  private:
+  // Emit a Jump to our parent_loop_'s end label which could be a JumpLoop or,
+  // iff they are a nested inner loop with the same loop header bytecode offset
+  // as their parent's, a Jump to its parent's end label.
+  void JumpToLoopEnd() { EmitJump(&end_labels_); }
+  void BindLoopEnd();
+
   BytecodeLoopHeader loop_header_;
 
   // Unbound labels that identify jumps for continue statements in the code and
   // jumps from checking the loop condition to the header for do-while loops.
   BytecodeLabels continue_labels_;
 
+  // Unbound labels that identify jumps for nested inner loops which share the
+  // same header offset as this loop. Said inner loops will Jump to our end
+  // label, which could be a JumpLoop or, iff we are a nested inner loop too, a
+  // Jump to our parent's end label.
+  BytecodeLabels end_labels_;
+
   int block_coverage_body_slot_;
 };
 
-
 // A class to help with co-ordinating break statements with their switch.
 class V8_EXPORT_PRIVATE SwitchBuilder final
     : public BreakableControlFlowBuilder {
@@ -165,7 +176,6 @@ class V8_EXPORT_PRIVATE SwitchBuilder final
   ZoneVector<BytecodeLabel> case_sites_;
 };
 
-
 // A class to help with co-ordinating control flow in try-catch statements.
 class V8_EXPORT_PRIVATE TryCatchBuilder final : public ControlFlowBuilder {
  public:
@@ -194,7 +204,6 @@ class V8_EXPORT_PRIVATE TryCatchBuilder final : public ControlFlowBuilder {
   TryCatchStatement* statement_;
 };
 
-
 // A class to help with co-ordinating control flow in try-finally statements.
 class V8_EXPORT_PRIVATE TryFinallyBuilder final : public ControlFlowBuilder {
  public:

test/unittests/compiler/bytecode-analysis-unittest.cc

@@ -256,7 +256,7 @@ TEST_F(BytecodeAnalysisTest, SimpleLoop) {
     expected_liveness.emplace_back("L..L", "L.L.");
 
     loop_builder.BindContinueTarget();
-    loop_builder.JumpToHeader(0);
+    loop_builder.JumpToHeader(0, nullptr);
     expected_liveness.emplace_back("L.L.", "L.L.");
   }
 
@@ -361,7 +361,7 @@ TEST_F(BytecodeAnalysisTest, DiamondInLoop) {
     builder.Bind(&end_label);
 
     loop_builder.BindContinueTarget();
-    loop_builder.JumpToHeader(0);
+    loop_builder.JumpToHeader(0, nullptr);
     expected_liveness.emplace_back("L...", "L...");
   }
 
@@ -433,12 +433,12 @@ TEST_F(BytecodeAnalysisTest, KillingLoopInsideLoop) {
       expected_liveness.emplace_back("LL.L", "LL..");
 
       inner_loop_builder.BindContinueTarget();
-      inner_loop_builder.JumpToHeader(1);
+      inner_loop_builder.JumpToHeader(1, &loop_builder);
       expected_liveness.emplace_back(".L..", ".L..");
     }
 
     loop_builder.BindContinueTarget();
-    loop_builder.JumpToHeader(0);
+    loop_builder.JumpToHeader(0, nullptr);
     expected_liveness.emplace_back("LL..", "LL..");
   }