[Interpreter] Increase allowed expression depth for logical test expressions

Changes the handling of TestResultScopes to allow them to be reused by
logical tests by rewiring instead of using a new TestResultScope. 
Also does the following:
 - moves some fields about in TestResultScope to reduce it's size
 - moves RegisterListFreeEvent to the end of ReleaseRegisters to enable
   it to be tail-called.

This increases the allowed depth of logical expressions which the
compiler can handle without overflowing the stack by about 2x on x64.

BUG=chromium:731861

Change-Id: I7733797bec5e52d07eec6332c07e2a886f2bbde1
Reviewed-on: https://chromium-review.googlesource.com/539521
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46039}

src/interpreter/bytecode-generator.cc

@@ -578,10 +578,10 @@ class BytecodeGenerator::TestResultScope final : public ExpressionResultScope {
   TestResultScope(BytecodeGenerator* generator, BytecodeLabels* then_labels,
                   BytecodeLabels* else_labels, TestFallthrough fallthrough)
       : ExpressionResultScope(generator, Kind::kTest),
-        then_labels_(then_labels),
-        else_labels_(else_labels),
+        result_consumed_by_test_(false),
         fallthrough_(fallthrough),
-        result_consumed_by_test_(false) {}
+        then_labels_(then_labels),
+        else_labels_(else_labels) {}
 
   // Used when code special cases for TestResultScope and consumes any
   // possible value by testing and jumping to a then/else label.
@@ -590,12 +590,26 @@ class BytecodeGenerator::TestResultScope final : public ExpressionResultScope {
   }
   bool result_consumed_by_test() { return result_consumed_by_test_; }
 
+  // Inverts the control flow of the operation, swapping the then and else
+  // labels and the fallthrough.
+  void InvertControlFlow() {
+    std::swap(then_labels_, else_labels_);
+    fallthrough_ = inverted_fallthrough();
+  }
+
   BytecodeLabel* NewThenLabel() { return then_labels_->New(); }
   BytecodeLabel* NewElseLabel() { return else_labels_->New(); }
 
   BytecodeLabels* then_labels() const { return then_labels_; }
   BytecodeLabels* else_labels() const { return else_labels_; }
 
+  void set_then_labels(BytecodeLabels* then_labels) {
+    then_labels_ = then_labels;
+  }
+  void set_else_labels(BytecodeLabels* else_labels) {
+    else_labels_ = else_labels;
+  }
+
   TestFallthrough fallthrough() const { return fallthrough_; }
   TestFallthrough inverted_fallthrough() const {
     switch (fallthrough_) {
@@ -607,12 +621,15 @@ class BytecodeGenerator::TestResultScope final : public ExpressionResultScope {
         return TestFallthrough::kNone;
     }
   }
+  void set_fallthrough(TestFallthrough fallthrough) {
+    fallthrough_ = fallthrough;
+  }
 
  private:
+  bool result_consumed_by_test_;
+  TestFallthrough fallthrough_;
   BytecodeLabels* then_labels_;
   BytecodeLabels* else_labels_;
-  TestFallthrough fallthrough_;
-  bool result_consumed_by_test_;
 
   DISALLOW_COPY_AND_ASSIGN(TestResultScope);
 };
@@ -625,8 +642,8 @@ class BytecodeGenerator::AdditionResultScope final
   explicit AdditionResultScope(BytecodeGenerator* generator,
                                RegisterList* operand_registers)
       : ExpressionResultScope(generator, Kind::kAddition),
-        operand_registers_(operand_registers),
-        result_deferred_until_concat_(false) {}
+        result_deferred_until_concat_(false),
+        operand_registers_(operand_registers) {}
 
   RegisterList* operand_registers() { return operand_registers_; }
 
@@ -642,8 +659,10 @@ class BytecodeGenerator::AdditionResultScope final
   bool result_deferred_until_concat() { return result_deferred_until_concat_; }
 
  private:
-  RegisterList* operand_registers_;
   bool result_deferred_until_concat_;
+  RegisterList* operand_registers_;
+
+  DISALLOW_COPY_AND_ASSIGN(AdditionResultScope);
 };
 
 // Used to build a list of global declaration initial value pairs.
