[Compiler] Refactor CompileUnoptimizedCode to do all Finalization at the end.

Refactors CompileUnoptimizedCode to do all the finalization for both
inner and the outermost function after having prepared and executed their
compile jobs. This will enable the function to be split into an off-thread
phase and a finalization main thread phase.

BUG=v8:5203

Change-Id: I400933c27b7aa52f9a7318b721adecfc94c80981
Reviewed-on: https://chromium-review.googlesource.com/602236
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47199}

src/asmjs/asm-js.cc

@@ -179,17 +179,16 @@ void ReportInstantiationFailure(Handle<Script> script, int position,
 //      translated to a valid WebAssembly module. The result are two vectors
 //      representing the encoded module as well as encoded source position
 //      information and a StdlibSet bit set.
-//  [2] FinalizeJobImp: The module is handed to WebAssembly which decodes it
+//  [2] FinalizeJobImpl: The module is handed to WebAssembly which decodes it
 //      into an internal representation and eventually compiles it to machine
 //      code.
 class AsmJsCompilationJob final : public CompilationJob {
  public:
   explicit AsmJsCompilationJob(ParseInfo* parse_info, FunctionLiteral* literal,
-                               Handle<SharedFunctionInfo> shared_info,
                                Isolate* isolate)
       : CompilationJob(isolate, parse_info, &compilation_info_, "AsmJs"),
         zone_(isolate->allocator(), ZONE_NAME),
-        compilation_info_(&zone_, isolate, parse_info, literal, shared_info),
+        compilation_info_(&zone_, isolate, parse_info, literal),
         module_(nullptr),
         asm_offsets_(nullptr),
         translate_time_(0),
@@ -306,9 +305,8 @@ CompilationJob::Status AsmJsCompilationJob::FinalizeJobImpl() {
 
 CompilationJob* AsmJs::NewCompilationJob(ParseInfo* parse_info,
                                          FunctionLiteral* literal,
-                                         Handle<SharedFunctionInfo> shared_info,
                                          Isolate* isolate) {
-  return new AsmJsCompilationJob(parse_info, literal, shared_info, isolate);
+  return new AsmJsCompilationJob(parse_info, literal, isolate);
 }
 
 MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,

src/asmjs/asm-js.h

@@ -22,9 +22,9 @@ class SharedFunctionInfo;
 // Interface to compile and instantiate for asm.js modules.
 class AsmJs {
  public:
-  static CompilationJob* NewCompilationJob(
-      ParseInfo* parse_info, FunctionLiteral* literal,
-      Handle<SharedFunctionInfo> shared_info, Isolate* isolate);
+  static CompilationJob* NewCompilationJob(ParseInfo* parse_info,
+                                           FunctionLiteral* literal,
+                                           Isolate* isolate);
   static MaybeHandle<Object> InstantiateAsmWasm(Isolate* isolate,
                                                 Handle<SharedFunctionInfo>,
                                                 Handle<FixedArray> wasm_data,

src/compilation-info.cc

@@ -18,8 +18,7 @@ namespace internal {
 
 CompilationInfo::CompilationInfo(Zone* zone, Isolate* isolate,
                                  ParseInfo* parse_info,
-                                 FunctionLiteral* literal,
-                                 Handle<SharedFunctionInfo> shared)
+                                 FunctionLiteral* literal)
     : CompilationInfo(parse_info->script(), {},
                       Code::ComputeFlags(Code::FUNCTION), BASE, isolate, zone) {
   // NOTE: The parse_info passed here represents the global information gathered
@@ -29,7 +28,6 @@ CompilationInfo::CompilationInfo(Zone* zone, Isolate* isolate,
   // details of the script being parsed are relevant to this CompilationInfo.
   DCHECK_NOT_NULL(literal);
   literal_ = literal;
-  shared_info_ = shared;
   source_range_map_ = parse_info->source_range_map();
 
   // Collect source positions for optimized code when profiling or if debugger

src/compilation-info.h

@@ -57,7 +57,7 @@ class V8_EXPORT_PRIVATE CompilationInfo final {
 
   // Construct a compilation info for unoptimized compilation.
   CompilationInfo(Zone* zone, Isolate* isolate, ParseInfo* parse_info,
-                  FunctionLiteral* literal, Handle<SharedFunctionInfo> shared);
+                  FunctionLiteral* literal);
   // Construct a compilation info for optimized compilation.
   CompilationInfo(Zone* zone, Isolate* isolate, Handle<Script> script,
                   Handle<SharedFunctionInfo> shared,

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

@@ -413,8 +413,8 @@ void UnoptimizedCompileJob::PrepareToCompileOnMainThread(Isolate* isolate) {
   DCHECK(status() == Status::kAnalyzed);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kPrepareToCompile);
 
-  compilation_job_.reset(Compiler::PrepareUnoptimizedCompilationJob(
-      parse_info_.get(), shared_, isolate));
+  compilation_job_.reset(
+      Compiler::PrepareUnoptimizedCompilationJob(parse_info_.get(), isolate));
   if (!compilation_job_.get()) {
     if (!isolate->has_pending_exception()) isolate->StackOverflow();
     status_ = Status::kFailed;
@@ -458,6 +458,12 @@ void UnoptimizedCompileJob::FinalizeCompilingOnMainThread(Isolate* isolate) {
 
   {
     HandleScope scope(isolate);
+    // Internalize ast values onto the heap.
+    parse_info_->ast_value_factory()->Internalize(isolate);
+    // Allocate scope infos for the literal.
+    DeclarationScope::AllocateScopeInfos(parse_info_.get(), isolate,
+                                         AnalyzeMode::kRegular);
+    compilation_job_->compilation_info()->set_shared_info(shared_);
     if (compilation_job_->state() == CompilationJob::State::kFailed ||
         !Compiler::FinalizeCompilationJob(compilation_job_.release())) {
       if (!isolate->has_pending_exception()) isolate->StackOverflow();

src/compiler.cc

@@ -291,20 +291,19 @@ bool UseAsmWasm(FunctionLiteral* literal,
   return literal->scope()->asm_module();
 }
 
-CompilationJob* GetUnoptimizedCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
-    Handle<SharedFunctionInfo> shared_info, Isolate* isolate) {
+CompilationJob* GetUnoptimizedCompilationJob(ParseInfo* parse_info,
+                                             FunctionLiteral* literal,
+                                             Isolate* isolate) {
   // Function should have been parsed and analyzed before creating a compilation
   // job.
   DCHECK_NOT_NULL(literal);
   DCHECK_NOT_NULL(parse_info->scope());
 
   if (ShouldUseFullCodegen(literal)) {
-    return FullCodeGenerator::NewCompilationJob(parse_info, literal,
-                                                shared_info, isolate);
+    return FullCodeGenerator::NewCompilationJob(parse_info, literal, isolate);
   } else {
     return interpreter::Interpreter::NewCompilationJob(parse_info, literal,
-                                                       shared_info, isolate);
+                                                       isolate);
   }
 }
 
@@ -387,29 +386,7 @@ void SetSharedFunctionFlagsFromLiteral(FunctionLiteral* literal,
 CompilationJob::Status FinalizeUnoptimizedCompilationJob(CompilationJob* job) {
   CompilationInfo* compilation_info = job->compilation_info();
   ParseInfo* parse_info = job->parse_info();
-  Isolate* isolate = compilation_info->isolate();
 
-  // Internalize ast values onto the heap.
-  parse_info->ast_value_factory()->Internalize(isolate);
-
-  // Allocate scope infos for the literal.
-  DeclarationScope::AllocateScopeInfos(parse_info, isolate,
-                                       AnalyzeMode::kRegular);
-
-  if (parse_info->is_toplevel()) {
-    // Allocate a shared function info and an array for shared function infos
-    // for inner functions.
-    EnsureSharedFunctionInfosArrayOnScript(parse_info, isolate);
-    DCHECK_EQ(kNoSourcePosition,
-              compilation_info->literal()->function_token_position());
-    if (!compilation_info->has_shared_info()) {
-      Handle<SharedFunctionInfo> shared =
-          isolate->factory()->NewSharedFunctionInfoForLiteral(
-              compilation_info->literal(), compilation_info->script());
-      shared->set_is_toplevel(true);
-      compilation_info->set_shared_info(shared);
-    }
-  }
   SetSharedFunctionFlagsFromLiteral(compilation_info->literal(),
                                     compilation_info->shared_info());
 
@@ -439,52 +416,29 @@ bool Renumber(ParseInfo* parse_info,
                                 parse_info->collect_type_profile());
 }
 
-bool RunUnoptimizedCompilationJob(CompilationJob* job) {
-  if (job->PrepareJob() != CompilationJob::SUCCEEDED) return false;
-  if (job->ExecuteJob() != CompilationJob::SUCCEEDED) return false;
-  return FinalizeUnoptimizedCompilationJob(job) == CompilationJob::SUCCEEDED;
-}
-
-Handle<SharedFunctionInfo> GenerateUnoptimizedCode(
+std::unique_ptr<CompilationJob> PrepareAndExecuteUnoptimizedCompileJob(
     ParseInfo* parse_info, FunctionLiteral* literal,
     Handle<SharedFunctionInfo> shared_info, Isolate* isolate) {
   if (UseAsmWasm(literal, shared_info, parse_info->is_debug())) {
-    std::unique_ptr<CompilationJob> job(
-        AsmJs::NewCompilationJob(parse_info, literal, shared_info, isolate));
-    if (RunUnoptimizedCompilationJob(job.get())) {
-      return job->compilation_info()->shared_info();
+    std::unique_ptr<CompilationJob> asm_job(
+        AsmJs::NewCompilationJob(parse_info, literal, isolate));
+    if (asm_job->PrepareJob() == CompilationJob::SUCCEEDED &&
+        asm_job->ExecuteJob() == CompilationJob::SUCCEEDED) {
+      return asm_job;
     }
     // asm.js validation failed, fall through to standard unoptimized compile.
+    // Note: we rely on the fact that AsmJs jobs have done all validation in the
+    // PrepareJob and ExecuteJob phases and can't fail in FinalizeJob with
+    // with a validation error or another error that could be solve by falling
+    // through to standard unoptimized compile.
   }
   std::unique_ptr<CompilationJob> job(
-      GetUnoptimizedCompilationJob(parse_info, literal, shared_info, isolate));
-  if (RunUnoptimizedCompilationJob(job.get())) {
-    return job->compilation_info()->shared_info();
+      GetUnoptimizedCompilationJob(parse_info, literal, isolate));
+  if (job->PrepareJob() == CompilationJob::SUCCEEDED &&
+      job->ExecuteJob() == CompilationJob::SUCCEEDED) {
+    return job;
   }
-  return Handle<SharedFunctionInfo>::null();  // Compilation failed.
-}
-
-bool CompileUnoptimizedInnerFunctions(
-    Compiler::EagerInnerFunctionLiterals* literals, Handle<Script> script,
-    ParseInfo* outer_parse_info, Isolate* isolate) {
-  TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
-               "V8.CompileUnoptimizedInnerFunctions");
-  RuntimeCallTimerScope runtimeTimer(isolate,
-                                     &RuntimeCallStats::CompileInnerFunction);
-  for (auto it : *literals) {
-    FunctionLiteral* literal = it->value();
-    Handle<SharedFunctionInfo> shared =
-        Compiler::GetSharedFunctionInfo(literal, script, isolate);
-    if (shared->is_compiled()) continue;
-
-    // Generate unoptimized code now.
-    if (GenerateUnoptimizedCode(outer_parse_info, literal, shared, isolate)
-            .is_null()) {
-      if (!isolate->has_pending_exception()) isolate->StackOverflow();
-      return false;
-    }
-  }
-  return true;
+  return std::unique_ptr<CompilationJob>();  // Compilation failed, return null.
 }
 
 bool InnerFunctionShouldUseFullCodegen(
@@ -524,19 +478,77 @@ Handle<SharedFunctionInfo> CompileUnoptimizedCode(
     }
   }
 
-  Handle<SharedFunctionInfo> result = GenerateUnoptimizedCode(
-      parse_info, parse_info->literal(), shared_info, isolate);
-  // Set toplevel to false for inner functions.
-  // TODO(rmcilroy): Remove this once we no longer rely on
-  // parse_info->toplevel() in FinalizeUnoptimizedCompilationJob.
-  parse_info->set_toplevel(false);
-  if (result.is_null() ||
-      !CompileUnoptimizedInnerFunctions(&inner_literals, parse_info->script(),
-                                        parse_info, isolate)) {
+  // Prepare and execute compilation of the outer-most function.
+  std::unique_ptr<CompilationJob> outer_job(
+      PrepareAndExecuteUnoptimizedCompileJob(parse_info, parse_info->literal(),
+                                             shared_info, isolate));
+  if (!outer_job) {
     if (!isolate->has_pending_exception()) isolate->StackOverflow();
     return Handle<SharedFunctionInfo>::null();
   }
-  return result;
+
+  // Prepare and execute compilation jobs for eager inner functions.
+  std::forward_list<std::unique_ptr<CompilationJob>> inner_jobs;
+  for (auto it : inner_literals) {
+    FunctionLiteral* inner_literal = it->value();
+    std::unique_ptr<CompilationJob> inner_job(
+        PrepareAndExecuteUnoptimizedCompileJob(
+            parse_info, inner_literal, Handle<SharedFunctionInfo>::null(),
+            isolate));
+    if (!inner_job) {
+      if (!isolate->has_pending_exception()) isolate->StackOverflow();
+      return Handle<SharedFunctionInfo>::null();
+    }
+    inner_jobs.emplace_front(std::move(inner_job));
+  }
+
+  // Internalize ast values onto the heap.
+  parse_info->ast_value_factory()->Internalize(isolate);
+
+  // Allocate scope infos for the literal.
+  DeclarationScope::AllocateScopeInfos(parse_info, isolate,
+                                       AnalyzeMode::kRegular);
+
+  // If compiling top-level code, allocate a shared function info and an array
+  // for shared function infos for inner functions.
+  if (parse_info->is_toplevel()) {
+    EnsureSharedFunctionInfosArrayOnScript(parse_info, isolate);
+    DCHECK_EQ(kNoSourcePosition,
+              parse_info->literal()->function_token_position());
+    if (shared_info.is_null()) {
+      shared_info = isolate->factory()->NewSharedFunctionInfoForLiteral(
+          parse_info->literal(), parse_info->script());
+      shared_info->set_is_toplevel(true);
+    }
+  }
+
+  // Finalize the outer-most function's compilation job.
+  outer_job->compilation_info()->set_shared_info(shared_info);
+  if (FinalizeUnoptimizedCompilationJob(outer_job.get()) !=
+      CompilationJob::SUCCEEDED) {
+    if (!isolate->has_pending_exception()) isolate->StackOverflow();
+    return Handle<SharedFunctionInfo>::null();
+  }
+
+  // Finalize the inner functions' compilation jobs.
+  for (auto&& inner_job : inner_jobs) {
+    Handle<SharedFunctionInfo> inner_shared_info =
+        Compiler::GetSharedFunctionInfo(
+            inner_job->compilation_info()->literal(), parse_info->script(),
+            isolate);
+    // The inner function might be compiled already if compiling for debug.
+    // TODO(rmcilroy): Fix this and DCHECK !is_compiled() once Full-Codegen dies
+    if (inner_shared_info->is_compiled()) continue;
+    inner_job->compilation_info()->set_shared_info(inner_shared_info);
+    if (FinalizeUnoptimizedCompilationJob(inner_job.get()) !=
+        CompilationJob::SUCCEEDED) {
+      if (!isolate->has_pending_exception()) isolate->StackOverflow();
+      return Handle<SharedFunctionInfo>::null();
+    }
+  }
+
+  // Compilation succeeded, return result.
+  return shared_info;
 }
 
 MUST_USE_RESULT MaybeHandle<Code> CompileUnoptimizedFunction(
@@ -1598,11 +1610,10 @@ MaybeHandle<Code> Compiler::GetOptimizedCodeForOSR(Handle<JSFunction> function,
 }
 
 CompilationJob* Compiler::PrepareUnoptimizedCompilationJob(
-    ParseInfo* parse_info, Handle<SharedFunctionInfo> shared_info,
-    Isolate* isolate) {
+    ParseInfo* parse_info, Isolate* isolate) {
   VMState<COMPILER> state(isolate);
-  std::unique_ptr<CompilationJob> job(GetUnoptimizedCompilationJob(
-      parse_info, parse_info->literal(), shared_info, isolate));
+  std::unique_ptr<CompilationJob> job(
+      GetUnoptimizedCompilationJob(parse_info, parse_info->literal(), isolate));
   if (job->PrepareJob() != CompilationJob::SUCCEEDED) {
     return nullptr;
   }

src/compiler.h

@@ -53,9 +53,8 @@ class V8_EXPORT_PRIVATE Compiler : public AllStatic {
   static MaybeHandle<JSArray> CompileForLiveEdit(Handle<Script> script);
 
   // Prepare a compilation job for unoptimized code. Requires ParseAndAnalyse.
-  static CompilationJob* PrepareUnoptimizedCompilationJob(
-      ParseInfo* parse_info, Handle<SharedFunctionInfo> shared_info,
-      Isolate* isolate);
+  static CompilationJob* PrepareUnoptimizedCompilationJob(ParseInfo* parse_info,
+                                                          Isolate* isolate);
 
   // Generate and install code from previously queued compilation job.
   static bool FinalizeCompilationJob(CompilationJob* job);

src/full-codegen/full-codegen.cc

@@ -30,12 +30,10 @@ namespace internal {
 class FullCodegenCompilationJob final : public CompilationJob {
  public:
   explicit FullCodegenCompilationJob(ParseInfo* parse_info,
-                                     FunctionLiteral* literal,
-                                     Handle<SharedFunctionInfo> shared_info,
-                                     Isolate* isolate)
+                                     FunctionLiteral* literal, Isolate* isolate)
       : CompilationJob(isolate, parse_info, &compilation_info_, "Full-Codegen"),
         zone_(isolate->allocator(), ZONE_NAME),
-        compilation_info_(&zone_, isolate, parse_info, literal, shared_info) {}
+        compilation_info_(&zone_, isolate, parse_info, literal) {}
 
   bool can_execute_on_background_thread() const override { return false; }
 
@@ -80,11 +78,10 @@ FullCodeGenerator::FullCodeGenerator(MacroAssembler* masm,
 }
 
 // static
-CompilationJob* FullCodeGenerator::NewCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
-    Handle<SharedFunctionInfo> shared_info, Isolate* isolate) {
-  return new FullCodegenCompilationJob(parse_info, literal, shared_info,
-                                       isolate);
+CompilationJob* FullCodeGenerator::NewCompilationJob(ParseInfo* parse_info,
+                                                     FunctionLiteral* literal,
+                                                     Isolate* isolate) {
+  return new FullCodegenCompilationJob(parse_info, literal, isolate);
 }
 
 // static

src/full-codegen/full-codegen.h

@@ -39,9 +39,9 @@ class FullCodeGenerator final : public AstVisitor<FullCodeGenerator> {
 
   void Initialize(uintptr_t stack_limit);
 
-  static CompilationJob* NewCompilationJob(
-      ParseInfo* parse_info, FunctionLiteral* literal,
-      Handle<SharedFunctionInfo> shared_info, Isolate* isolate);
+  static CompilationJob* NewCompilationJob(ParseInfo* parse_info,
+                                           FunctionLiteral* literal,
+                                           Isolate* isolate);
 
   static bool MakeCode(ParseInfo* parse_info, CompilationInfo* info,
                        uintptr_t stack_limit);

src/interpreter/interpreter.cc

@@ -25,7 +25,6 @@ namespace interpreter {
 class InterpreterCompilationJob final : public CompilationJob {
  public:
   InterpreterCompilationJob(ParseInfo* parse_info, FunctionLiteral* literal,
-                            Handle<SharedFunctionInfo> shared_info,
                             Isolate* isolate);
 
  protected:
@@ -147,12 +146,12 @@ bool ShouldPrintBytecode(Handle<SharedFunctionInfo> shared) {
 
 }  // namespace
 
-InterpreterCompilationJob::InterpreterCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
-    Handle<SharedFunctionInfo> shared_info, Isolate* isolate)
+InterpreterCompilationJob::InterpreterCompilationJob(ParseInfo* parse_info,
+                                                     FunctionLiteral* literal,
+                                                     Isolate* isolate)
     : CompilationJob(isolate, parse_info, &compilation_info_, "Ignition"),
       zone_(isolate->allocator(), ZONE_NAME),
-      compilation_info_(&zone_, isolate, parse_info, literal, shared_info),
+      compilation_info_(&zone_, isolate, parse_info, literal),
       generator_(&compilation_info_),
       runtime_call_stats_(isolate->counters()->runtime_call_stats()),
       background_execute_counter_("CompileBackgroundIgnition") {}
@@ -210,11 +209,10 @@ InterpreterCompilationJob::Status InterpreterCompilationJob::FinalizeJobImpl() {
   return SUCCEEDED;
 }
 
-CompilationJob* Interpreter::NewCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
-    Handle<SharedFunctionInfo> shared_info, Isolate* isolate) {
-  return new InterpreterCompilationJob(parse_info, literal, shared_info,
-                                       isolate);
+CompilationJob* Interpreter::NewCompilationJob(ParseInfo* parse_info,
+                                               FunctionLiteral* literal,
+                                               Isolate* isolate) {
+  return new InterpreterCompilationJob(parse_info, literal, isolate);
 }
 
 bool Interpreter::IsDispatchTableInitialized() {

src/interpreter/interpreter.h

@@ -26,7 +26,6 @@ class CompilationJob;
 class FunctionLiteral;
 class ParseInfo;
 class SetupIsolateDelegate;
-class SharedFunctionInfo;
 class RootVisitor;
 
 namespace interpreter {
@@ -42,9 +41,9 @@ class Interpreter {
   static int InterruptBudget();
 
   // Creates a compilation job which will generate bytecode for |literal|.
-  static CompilationJob* NewCompilationJob(
-      ParseInfo* parse_info, FunctionLiteral* literal,
-      Handle<SharedFunctionInfo> shared_info, Isolate* isolate);
+  static CompilationJob* NewCompilationJob(ParseInfo* parse_info,
+                                           FunctionLiteral* literal,
+                                           Isolate* isolate);
 
   // Return bytecode handler for |bytecode|.
   Code* GetBytecodeHandler(Bytecode bytecode, OperandScale operand_scale);