[compiler] Collect eager inner functions in bytecode generation

Instead of collecting eagerly compilable inner function literals (IIFEs
etc.) during AST numbering, collect them during bytecode generation,
exposing them on the CompilationJob.

Bug: v8:7178
Change-Id: I47451f412d2796e5857b4bc38c4f29c80cb0745d
Reviewed-on: https://chromium-review.googlesource.com/873872
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50842}

src/ast/ast-numbering.cc

@@ -14,9 +14,7 @@ namespace internal {
 
 class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
  public:
-  AstNumberingVisitor(uintptr_t stack_limit, Zone* zone,
-                      Compiler::EagerInnerFunctionLiterals* eager_literals)
-      : zone_(zone), eager_literals_(eager_literals) {
+  AstNumberingVisitor(uintptr_t stack_limit, Zone* zone) : zone_(zone) {
     InitializeAstVisitor(stack_limit);
   }
 
@@ -39,7 +37,6 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
   Zone* zone() const { return zone_; }
 
   Zone* zone_;
-  Compiler::EagerInnerFunctionLiterals* eager_literals_;
   FunctionKind function_kind_;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
@@ -351,14 +348,9 @@ void AstNumberingVisitor::VisitArguments(ZoneList<Expression*>* arguments) {
 
 void AstNumberingVisitor::VisitFunctionLiteral(FunctionLiteral* node) {
   if (node->ShouldEagerCompile()) {
-    if (eager_literals_) {
-      eager_literals_->Add(new (zone())
-                               ThreadedListZoneEntry<FunctionLiteral*>(node));
-    }
-
     // If the function literal is being eagerly compiled, recurse into the
     // declarations and body of the function literal.
-    if (!AstNumbering::Renumber(stack_limit_, zone_, node, eager_literals_)) {
+    if (!AstNumbering::Renumber(stack_limit_, zone_, node)) {
       SetStackOverflow();
       return;
     }
@@ -381,14 +373,13 @@ bool AstNumberingVisitor::Renumber(FunctionLiteral* node) {
   return !HasStackOverflow();
 }
 
-bool AstNumbering::Renumber(
-    uintptr_t stack_limit, Zone* zone, FunctionLiteral* function,
-    Compiler::EagerInnerFunctionLiterals* eager_literals) {
+bool AstNumbering::Renumber(uintptr_t stack_limit, Zone* zone,
+                            FunctionLiteral* function) {
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
   DisallowHandleDereference no_deref;
 
-  AstNumberingVisitor visitor(stack_limit, zone, eager_literals);
+  AstNumberingVisitor visitor(stack_limit, zone);
   return visitor.Renumber(function);
 }
 }  // namespace internal

src/ast/ast-numbering.h

@@ -14,21 +14,12 @@ namespace internal {
 class FunctionLiteral;
 class Isolate;
 class Zone;
-template <typename T>
-class ThreadedList;
-template <typename T>
-class ThreadedListZoneEntry;
-template <typename T>
-class ZoneVector;
 
 namespace AstNumbering {
 // Assign bailout IDs, and generator suspend IDs to an AST node tree; perform
-// catch prediction for TryStatements. If |eager_literals| is non-null, adds any
-// eager inner literal functions into it.
-bool Renumber(
-    uintptr_t stack_limit, Zone* zone, FunctionLiteral* function,
-    ThreadedList<ThreadedListZoneEntry<FunctionLiteral*>>* eager_literals);
-}
+// catch prediction for TryStatements.
+bool Renumber(uintptr_t stack_limit, Zone* zone, FunctionLiteral* function);
+}  // namespace AstNumbering
 
 // Some details on suspend IDs
 // -------------------------

src/compiler-dispatcher/unoptimized-compile-job.cc

@@ -251,7 +251,7 @@ void UnoptimizedCompileJob::Compile(bool on_background_thread) {
   }
 
   compilation_job_.reset(interpreter::Interpreter::NewCompilationJob(
-      parse_info_.get(), parse_info_->literal(), allocator_));
+      parse_info_.get(), parse_info_->literal(), allocator_, nullptr));
 
   if (!compilation_job_.get()) {
     parse_info_->pending_error_handler()->set_stack_overflow();

src/compiler.cc

@@ -370,20 +370,19 @@ CompilationJob::Status FinalizeUnoptimizedCompilationJob(CompilationJob* job,
   return status;
 }
 
-bool Renumber(ParseInfo* parse_info,
-              Compiler::EagerInnerFunctionLiterals* eager_literals) {
+bool Renumber(ParseInfo* parse_info) {
   RuntimeCallTimerScope runtimeTimer(
       parse_info->runtime_call_stats(),
       parse_info->on_background_thread()
           ? RuntimeCallCounterId::kCompileBackgroundRenumber
           : RuntimeCallCounterId::kCompileRenumber);
   return AstNumbering::Renumber(parse_info->stack_limit(), parse_info->zone(),
-                                parse_info->literal(), eager_literals);
+                                parse_info->literal());
 }
 
-std::unique_ptr<CompilationJob> PrepareAndExecuteUnoptimizedCompileJob(
+std::unique_ptr<CompilationJob> PrepareAndExecuteUnoptimizedCompileJobs(
     ParseInfo* parse_info, FunctionLiteral* literal,
-    AccountingAllocator* allocator) {
+    AccountingAllocator* allocator, CompilationJobList* inner_function_jobs) {
   if (UseAsmWasm(literal, parse_info->is_asm_wasm_broken())) {
     std::unique_ptr<CompilationJob> asm_job(
         AsmJs::NewCompilationJob(parse_info, literal, allocator));
@@ -396,14 +395,27 @@ std::unique_ptr<CompilationJob> PrepareAndExecuteUnoptimizedCompileJob(
     // with a validation error or another error that could be solve by falling
     // through to standard unoptimized compile.
   }
+  ZoneVector<FunctionLiteral*> eager_inner_literals(0, parse_info->zone());
   std::unique_ptr<CompilationJob> job(
-      interpreter::Interpreter::NewCompilationJob(parse_info, literal,
-                                                  allocator));
+      interpreter::Interpreter::NewCompilationJob(
+          parse_info, literal, allocator, &eager_inner_literals));
 
-  if (job->ExecuteJob() == CompilationJob::SUCCEEDED) {
-    return job;
+  if (job->ExecuteJob() != CompilationJob::SUCCEEDED) {
+    // Compilation failed, return null.
+    return std::unique_ptr<CompilationJob>();
+  }
+
+  // Recursively compile eager inner literals.
+  for (FunctionLiteral* inner_literal : eager_inner_literals) {
+    std::unique_ptr<CompilationJob> inner_job(
+        PrepareAndExecuteUnoptimizedCompileJobs(
+            parse_info, inner_literal, allocator, inner_function_jobs));
+    // Compilation failed, return null.
+    if (!inner_job) return std::unique_ptr<CompilationJob>();
+    inner_function_jobs->emplace_front(std::move(inner_job));
   }
-  return std::unique_ptr<CompilationJob>();  // Compilation failed, return null.
+
+  return job;
 }
 
 std::unique_ptr<CompilationJob> GenerateUnoptimizedCode(
@@ -414,27 +426,16 @@ std::unique_ptr<CompilationJob> GenerateUnoptimizedCode(
   DisallowHandleDereference no_deref;
   DCHECK(inner_function_jobs->empty());
 
-  Compiler::EagerInnerFunctionLiterals inner_literals;
-  if (!Compiler::Analyze(parse_info, &inner_literals)) {
+  if (!Compiler::Analyze(parse_info)) {
     return std::unique_ptr<CompilationJob>();
   }
 
   // Prepare and execute compilation of the outer-most function.
   std::unique_ptr<CompilationJob> outer_function_job(
-      PrepareAndExecuteUnoptimizedCompileJob(parse_info, parse_info->literal(),
-                                             allocator));
+      PrepareAndExecuteUnoptimizedCompileJobs(parse_info, parse_info->literal(),
+                                              allocator, inner_function_jobs));
   if (!outer_function_job) return std::unique_ptr<CompilationJob>();
 
-  // Prepare and execute compilation jobs for eager inner functions.
-  for (auto it : inner_literals) {
-    FunctionLiteral* inner_literal = it->value();
-    std::unique_ptr<CompilationJob> inner_job(
-        PrepareAndExecuteUnoptimizedCompileJob(parse_info, inner_literal,
-                                               allocator));
-    if (!inner_job) return std::unique_ptr<CompilationJob>();
-    inner_function_jobs->emplace_front(std::move(inner_job));
-  }
-
   // Character stream shouldn't be used again.
   parse_info->ResetCharacterStream();
 
@@ -857,8 +858,7 @@ bool FailWithPendingException(Isolate* isolate,
 // ----------------------------------------------------------------------------
 // Implementation of Compiler
 
-bool Compiler::Analyze(ParseInfo* parse_info,
-                       EagerInnerFunctionLiterals* eager_literals) {
+bool Compiler::Analyze(ParseInfo* parse_info) {
   DCHECK_NOT_NULL(parse_info->literal());
   RuntimeCallTimerScope runtimeTimer(
       parse_info->runtime_call_stats(),
@@ -867,7 +867,7 @@ bool Compiler::Analyze(ParseInfo* parse_info,
           : RuntimeCallCounterId::kCompileAnalyse);
   if (!Rewriter::Rewrite(parse_info)) return false;
   DeclarationScope::Analyze(parse_info);
-  if (!Renumber(parse_info, eager_literals)) return false;
+  if (!Renumber(parse_info)) return false;
   return true;
 }
 

src/compiler.h

@@ -24,10 +24,6 @@ class CompilationJob;
 class JavaScriptFrame;
 class ParseInfo;
 class ScriptData;
-template <typename T>
-class ThreadedList;
-template <typename T>
-class ThreadedListZoneEntry;
 
 typedef std::forward_list<std::unique_ptr<CompilationJob>> CompilationJobList;
 
@@ -71,17 +67,12 @@ class V8_EXPORT_PRIVATE Compiler : public AllStatic {
   // offer this chance, optimized closure instantiation will not call this.
   static void PostInstantiation(Handle<JSFunction> function, PretenureFlag);
 
-  typedef ThreadedList<ThreadedListZoneEntry<FunctionLiteral*>>
-      EagerInnerFunctionLiterals;
-
   // Parser::Parse, then Compiler::Analyze.
   static bool ParseAndAnalyze(ParseInfo* parse_info,
                               Handle<SharedFunctionInfo> shared_info,
                               Isolate* isolate);
-  // Rewrite, analyze scopes, and renumber. If |eager_literals| is non-null, it
-  // is appended with inner function literals which should be eagerly compiled.
-  static bool Analyze(ParseInfo* parse_info,
-                      EagerInnerFunctionLiterals* eager_literals = nullptr);
+  // Rewrite, analyze scopes, and renumber.
+  static bool Analyze(ParseInfo* parse_info);
 
   // ===========================================================================
   // The following family of methods instantiates new functions for scripts or

src/interpreter/bytecode-generator.cc

@@ -853,8 +853,22 @@ class BytecodeGenerator::IteratorRecord final {
   Register next_;
 };
 
+#ifdef DEBUG
+
+static bool IsInEagerLiterals(
+    FunctionLiteral* literal,
+    const ZoneVector<FunctionLiteral*>& eager_literals) {
+  for (FunctionLiteral* eager_literal : eager_literals) {
+    if (literal == eager_literal) return true;
+  }
+  return false;
+}
+
+#endif  // DEBUG
+
 BytecodeGenerator::BytecodeGenerator(
-    CompilationInfo* info, const AstStringConstants* ast_string_constants)
+    CompilationInfo* info, const AstStringConstants* ast_string_constants,
+    ZoneVector<FunctionLiteral*>* eager_inner_literals)
     : zone_(info->zone()),
       builder_(zone(), info->num_parameters_including_this(),
                info->scope()->num_stack_slots(), info->feedback_vector_spec(),
@@ -863,6 +877,7 @@ BytecodeGenerator::BytecodeGenerator(
       ast_string_constants_(ast_string_constants),
       closure_scope_(info->scope()),
       current_scope_(info->scope()),
+      eager_inner_literals_(eager_inner_literals),
       feedback_slot_cache_(new (zone()) FeedbackSlotCache(zone())),
       globals_builder_(new (zone()) GlobalDeclarationsBuilder(zone())),
       block_coverage_builder_(nullptr),
@@ -1225,6 +1240,7 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) {
       FeedbackSlot literal_slot = GetCachedCreateClosureSlot(decl->fun());
       globals_builder()->AddFunctionDeclaration(variable->raw_name(), slot,
                                                 literal_slot, decl->fun());
+      AddToEagerLiteralsIfEager(decl->fun());
       break;
     }
     case VariableLocation::PARAMETER:
@@ -1257,6 +1273,8 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) {
       BuildVariableAssignment(variable, Token::INIT, HoleCheckMode::kElided);
       break;
   }
+  DCHECK_IMPLIES(decl->fun()->ShouldEagerCompile(),
+                 IsInEagerLiterals(decl->fun(), *eager_inner_literals_));
 }
 
 void BytecodeGenerator::VisitModuleNamespaceImports() {
@@ -1783,6 +1801,14 @@ void BytecodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) {
   FeedbackSlot slot = GetCachedCreateClosureSlot(expr);
   builder()->CreateClosure(entry, feedback_index(slot), flags);
   function_literals_.push_back(std::make_pair(expr, entry));
+  AddToEagerLiteralsIfEager(expr);
+}
+
+void BytecodeGenerator::AddToEagerLiteralsIfEager(FunctionLiteral* literal) {
+  if (eager_inner_literals_ && literal->ShouldEagerCompile()) {
+    DCHECK(!IsInEagerLiterals(literal, *eager_inner_literals_));
+    eager_inner_literals_->push_back(literal);
+  }
 }
 
 void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {

src/interpreter/bytecode-generator.h

@@ -28,8 +28,9 @@ class BytecodeJumpTable;
 
 class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
  public:
-  explicit BytecodeGenerator(CompilationInfo* info,
-                             const AstStringConstants* ast_string_constants);
+  explicit BytecodeGenerator(
+      CompilationInfo* info, const AstStringConstants* ast_string_constants,
+      ZoneVector<FunctionLiteral*>* eager_inner_literals);
 
   void GenerateBytecode(uintptr_t stack_limit);
   Handle<BytecodeArray> FinalizeBytecode(Isolate* isolate,
@@ -260,6 +261,8 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
                                          Variable* variable);
   FeedbackSlot GetCachedCreateClosureSlot(FunctionLiteral* literal);
 
+  void AddToEagerLiteralsIfEager(FunctionLiteral* literal);
+
   static constexpr ToBooleanMode ToBooleanModeFromTypeHint(TypeHint type_hint) {
     return type_hint == TypeHint::kBoolean ? ToBooleanMode::kAlreadyBoolean
                                            : ToBooleanMode::kConvertToBoolean;
@@ -321,6 +324,9 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   DeclarationScope* closure_scope_;
   Scope* current_scope_;
 
+  // External vector of literals to be eagerly compiled.
+  ZoneVector<FunctionLiteral*>* eager_inner_literals_;
+
   FeedbackSlotCache* feedback_slot_cache_;
 
   GlobalDeclarationsBuilder* globals_builder_;

src/interpreter/interpreter.cc

@@ -29,7 +29,8 @@ namespace interpreter {
 class InterpreterCompilationJob final : public CompilationJob {
  public:
   InterpreterCompilationJob(ParseInfo* parse_info, FunctionLiteral* literal,
-                            AccountingAllocator* allocator);
+                            AccountingAllocator* allocator,
+                            ZoneVector<FunctionLiteral*>* eager_inner_literals);
 
  protected:
   Status PrepareJobImpl(Isolate* isolate) final;
@@ -163,12 +164,14 @@ bool ShouldPrintBytecode(Handle<SharedFunctionInfo> shared) {
 
 InterpreterCompilationJob::InterpreterCompilationJob(
     ParseInfo* parse_info, FunctionLiteral* literal,
-    AccountingAllocator* allocator)
+    AccountingAllocator* allocator,
+    ZoneVector<FunctionLiteral*>* eager_inner_literals)
     : CompilationJob(parse_info->stack_limit(), parse_info, &compilation_info_,
                      "Ignition", State::kReadyToExecute),
       zone_(allocator, ZONE_NAME),
       compilation_info_(&zone_, parse_info, literal),
-      generator_(&compilation_info_, parse_info->ast_string_constants()) {}
+      generator_(&compilation_info_, parse_info->ast_string_constants(),
+                 eager_inner_literals) {}
 
 InterpreterCompilationJob::Status InterpreterCompilationJob::PrepareJobImpl(
     Isolate* isolate) {
@@ -226,10 +229,12 @@ InterpreterCompilationJob::Status InterpreterCompilationJob::FinalizeJobImpl(
   return SUCCEEDED;
 }
 
-CompilationJob* Interpreter::NewCompilationJob(ParseInfo* parse_info,
-                                               FunctionLiteral* literal,
-                                               AccountingAllocator* allocator) {
-  return new InterpreterCompilationJob(parse_info, literal, allocator);
+CompilationJob* Interpreter::NewCompilationJob(
+    ParseInfo* parse_info, FunctionLiteral* literal,
+    AccountingAllocator* allocator,
+    ZoneVector<FunctionLiteral*>* eager_inner_literals) {
+  return new InterpreterCompilationJob(parse_info, literal, allocator,
+                                       eager_inner_literals);
 }
 
 bool Interpreter::IsDispatchTableInitialized() const {

src/interpreter/interpreter.h

@@ -27,6 +27,8 @@ class FunctionLiteral;
 class ParseInfo;
 class RootVisitor;
 class SetupIsolateDelegate;
+template <typename>
+class ZoneVector;
 
 namespace interpreter {
 
@@ -38,9 +40,12 @@ class Interpreter {
   virtual ~Interpreter() {}
 
   // Creates a compilation job which will generate bytecode for |literal|.
-  static CompilationJob* NewCompilationJob(ParseInfo* parse_info,
-                                           FunctionLiteral* literal,
-                                           AccountingAllocator* allocator);
+  // Additionally, if |eager_inner_literals| is not null, adds any eagerly
+  // compilable inner FunctionLiterals to this list.
+  static CompilationJob* NewCompilationJob(
+      ParseInfo* parse_info, FunctionLiteral* literal,
+      AccountingAllocator* allocator,
+      ZoneVector<FunctionLiteral*>* eager_inner_literals);
 
   // If the bytecode handler for |bytecode| and |operand_scale| has not yet
   // been loaded, deserialize it. Then return the handler.