@@ -3022,19 +3041,18 @@ void BytecodeGenerator::VisitNot(UnaryOperation* expr) {
   if (execution_result()->IsEffect()) {
     VisitForEffect(expr->expression());
   } else if (execution_result()->IsTest()) {
+    // No actual logical negation happening, we just swap the control flow, by
+    // swapping the target labels and the fallthrough branch, and visit in the
+    // same test result context.
     TestResultScope* test_result = execution_result()->AsTest();
-    // No actual logical negation happening, we just swap the control flow by
-    // swapping the target labels and the fallthrough branch.
-    VisitForTest(expr->expression(), test_result->else_labels(),
-                 test_result->then_labels(),
-                 test_result->inverted_fallthrough());
-    test_result->SetResultConsumedByTest();
+    test_result->InvertControlFlow();
+    VisitInSameTestExecutionScope(expr->expression());
   } else {
     TypeHint type_hint = VisitForAccumulatorValue(expr->expression());
     builder()->LogicalNot(ToBooleanModeFromTypeHint(type_hint));
-  }
     // Always returns a boolean value.
     execution_result()->set_type_hint(TypeHint::kBoolean);
+  }
 }
 
 void BytecodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
@@ -3506,24 +3524,43 @@ void BytecodeGenerator::VisitCommaExpression(BinaryOperation* binop) {
   Visit(binop->right());
 }
 
