[Liftoff] Execute more work during background compilation

We can actually prepare everything in the background, all that remains
to do in the finisher task (on the main thread) is actually adding the
code to the {NativeModule}.
As a next step, even that should happen in the background.

R=mstarzinger@chromium.org

Bug: v8:6600
Change-Id: I570f99a9aa7dc7e324046da36cca9b4297f1bc5e
Reviewed-on: https://chromium-review.googlesource.com/1126391
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54239}

src/wasm/baseline/liftoff-compiler.cc

@@ -133,9 +133,6 @@ class LiftoffCompiler {
 
   LiftoffCompiler(LiftoffAssembler* liftoff_asm,
                   compiler::CallDescriptor* call_descriptor, ModuleEnv* env,
-                  SourcePositionTableBuilder* source_position_table_builder,
-                  std::vector<trap_handler::ProtectedInstructionData>*
-                      protected_instructions,
                   Zone* compilation_zone)
       : asm_(liftoff_asm),
         descriptor_(
@@ -146,8 +143,6 @@ class LiftoffCompiler {
                                ? env_->module->maximum_pages
                                : wasm::kV8MaxWasmMemoryPages} *
                   wasm::kWasmPageSize),
-        source_position_table_builder_(source_position_table_builder),
-        protected_instructions_(protected_instructions),
         compilation_zone_(compilation_zone),
         safepoint_table_builder_(compilation_zone_) {}
 
@@ -155,6 +150,20 @@ class LiftoffCompiler {
 
   bool ok() const { return ok_; }
 
+  OwnedVector<uint8_t> GetSourcePositionTable() {
+    return source_position_table_builder_.ToSourcePositionTableVector();
+  }
+
+  OwnedVector<trap_handler::ProtectedInstructionData> GetProtectedInstructions()
+      const {
+    return OwnedVector<trap_handler::ProtectedInstructionData>::Of(
+        protected_instructions_);
+  }
+
+  uint32_t GetTotalFrameSlotCount() const {
+    return __ GetTotalFrameSlotCount();
+  }
+
   void unsupported(Decoder* decoder, const char* reason) {
     ok_ = false;
     TRACE("unsupported: %s\n", reason);
@@ -163,8 +172,8 @@ class LiftoffCompiler {
   }
 
   bool DidAssemblerBailout(Decoder* decoder) {
-    if (decoder->failed() || !asm_->did_bailout()) return false;
-    unsupported(decoder, asm_->bailout_reason());
+    if (decoder->failed() || !__ did_bailout()) return false;
+    unsupported(decoder, __ bailout_reason());
     return true;
   }
 
@@ -380,7 +389,7 @@ class LiftoffCompiler {
     if (is_mem_out_of_bounds && env_->use_trap_handler) {
       uint32_t pc = static_cast<uint32_t>(__ pc_offset());
       DCHECK_EQ(pc, __ pc_offset());
-      protected_instructions_->emplace_back(
+      protected_instructions_.emplace_back(
           trap_handler::ProtectedInstructionData{ool.pc, pc});
     }
 
@@ -398,7 +407,7 @@ class LiftoffCompiler {
 
     if (!ool.regs_to_save.is_empty()) __ PushRegisters(ool.regs_to_save);
 
-    source_position_table_builder_->AddPosition(
+    source_position_table_builder_.AddPosition(
         __ pc_offset(), SourcePosition(ool.position), false);
     __ CallRuntimeStub(ool.stub);
     safepoint_table_builder_.DefineSafepoint(asm_, Safepoint::kSimple, 0,
@@ -1387,8 +1396,8 @@ class LiftoffCompiler {
     __ Store(info.gp(), no_reg, offsetof(wasm::MemoryTracingInfo, mem_rep),
              address, StoreType::kI32Store8, pinned);
 
-    source_position_table_builder_->AddPosition(
-        __ pc_offset(), SourcePosition(position), false);
+    source_position_table_builder_.AddPosition(__ pc_offset(),
+                                               SourcePosition(position), false);
 
     Register args[] = {info.gp()};
     GenerateRuntimeCall(Runtime::kWasmTraceMemory, arraysize(args), args);
@@ -1594,7 +1603,7 @@ class LiftoffCompiler {
       LiftoffRegister* explicit_instance = &target_instance;
       Register target_reg = target.gp();
       __ PrepareCall(imm.sig, call_descriptor, &target_reg, explicit_instance);
-      source_position_table_builder_->AddPosition(
+      source_position_table_builder_.AddPosition(
           __ pc_offset(), SourcePosition(decoder->position()), false);
 
       __ CallIndirect(imm.sig, call_descriptor, target_reg);
@@ -1607,7 +1616,7 @@ class LiftoffCompiler {
       // A direct call within this module just gets the current instance.
       __ PrepareCall(imm.sig, call_descriptor);
 
-      source_position_table_builder_->AddPosition(
+      source_position_table_builder_.AddPosition(
           __ pc_offset(), SourcePosition(decoder->position()), false);
 
       // Just encode the function index. This will be patched at instantiation.
@@ -1727,7 +1736,7 @@ class LiftoffCompiler {
             pinned);
     LiftoffRegister* explicit_instance = &tmp_const;
 
-    source_position_table_builder_->AddPosition(
+    source_position_table_builder_.AddPosition(
         __ pc_offset(), SourcePosition(decoder->position()), false);
 
     auto call_descriptor =
@@ -1788,8 +1797,8 @@ class LiftoffCompiler {
   const uint64_t max_size_;
   bool ok_ = true;
   std::vector<OutOfLineCode> out_of_line_code_;
-  SourcePositionTableBuilder* const source_position_table_builder_;
-  std::vector<trap_handler::ProtectedInstructionData>* protected_instructions_;
+  SourcePositionTableBuilder source_position_table_builder_;
+  std::vector<trap_handler::ProtectedInstructionData> protected_instructions_;
   // Zone used to store information during compilation. The result will be
   // stored independently, such that this zone can die together with the
   // LiftoffCompiler after compilation.
@@ -1805,10 +1814,10 @@ class LiftoffCompiler {
     StdoutStream os;
     for (int control_depth = decoder->control_depth() - 1; control_depth >= -1;
          --control_depth) {
-      LiftoffAssembler::CacheState* cache_state =
-          control_depth == -1
-              ? asm_->cache_state()
-              : &decoder->control_at(control_depth)->label_state;
+      auto* cache_state =
+          control_depth == -1 ? __ cache_state()
+                              : &decoder->control_at(control_depth)
+                                     ->label_state;
       os << PrintCollection(cache_state->stack_state);
       if (control_depth != -1) PrintF("; ");
     }
@@ -1832,19 +1841,22 @@ bool LiftoffCompilationUnit::ExecuteCompilation() {
       compiler::GetWasmCallDescriptor(&zone, wasm_unit_->func_body_.sig);
   base::Optional<TimedHistogramScope> liftoff_compile_time_scope(
       base::in_place, wasm_unit_->counters_->liftoff_compile_time());
-  DCHECK(protected_instructions_.empty());
   wasm::WasmFullDecoder<wasm::Decoder::kValidate, wasm::LiftoffCompiler>
       decoder(&zone, module, wasm_unit_->func_body_, &asm_, call_descriptor,
-              wasm_unit_->env_, &source_position_table_builder_,
-              &protected_instructions_, &zone);
+              wasm_unit_->env_, &zone);
   decoder.Decode();
   liftoff_compile_time_scope.reset();
-  if (!decoder.interface().ok()) {
+  wasm::LiftoffCompiler* compiler = &decoder.interface();
+  bool liftoff_bailout = !compiler->ok();
+  bool validation_error = decoder.failed();
+  if (liftoff_bailout || validation_error) {
     // Liftoff compilation failed.
-    wasm_unit_->counters_->liftoff_unsupported_functions()->Increment();
+    if (liftoff_bailout) {
+      wasm_unit_->counters_->liftoff_unsupported_functions()->Increment();
+    }
+    asm_.AbortCompilation();
     return false;
   }
-  if (decoder.failed()) return false;  // Validation error
 
   if (FLAG_trace_wasm_decode_time) {
     double compile_ms = compile_timer.Elapsed().InMillisecondsF();
@@ -1856,39 +1868,29 @@ bool LiftoffCompilationUnit::ExecuteCompilation() {
         compile_ms);
   }
 
+  wasm_unit_->counters_->liftoff_compiled_functions()->Increment();
+
+  asm_.GetCode(nullptr, &desc_);
+  source_positions_ = compiler->GetSourcePositionTable();
+  protected_instructions_ = compiler->GetProtectedInstructions();
+  frame_slot_count_ = compiler->GetTotalFrameSlotCount();
+  safepoint_table_offset_ = compiler->GetSafepointTableOffset();
+
   // Record the memory cost this unit places on the system until
   // it is finalized.
   wasm_unit_->memory_cost_ =
-      asm_.pc_offset() + protected_instructions_.size() *
-                             sizeof(trap_handler::ProtectedInstructionData);
+      desc_.buffer_size + source_positions_.size() +
+      protected_instructions_.size() * sizeof(*protected_instructions_.start());
 
-  safepoint_table_offset_ = decoder.interface().GetSafepointTableOffset();
-  wasm_unit_->counters_->liftoff_compiled_functions()->Increment();
   return true;
 }
 
-wasm::WasmCode* LiftoffCompilationUnit::FinishCompilation(
-    wasm::ErrorThrower* thrower) {
-  CodeDesc desc;
-  asm_.GetCode(nullptr, &desc);
-
-  OwnedVector<byte> source_positions =
-      source_position_table_builder_.ToSourcePositionTableVector();
-  auto protected_instructions_copy =
-      OwnedVector<trap_handler::ProtectedInstructionData>::Of(
-          protected_instructions_);
-  wasm::WasmCode* code = wasm_unit_->native_module_->AddCode(
-      wasm_unit_->func_index_, desc, asm_.GetTotalFrameSlotCount(),
-      safepoint_table_offset_, 0, std::move(protected_instructions_copy),
-      std::move(source_positions), wasm::WasmCode::kLiftoff);
-
-  return code;
-}
-
-void LiftoffCompilationUnit::AbortCompilation() {
-  // The compilation is aborted. Put the assembler in a clean mode before
-  // its deletion.
-  asm_.AbortCompilation();
+wasm::WasmCode* LiftoffCompilationUnit::FinishCompilation(wasm::ErrorThrower*) {
+  // TODO(clemensh): Also move this to {ExecuteCompilation}.
+  return wasm_unit_->native_module_->AddCode(
+      wasm_unit_->func_index_, desc_, frame_slot_count_,
+      safepoint_table_offset_, 0, std::move(protected_instructions_),
+      std::move(source_positions_), wasm::WasmCode::kLiftoff);
 }
 
 #undef __

src/wasm/baseline/liftoff-compiler.h

@@ -22,14 +22,19 @@ class LiftoffCompilationUnit final {
 
   bool ExecuteCompilation();
   wasm::WasmCode* FinishCompilation(wasm::ErrorThrower*);
-  void AbortCompilation();
 
  private:
   WasmCompilationUnit* const wasm_unit_;
-  wasm::LiftoffAssembler asm_;
+  // Must stay alive until the code is added to the {NativeModule}, because it
+  // contains the instruction buffer.
+  LiftoffAssembler asm_;
+
+  // Result of compilation:
+  CodeDesc desc_;
+  OwnedVector<byte> source_positions_;
+  OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions_;
+  uint32_t frame_slot_count_;
   int safepoint_table_offset_;
-  SourcePositionTableBuilder source_position_table_builder_;
-  std::vector<trap_handler::ProtectedInstructionData> protected_instructions_;
 
   DISALLOW_COPY_AND_ASSIGN(LiftoffCompilationUnit);
 };

src/wasm/function-compiler.cc

@@ -118,7 +118,6 @@ void WasmCompilationUnit::SwitchMode(CompilationMode new_mode) {
       return;
     case CompilationMode::kTurbofan:
       DCHECK(!turbofan_unit_);
-      if (liftoff_unit_ != nullptr) liftoff_unit_->AbortCompilation();
       liftoff_unit_.reset();
       turbofan_unit_.reset(new compiler::TurbofanWasmCompilationUnit(this));
       return;