+void BytecodeGenerator::BuildLogicalTest(Token::Value token, Expression* left,
+                                         Expression* right) {
+  DCHECK(token == Token::OR || token == Token::AND);
+  TestResultScope* test_result = execution_result()->AsTest();
+  BytecodeLabels* then_labels = test_result->then_labels();
+  BytecodeLabels* else_labels = test_result->else_labels();
+  TestFallthrough fallthrough = test_result->fallthrough();
+  {
+    // Visit the left side using current TestResultScope.
+    BytecodeLabels test_right(zone());
+    if (token == Token::OR) {
+      test_result->set_fallthrough(TestFallthrough::kElse);
+      test_result->set_else_labels(&test_right);
+    } else {
+      DCHECK_EQ(Token::AND, token);
+      test_result->set_fallthrough(TestFallthrough::kThen);
+      test_result->set_then_labels(&test_right);
+    }
+    VisitInSameTestExecutionScope(left);
+    test_right.Bind(builder());
+  }
+  // Visit the right side in a new TestResultScope.
+  VisitForTest(right, then_labels, else_labels, fallthrough);
+}
+
 void BytecodeGenerator::VisitLogicalOrExpression(BinaryOperation* binop) {
   Expression* left = binop->left();
   Expression* right = binop->right();
 
   if (execution_result()->IsTest()) {
     TestResultScope* test_result = execution_result()->AsTest();
-
     if (left->ToBooleanIsTrue()) {
       builder()->Jump(test_result->NewThenLabel());
     } else if (left->ToBooleanIsFalse() && right->ToBooleanIsFalse()) {
       builder()->Jump(test_result->NewElseLabel());
     } else {
-      BytecodeLabels test_right(zone());
-      VisitForTest(left, test_result->then_labels(), &test_right,
-                   TestFallthrough::kElse);
-      test_right.Bind(builder());
-      VisitForTest(right, test_result->then_labels(),
-                   test_result->else_labels(), test_result->fallthrough());
+      BuildLogicalTest(Token::OR, left, right);
     }
     test_result->SetResultConsumedByTest();
   } else {
@@ -3547,18 +3584,12 @@ void BytecodeGenerator::VisitLogicalAndExpression(BinaryOperation* binop) {
 
   if (execution_result()->IsTest()) {
     TestResultScope* test_result = execution_result()->AsTest();
-
     if (left->ToBooleanIsFalse()) {
       builder()->Jump(test_result->NewElseLabel());
     } else if (left->ToBooleanIsTrue() && right->ToBooleanIsTrue()) {
       builder()->Jump(test_result->NewThenLabel());
     } else {
-      BytecodeLabels test_right(zone());
-      VisitForTest(left, &test_right, test_result->else_labels(),
-                   TestFallthrough::kThen);
-      test_right.Bind(builder());
-      VisitForTest(right, test_result->then_labels(),
-                   test_result->else_labels(), test_result->fallthrough());
+      BuildLogicalTest(Token::AND, left, right);
     }
     test_result->SetResultConsumedByTest();
   } else {
@@ -3803,6 +3834,35 @@ void BytecodeGenerator::VisitFunctionClosureForContext() {
   }
 }
 
+void BytecodeGenerator::BuildPushUndefinedIntoRegisterList(
+    RegisterList* reg_list) {
+  Register reg = register_allocator()->GrowRegisterList(reg_list);
+  builder()->LoadUndefined().StoreAccumulatorInRegister(reg);
+}
+
+void BytecodeGenerator::BuildLoadPropertyKey(LiteralProperty* property,
+                                             Register out_reg) {
+  if (property->key()->IsStringLiteral()) {
+    VisitForRegisterValue(property->key(), out_reg);
+  } else {
+    VisitForAccumulatorValue(property->key());
+    builder()->ToName(out_reg);
+  }
+}
+
+int BytecodeGenerator::AllocateBlockCoverageSlotIfEnabled(SourceRange range) {
+  return (block_coverage_builder_ == nullptr)
+             ? BlockCoverageBuilder::kNoCoverageArraySlot
+             : block_coverage_builder_->AllocateBlockCoverageSlot(range);
+}
+
+void BytecodeGenerator::BuildIncrementBlockCoverageCounterIfEnabled(
+    int coverage_array_slot) {
+  if (block_coverage_builder_ != nullptr) {
+    block_coverage_builder_->IncrementBlockCounter(coverage_array_slot);
+  }
+}
+
 // Visits the expression |expr| and places the result in the accumulator.
 BytecodeGenerator::TypeHint BytecodeGenerator::VisitForAccumulatorValue(
     Expression* expr) {
@@ -3878,32 +3938,20 @@ void BytecodeGenerator::VisitAndPushIntoRegisterList(Expression* expr,
   builder()->StoreAccumulatorInRegister(destination);
 }
 
-void BytecodeGenerator::BuildPushUndefinedIntoRegisterList(
-    RegisterList* reg_list) {
-  Register reg = register_allocator()->GrowRegisterList(reg_list);
-  builder()->LoadUndefined().StoreAccumulatorInRegister(reg);
-}
-
-void BytecodeGenerator::BuildLoadPropertyKey(LiteralProperty* property,
-                                             Register out_reg) {
-  if (property->key()->IsStringLiteral()) {
-    VisitForRegisterValue(property->key(), out_reg);
-  } else {
-    VisitForAccumulatorValue(property->key());
-    builder()->ToName(out_reg);
-  }
-}
-
-int BytecodeGenerator::AllocateBlockCoverageSlotIfEnabled(SourceRange range) {
-  return (block_coverage_builder_ == nullptr)
-             ? BlockCoverageBuilder::kNoCoverageArraySlot
-             : block_coverage_builder_->AllocateBlockCoverageSlot(range);
-}
-
-void BytecodeGenerator::BuildIncrementBlockCoverageCounterIfEnabled(
-    int coverage_array_slot) {
-  if (block_coverage_builder_ != nullptr) {
-    block_coverage_builder_->IncrementBlockCounter(coverage_array_slot);
+void BytecodeGenerator::BuildTest(ToBooleanMode mode,
+                                  BytecodeLabels* then_labels,
+                                  BytecodeLabels* else_labels,
+                                  TestFallthrough fallthrough) {
+  switch (fallthrough) {
+    case TestFallthrough::kThen:
+      builder()->JumpIfFalse(mode, else_labels->New());
+      break;
+    case TestFallthrough::kElse:
+      builder()->JumpIfTrue(mode, then_labels->New());
+      break;
+    case TestFallthrough::kNone:
+      builder()->JumpIfTrue(mode, then_labels->New());
+      builder()->Jump(else_labels->New());
   }
 }
 
@@ -3923,20 +3971,30 @@ void BytecodeGenerator::VisitForTest(Expression* expr,
     Visit(expr);
     result_consumed = test_result.result_consumed_by_test();
     type_hint = test_result.type_hint();
+    // Labels and fallthrough might have been mutated, so update based on
+    // TestResultScope.
+    then_labels = test_result.then_labels();
+    else_labels = test_result.else_labels();
+    fallthrough = test_result.fallthrough();
   }
   if (!result_consumed) {
-    ToBooleanMode mode(ToBooleanModeFromTypeHint(type_hint));
-    switch (fallthrough) {
-      case TestFallthrough::kThen:
-        builder()->JumpIfFalse(mode, else_labels->New());
-        break;
-      case TestFallthrough::kElse:
-        builder()->JumpIfTrue(mode, then_labels->New());
-        break;
-      case TestFallthrough::kNone:
-        builder()->JumpIfTrue(mode, then_labels->New());
-        builder()->Jump(else_labels->New());
+    BuildTest(ToBooleanModeFromTypeHint(type_hint), then_labels, else_labels,
+              fallthrough);
+  }
+}
+
+void BytecodeGenerator::VisitInSameTestExecutionScope(Expression* expr) {
+  DCHECK(execution_result()->IsTest());
+  {
+    RegisterAllocationScope reg_scope(this);
+    Visit(expr);
   }
+  if (!execution_result()->AsTest()->result_consumed_by_test()) {
+    TestResultScope* result_scope = execution_result()->AsTest();
+    BuildTest(ToBooleanModeFromTypeHint(result_scope->type_hint()),
+              result_scope->then_labels(), result_scope->else_labels(),
+              result_scope->fallthrough());
+    result_scope->SetResultConsumedByTest();
   }
 }
 

src/interpreter/bytecode-generator.h

@@ -159,6 +159,11 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   void VisitForInAssignment(Expression* expr, FeedbackSlot slot);
   void VisitModuleNamespaceImports();
 
+  // Builds a logical OR/AND within a test context by rewiring the jumps based
+  // on the expression values.
+  void BuildLogicalTest(Token::Value token, Expression* left,
+                        Expression* right);
+
   // Builds an addition expression. If the result is a known string addition,
   // then rather than emitting the add, the operands will converted to
   // primitive, then to string and stored in registers in the
@@ -181,6 +186,9 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   int AllocateBlockCoverageSlotIfEnabled(SourceRange range);
   void BuildIncrementBlockCoverageCounterIfEnabled(int coverage_array_slot);
 
+  void BuildTest(ToBooleanMode mode, BytecodeLabels* then_labels,
+                 BytecodeLabels* else_labels, TestFallthrough fallthrough);
+
   // Visitors for obtaining expression result in the accumulator, in a
   // register, or just getting the effect. Some visitors return a TypeHint which
   // specifies the type of the result of the visited expression.
@@ -194,6 +202,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
                     BytecodeLabels* else_labels, TestFallthrough fallthrough);
   TypeHint VisitForAddOperand(Expression* expr, RegisterList* operand_registers,
                               Register* out_register);
+  void VisitInSameTestExecutionScope(Expression* expr);
 
   // Returns the runtime function id for a store to super for the function's
   // language mode.

src/interpreter/bytecode-register-allocator.h

@@ -91,11 +91,11 @@ class BytecodeRegisterAllocator final {
 
   // Release all registers above |register_index|.
   void ReleaseRegisters(int register_index) {
+    int count = next_register_index_ - register_index;
+    next_register_index_ = register_index;
     if (observer_) {
-      observer_->RegisterListFreeEvent(
-          RegisterList(register_index, next_register_index_ - register_index));
+      observer_->RegisterListFreeEvent(RegisterList(register_index, count));
     }
-    next_register_index_ = register_index;
   }
 
   // Returns true if the register |reg| is a live register.