Revert "[wasm] Split compilation in three stages"

This reverts commit 4e1d7c87.

Reason for revert:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20arm%20-%20sim%20-%20debug/14986

Original change's description:
> [wasm] Split compilation in three stages
> 
> In order to refactor ownership between objects in wasm compilation, the
> compilation (executed by background tasks) is split in three stages:
> getting a compilation unit (while holding a mutex), executing the work
> (without any mutex and without keeping the NativeModule alive), and
> submitting the work (with a mutex again).
> 
> This CL prepares this design by splitting compilation from submission.
> Both steps are still executed right after each other. This will be
> changed in a follow-up CL.
> 
> R=​titzer@chromium.org
> CC=​mstarzinger@chromium.org
> 
> Bug: v8:8689
> Change-Id: I2f92aee8e2f2d45470d8c63314ed026341630902
> Reviewed-on: https://chromium-review.googlesource.com/c/1414920
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58929}

TBR=titzer@chromium.org,clemensh@chromium.org

Change-Id: Ic3d0287b354ef5f834b76bc2cdc096d2231f4477
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8689
Reviewed-on: https://chromium-review.googlesource.com/c/1422917Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58932}

src/compiler/backend/code-generator.cc

@@ -45,8 +45,7 @@ CodeGenerator::CodeGenerator(
     InstructionSequence* code, OptimizedCompilationInfo* info, Isolate* isolate,
     base::Optional<OsrHelper> osr_helper, int start_source_position,
     JumpOptimizationInfo* jump_opt, PoisoningMitigationLevel poisoning_level,
-    const AssemblerOptions& options, int32_t builtin_index,
-    std::unique_ptr<AssemblerBuffer> buffer)
+    const AssemblerOptions& options, int32_t builtin_index)
     : zone_(codegen_zone),
       isolate_(isolate),
       frame_access_state_(nullptr),
@@ -58,7 +57,7 @@ CodeGenerator::CodeGenerator(
       current_block_(RpoNumber::Invalid()),
       start_source_position_(start_source_position),
       current_source_position_(SourcePosition::Unknown()),
-      tasm_(isolate, options, CodeObjectRequired::kNo, std::move(buffer)),
+      tasm_(isolate, options, CodeObjectRequired::kNo),
       resolver_(this),
       safepoints_(zone()),
       handlers_(zone()),

src/compiler/backend/code-generator.h

@@ -95,8 +95,8 @@ class CodeGenerator final : public GapResolver::Assembler {
                          int start_source_position,
                          JumpOptimizationInfo* jump_opt,
                          PoisoningMitigationLevel poisoning_level,
-                         const AssemblerOptions& options, int32_t builtin_index,
-                         std::unique_ptr<AssemblerBuffer> = {});
+                         const AssemblerOptions& options,
+                         int32_t builtin_index);
 
   // Generate native code. After calling AssembleCode, call FinalizeCode to
   // produce the actual code object. If an error occurs during either phase,

src/compiler/pipeline.cc

@@ -85,7 +85,6 @@
 #include "src/register-configuration.h"
 #include "src/utils.h"
 #include "src/wasm/function-body-decoder.h"
-#include "src/wasm/function-compiler.h"
 #include "src/wasm/wasm-engine.h"
 
 namespace v8 {
@@ -417,15 +416,14 @@ class PipelineData {
     start_source_position_ = position;
   }
 
-  void InitializeCodeGenerator(Linkage* linkage,
-                               std::unique_ptr<AssemblerBuffer> buffer) {
+  void InitializeCodeGenerator(Linkage* linkage) {
     DCHECK_NULL(code_generator_);
 
     code_generator_ = new CodeGenerator(
         codegen_zone(), frame(), linkage, sequence(), info(), isolate(),
         osr_helper_, start_source_position_, jump_optimization_info_,
         info()->GetPoisoningMitigationLevel(), assembler_options_,
-        info_->builtin_index(), std::move(buffer));
+        info_->builtin_index());
   }
 
   void BeginPhaseKind(const char* phase_kind_name) {
@@ -531,8 +529,7 @@ class PipelineImpl final {
   bool SelectInstructions(Linkage* linkage);
 
   // Step C. Run the code assembly pass.
-  void AssembleCode(Linkage* linkage,
-                    std::unique_ptr<AssemblerBuffer> buffer = {});
+  void AssembleCode(Linkage* linkage);
 
   // Step D. Run the code finalization pass.
   MaybeHandle<Code> FinalizeCode();
@@ -2358,21 +2355,16 @@ OptimizedCompilationJob* Pipeline::NewCompilationJob(
 }
 
 // static
-void Pipeline::GenerateCodeForWasmFunction(
+wasm::WasmCode* Pipeline::GenerateCodeForWasmFunction(
     OptimizedCompilationInfo* info, wasm::WasmEngine* wasm_engine,
     MachineGraph* mcgraph, CallDescriptor* call_descriptor,
     SourcePositionTable* source_positions, NodeOriginTable* node_origins,
-    wasm::FunctionBody function_body, const wasm::WasmModule* module,
+    wasm::FunctionBody function_body, wasm::NativeModule* native_module,
     int function_index) {
   ZoneStats zone_stats(wasm_engine->allocator());
   std::unique_ptr<PipelineStatistics> pipeline_statistics(
-      CreatePipelineStatistics(wasm_engine, function_body, module, info,
-                               &zone_stats));
-  // {instruction_buffer} must live longer than {PipelineData}, since
-  // {PipelineData} will reference the {instruction_buffer} via the
-  // {AssemblerBuffer} of the {Assembler} contained in the {CodeGenerator}.
-  std::unique_ptr<wasm::WasmInstructionBuffer> instruction_buffer =
-      wasm::WasmInstructionBuffer::New();
+      CreatePipelineStatistics(wasm_engine, function_body,
+                               native_module->module(), info, &zone_stats));
   PipelineData data(&zone_stats, wasm_engine, info, mcgraph,
                     pipeline_statistics.get(), source_positions, node_origins,
                     WasmAssemblerOptions());
@@ -2391,7 +2383,7 @@ void Pipeline::GenerateCodeForWasmFunction(
   pipeline.RunPrintAndVerify("Machine", true);
 
   data.BeginPhaseKind("wasm optimization");
-  const bool is_asm_js = module->origin == wasm::kAsmJsOrigin;
+  const bool is_asm_js = native_module->module()->origin == wasm::kAsmJsOrigin;
   if (FLAG_turbo_splitting && !is_asm_js) {
     data.info()->MarkAsSplittingEnabled();
   }
@@ -2430,19 +2422,21 @@ void Pipeline::GenerateCodeForWasmFunction(
   pipeline.ComputeScheduledGraph();
 
   Linkage linkage(call_descriptor);
-  if (!pipeline.SelectInstructions(&linkage)) return;
-  pipeline.AssembleCode(&linkage, instruction_buffer->CreateView());
+  if (!pipeline.SelectInstructions(&linkage)) return nullptr;
+  pipeline.AssembleCode(&linkage);
 
-  auto result = base::make_unique<wasm::WasmCompilationResult>();
   CodeGenerator* code_generator = pipeline.code_generator();
-  code_generator->tasm()->GetCode(nullptr, &result->code_desc);
+  CodeDesc code_desc;
+  code_generator->tasm()->GetCode(nullptr, &code_desc);
 
-  result->instr_buffer = instruction_buffer->ReleaseBuffer();
-  result->frame_slot_count = code_generator->frame()->GetTotalFrameSlotCount();
-  result->safepoint_table_offset = code_generator->GetSafepointTableOffset();
-  result->handler_table_offset = code_generator->GetHandlerTableOffset();
-  result->source_positions = code_generator->GetSourcePositionTable();
-  result->protected_instructions = code_generator->GetProtectedInstructions();
+  wasm::WasmCode* code = native_module->AddCode(
+      function_index, code_desc,
+      code_generator->frame()->GetTotalFrameSlotCount(),
+      code_generator->GetSafepointTableOffset(),
+      code_generator->GetHandlerTableOffset(),
+      code_generator->GetProtectedInstructions(),
+      code_generator->GetSourcePositionTable(), wasm::WasmCode::kFunction,
+      wasm::WasmCode::kTurbofan);
 
   if (data.info()->trace_turbo_json_enabled()) {
     TurboJsonFile json_of(data.info(), std::ios_base::app);
@@ -2450,9 +2444,9 @@ void Pipeline::GenerateCodeForWasmFunction(
 #ifdef ENABLE_DISASSEMBLER
     std::stringstream disassembler_stream;
     Disassembler::Decode(
-        nullptr, &disassembler_stream, result->code_desc.buffer,
-        result->code_desc.buffer + result->safepoint_table_offset,
-        CodeReference());  // TODO(clemensh): Fix code ref.
+        nullptr, &disassembler_stream, code->instructions().start(),
+        code->instructions().start() + code->safepoint_table_offset(),
+        CodeReference(code));
     for (auto const c : disassembler_stream.str()) {
       json_of << AsEscapedUC16ForJSON(c);
     }
@@ -2470,8 +2464,7 @@ void Pipeline::GenerateCodeForWasmFunction(
        << " using Turbofan" << std::endl;
   }
 
-  DCHECK(result->succeeded());
-  info->SetWasmCompilationResult(std::move(result));
+  return code;
 }
 
 bool Pipeline::AllocateRegistersForTesting(const RegisterConfiguration* config,
@@ -2666,11 +2659,10 @@ std::ostream& operator<<(std::ostream& out, const InstructionStartsAsJSON& s) {
   return out;
 }
 
-void PipelineImpl::AssembleCode(Linkage* linkage,
-                                std::unique_ptr<AssemblerBuffer> buffer) {
+void PipelineImpl::AssembleCode(Linkage* linkage) {
   PipelineData* data = this->data_;
   data->BeginPhaseKind("code generation");
-  data->InitializeCodeGenerator(linkage, std::move(buffer));
+  data->InitializeCodeGenerator(linkage);
 
   Run<AssembleCodePhase>();
   if (data->info()->trace_turbo_json_enabled()) {

src/compiler/pipeline.h

@@ -24,7 +24,6 @@ struct FunctionBody;
 class NativeModule;
 class WasmCode;
 class WasmEngine;
-struct WasmModule;
 }  // namespace wasm
 
 namespace compiler {
@@ -45,11 +44,11 @@ class Pipeline : public AllStatic {
                                                     bool has_script);
 
   // Run the pipeline for the WebAssembly compilation info.
-  static void GenerateCodeForWasmFunction(
+  static wasm::WasmCode* GenerateCodeForWasmFunction(
       OptimizedCompilationInfo* info, wasm::WasmEngine* wasm_engine,
       MachineGraph* mcgraph, CallDescriptor* call_descriptor,
       SourcePositionTable* source_positions, NodeOriginTable* node_origins,
-      wasm::FunctionBody function_body, const wasm::WasmModule* module,
+      wasm::FunctionBody function_body, wasm::NativeModule* native_module,
       int function_index);
 
   // Run the pipeline on a machine graph and generate code.

src/compiler/wasm-compiler.cc

@@ -5857,10 +5857,10 @@ TurbofanWasmCompilationUnit::TurbofanWasmCompilationUnit(
 TurbofanWasmCompilationUnit::~TurbofanWasmCompilationUnit() = default;
 
 bool TurbofanWasmCompilationUnit::BuildGraphForWasmFunction(
-    wasm::CompilationEnv* env, const wasm::FunctionBody& func_body,
-    wasm::WasmFeatures* detected, double* decode_ms, MachineGraph* mcgraph,
-    NodeOriginTable* node_origins, SourcePositionTable* source_positions,
-    wasm::WasmError* error_out) {
+    wasm::CompilationEnv* env, wasm::NativeModule* native_module,
+    const wasm::FunctionBody& func_body, wasm::WasmFeatures* detected,
+    double* decode_ms, MachineGraph* mcgraph, NodeOriginTable* node_origins,
+    SourcePositionTable* source_positions) {
   base::ElapsedTimer decode_timer;
   if (FLAG_trace_wasm_decode_time) {
     decode_timer.Start();
@@ -5878,7 +5878,8 @@ bool TurbofanWasmCompilationUnit::BuildGraphForWasmFunction(
                      << graph_construction_result.error().message()
                      << std::endl;
     }
-    *error_out = graph_construction_result.error();
+    native_module->compilation_state()->SetError(
+        wasm_unit_->func_index_, std::move(graph_construction_result).error());
     return false;
   }
 
@@ -5917,9 +5918,10 @@ Vector<const char> GetDebugName(Zone* zone, int index) {
 }
 }  // namespace
 
-wasm::WasmCompilationResult TurbofanWasmCompilationUnit::ExecuteCompilation(
-    wasm::CompilationEnv* env, const wasm::FunctionBody& func_body,
-    Counters* counters, wasm::WasmFeatures* detected) {
+void TurbofanWasmCompilationUnit::ExecuteCompilation(
+    wasm::CompilationEnv* env, wasm::NativeModule* native_module,
+    const wasm::FunctionBody& func_body, Counters* counters,
+    wasm::WasmFeatures* detected) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"),
                "ExecuteTurbofanCompilation");
   double decode_ms = 0;
@@ -5950,11 +5952,11 @@ wasm::WasmCompilationResult TurbofanWasmCompilationUnit::ExecuteCompilation(
                                       : nullptr;
   SourcePositionTable* source_positions =
       new (mcgraph->zone()) SourcePositionTable(mcgraph->graph());
-  wasm::WasmError error;
-  if (!BuildGraphForWasmFunction(env, func_body, detected, &decode_ms, mcgraph,
-                                 node_origins, source_positions, &error)) {
-    DCHECK(!error.empty());
-    return wasm::WasmCompilationResult{std::move(error)};
+  if (!BuildGraphForWasmFunction(env, native_module, func_body, detected,
+                                 &decode_ms, mcgraph, node_origins,
+                                 source_positions)) {
+    // Compilation failed.
+    return;
   }
 
   if (node_origins) {
@@ -5973,11 +5975,12 @@ wasm::WasmCompilationResult TurbofanWasmCompilationUnit::ExecuteCompilation(
     call_descriptor = GetI32WasmCallDescriptor(&zone, call_descriptor);
   }
 
-  Pipeline::GenerateCodeForWasmFunction(
-      &info, wasm_unit_->wasm_engine_, mcgraph, call_descriptor,
-      source_positions, node_origins, func_body, env->module,
-      wasm_unit_->func_index_);
-
+  if (wasm::WasmCode* wasm_code = Pipeline::GenerateCodeForWasmFunction(
+          &info, wasm_unit_->wasm_engine_, mcgraph, call_descriptor,
+          source_positions, node_origins, func_body, native_module,
+          wasm_unit_->func_index_)) {
+    wasm_unit_->SetResult(wasm_code, counters);
+  }
   if (FLAG_trace_wasm_decode_time) {
     double pipeline_ms = pipeline_timer.Elapsed().InMillisecondsF();
     PrintF(
@@ -5989,7 +5992,6 @@ wasm::WasmCompilationResult TurbofanWasmCompilationUnit::ExecuteCompilation(
   // TODO(bradnelson): Improve histogram handling of size_t.
   counters->wasm_compile_function_peak_memory_bytes()->AddSample(
       static_cast<int>(mcgraph->graph()->zone()->allocation_size()));
-  return std::move(*info.ReleaseWasmCompilationResult());
 }
 
 namespace {

src/compiler/wasm-compiler.h

@@ -51,17 +51,16 @@ class TurbofanWasmCompilationUnit {
   ~TurbofanWasmCompilationUnit();
 
   bool BuildGraphForWasmFunction(wasm::CompilationEnv* env,
+                                 wasm::NativeModule* native_module,
                                  const wasm::FunctionBody& func_body,
                                  wasm::WasmFeatures* detected,
                                  double* decode_ms, MachineGraph* mcgraph,
                                  NodeOriginTable* node_origins,
-                                 SourcePositionTable* source_positions,
-                                 wasm::WasmError* error_out);
+                                 SourcePositionTable* source_positions);
 
-  wasm::WasmCompilationResult ExecuteCompilation(wasm::CompilationEnv*,
-                                                 const wasm::FunctionBody&,
-                                                 Counters*,
-                                                 wasm::WasmFeatures* detected);
+  void ExecuteCompilation(wasm::CompilationEnv*, wasm::NativeModule*,
+                          const wasm::FunctionBody&, Counters*,
+                          wasm::WasmFeatures* detected);
 
  private:
   wasm::WasmCompilationUnit* const wasm_unit_;

src/optimized-compilation-info.cc

@@ -10,7 +10,6 @@
 #include "src/objects-inl.h"
 #include "src/objects/shared-function-info.h"
 #include "src/source-position.h"
-#include "src/wasm/function-compiler.h"
 
 namespace v8 {
 namespace internal {
@@ -150,16 +149,6 @@ StackFrame::Type OptimizedCompilationInfo::GetOutputStackFrameType() const {
   }
 }
 
-void OptimizedCompilationInfo::SetWasmCompilationResult(
-    std::unique_ptr<wasm::WasmCompilationResult> wasm_compilation_result) {
-  wasm_compilation_result_ = std::move(wasm_compilation_result);
-}
-
-std::unique_ptr<wasm::WasmCompilationResult>
-OptimizedCompilationInfo::ReleaseWasmCompilationResult() {
-  return std::move(wasm_compilation_result_);
-}
-
 bool OptimizedCompilationInfo::has_context() const {
   return !closure().is_null();
 }

src/optimized-compilation-info.h

@@ -26,10 +26,6 @@ class JavaScriptFrame;
 class JSGlobalObject;
 class Zone;
 
-namespace wasm {
-struct WasmCompilationResult;
-}
-
 // OptimizedCompilationInfo encapsulates the information needed to compile
 // optimized code for a given function, and the results of the optimized
 // compilation.
@@ -178,9 +174,6 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
 
   void SetCode(Handle<Code> code) { code_ = code; }
 
-  void SetWasmCompilationResult(std::unique_ptr<wasm::WasmCompilationResult>);
-  std::unique_ptr<wasm::WasmCompilationResult> ReleaseWasmCompilationResult();
-
   bool has_context() const;
   Context context() const;
 
@@ -298,9 +291,6 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
   // The compiled code.
   Handle<Code> code_;
 
-  // The WebAssembly compilation result, not published in the NativeModule yet.
-  std::unique_ptr<wasm::WasmCompilationResult> wasm_compilation_result_;
-
   // Entry point when compiling for OSR, {BailoutId::None} otherwise.
   BailoutId osr_offset_ = BailoutId::None();
 

src/vector.h

@@ -205,10 +205,7 @@ class OwnedVector {
             typename = typename std::enable_if<std::is_convertible<
                 std::unique_ptr<U>, std::unique_ptr<T>>::value>::type>
   OwnedVector(OwnedVector<U>&& other)
-      : data_(std::move(other.data_)), length_(other.length_) {
-    STATIC_ASSERT(sizeof(U) == sizeof(T));
-    other.length_ = 0;
-  }
+      : data_(other.ReleaseData()), length_(other.size()) {}
 
   // Returns the length of the vector as a size_t.
   constexpr size_t size() const { return length_; }
@@ -226,11 +223,8 @@ class OwnedVector {
   Vector<T> as_vector() const { return Vector<T>(start(), size()); }
 
   // Releases the backing data from this vector and transfers ownership to the
-  // caller. This vector will be empty afterwards.
-  std::unique_ptr<T[]> ReleaseData() {
-    length_ = 0;
-    return std::move(data_);
-  }
+  // caller. This vectors data can no longer be used afterwards.
+  std::unique_ptr<T[]> ReleaseData() { return std::move(data_); }
 
   // Allocates a new vector of the specified size via the default allocator.
   static OwnedVector<T> New(size_t size) {
@@ -252,13 +246,7 @@ class OwnedVector {
     return vec;
   }
 
-  bool operator==(std::nullptr_t) const { return data_ == nullptr; }
-  bool operator!=(std::nullptr_t) const { return data_ != nullptr; }
-
  private:
-  template <typename U>
-  friend class OwnedVector;
-
   std::unique_ptr<T[]> data_;
   size_t length_ = 0;
 };

src/wasm/baseline/liftoff-assembler.cc

@@ -484,9 +484,9 @@ constexpr AssemblerOptions DefaultLiftoffOptions() {
 
 // TODO(clemensh): Provide a reasonably sized buffer, based on wasm function
 // size.
-LiftoffAssembler::LiftoffAssembler(std::unique_ptr<AssemblerBuffer> buffer)
-    : TurboAssembler(nullptr, DefaultLiftoffOptions(), CodeObjectRequired::kNo,
-                     std::move(buffer)) {
+LiftoffAssembler::LiftoffAssembler()
+    : TurboAssembler(nullptr, DefaultLiftoffOptions(),
+                     CodeObjectRequired::kNo) {
   set_abort_hard(true);  // Avoid calls to Abort.
 }
 

src/wasm/baseline/liftoff-assembler.h

@@ -247,7 +247,7 @@ class LiftoffAssembler : public TurboAssembler {
     CacheState(const CacheState&) = delete;
   };
 
-  explicit LiftoffAssembler(std::unique_ptr<AssemblerBuffer>);
+  LiftoffAssembler();
   ~LiftoffAssembler() override;
 
   LiftoffRegister PopToRegister(LiftoffRegList pinned = {});

src/wasm/baseline/liftoff-compiler.cc

@@ -163,10 +163,8 @@ class LiftoffCompiler {
   };
 
   LiftoffCompiler(compiler::CallDescriptor* call_descriptor,
-                  CompilationEnv* env, Zone* compilation_zone,
-                  std::unique_ptr<AssemblerBuffer> buffer)
-      : asm_(std::move(buffer)),
-        descriptor_(
+                  CompilationEnv* env, Zone* compilation_zone)
+      : descriptor_(
             GetLoweredCallDescriptor(compilation_zone, call_descriptor)),
         env_(env),
         compilation_zone_(compilation_zone),
@@ -1950,9 +1948,11 @@ class LiftoffCompiler {
 
 }  // namespace
 
-WasmCompilationResult LiftoffCompilationUnit::ExecuteCompilation(
-    CompilationEnv* env, const FunctionBody& func_body, Counters* counters,
-    WasmFeatures* detected) {
+bool LiftoffCompilationUnit::ExecuteCompilation(CompilationEnv* env,
+                                                NativeModule* native_module,
+                                                const FunctionBody& func_body,
+                                                Counters* counters,
+                                                WasmFeatures* detected) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"),
                "ExecuteLiftoffCompilation");
   base::ElapsedTimer compile_timer;
@@ -1965,19 +1965,17 @@ WasmCompilationResult LiftoffCompilationUnit::ExecuteCompilation(
   auto call_descriptor = compiler::GetWasmCallDescriptor(&zone, func_body.sig);
   base::Optional<TimedHistogramScope> liftoff_compile_time_scope(
       base::in_place, counters->liftoff_compile_time());
-  std::unique_ptr<wasm::WasmInstructionBuffer> instruction_buffer =
-      wasm::WasmInstructionBuffer::New();
   WasmFullDecoder<Decoder::kValidate, LiftoffCompiler> decoder(
       &zone, module, env->enabled_features, detected, func_body,
-      call_descriptor, env, &zone, instruction_buffer->CreateView());
+      call_descriptor, env, &zone);
   decoder.Decode();
   liftoff_compile_time_scope.reset();
   LiftoffCompiler* compiler = &decoder.interface();
-  if (decoder.failed()) return WasmCompilationResult{decoder.error()};
+  if (decoder.failed()) return false;  // validation error
   if (!compiler->ok()) {
     // Liftoff compilation failed.
     counters->liftoff_unsupported_functions()->Increment();
-    return WasmCompilationResult{WasmError{0, "Liftoff bailout"}};
+    return false;
   }
 
   counters->liftoff_compiled_functions()->Increment();
@@ -1990,16 +1988,21 @@ WasmCompilationResult LiftoffCompilationUnit::ExecuteCompilation(
         static_cast<unsigned>(func_body.end - func_body.start), compile_ms);
   }
 
-  WasmCompilationResult result;
-  compiler->GetCode(&result.code_desc);
-  result.instr_buffer = instruction_buffer->ReleaseBuffer();
-  result.source_positions = compiler->GetSourcePositionTable();
-  result.protected_instructions = compiler->GetProtectedInstructions();
-  result.frame_slot_count = compiler->GetTotalFrameSlotCount();
-  result.safepoint_table_offset = compiler->GetSafepointTableOffset();
+  CodeDesc desc;
+  compiler->GetCode(&desc);
+  OwnedVector<byte> source_positions = compiler->GetSourcePositionTable();
+  OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions =
+      compiler->GetProtectedInstructions();
+  uint32_t frame_slot_count = compiler->GetTotalFrameSlotCount();
+  int safepoint_table_offset = compiler->GetSafepointTableOffset();
+
+  WasmCode* code = native_module->AddCode(
+      wasm_unit_->func_index_, desc, frame_slot_count, safepoint_table_offset,
+      0, std::move(protected_instructions), std::move(source_positions),
+      WasmCode::kFunction, WasmCode::kLiftoff);
+  wasm_unit_->SetResult(code, counters);
 
-  DCHECK(result.succeeded());
-  return result;
+  return true;
 }
 
 #undef __

src/wasm/baseline/liftoff-compiler.h

@@ -6,7 +6,6 @@
 #define V8_WASM_BASELINE_LIFTOFF_COMPILER_H_
 
 #include "src/base/macros.h"
-#include "src/wasm/function-compiler.h"
 
 namespace v8 {
 namespace internal {
@@ -18,6 +17,7 @@ namespace wasm {
 struct CompilationEnv;
 struct FunctionBody;
 class NativeModule;
+class WasmCompilationUnit;
 struct WasmFeatures;
 
 class LiftoffCompilationUnit final {
@@ -25,9 +25,8 @@ class LiftoffCompilationUnit final {
   explicit LiftoffCompilationUnit(WasmCompilationUnit* wasm_unit)
       : wasm_unit_(wasm_unit) {}
 
-  WasmCompilationResult ExecuteCompilation(CompilationEnv*, const FunctionBody&,
-                                           Counters*,
-                                           WasmFeatures* detected_features);
+  bool ExecuteCompilation(CompilationEnv*, NativeModule*, const FunctionBody&,
+                          Counters*, WasmFeatures* detected);
 
  private:
   WasmCompilationUnit* const wasm_unit_;

src/wasm/compilation-environment.h

@@ -8,7 +8,6 @@
 #include "src/wasm/wasm-features.h"
 #include "src/wasm/wasm-limits.h"
 #include "src/wasm/wasm-module.h"
-#include "src/wasm/wasm-tier.h"
 
 namespace v8 {
 namespace internal {
@@ -114,7 +113,6 @@ class CompilationState {
 
  private:
   friend class NativeModule;
-  friend class WasmCompilationUnit;
   CompilationState() = delete;
 
   static std::unique_ptr<CompilationState> New(Isolate*, NativeModule*);

src/wasm/function-compiler.cc

@@ -28,92 +28,8 @@ const char* GetExecutionTierAsString(ExecutionTier tier) {
   UNREACHABLE();
 }
 
-class WasmInstructionBufferImpl {
- public:
-  class View : public AssemblerBuffer {
-   public:
-    View(Vector<uint8_t> buffer, WasmInstructionBufferImpl* holder)
-        : buffer_(buffer), holder_(holder) {}
-
-    ~View() override {
-      if (buffer_.start() == holder_->old_buffer_.start()) {
-        DCHECK_EQ(buffer_.size(), holder_->old_buffer_.size());
-        holder_->old_buffer_ = {};
-      }
-    }
-
-    byte* start() const override { return buffer_.start(); }
-
-    int size() const override { return static_cast<int>(buffer_.size()); }
-
-    std::unique_ptr<AssemblerBuffer> Grow(int new_size) override {
-      // If we grow, we must be the current buffer of {holder_}.
-      DCHECK_EQ(buffer_.start(), holder_->buffer_.start());
-      DCHECK_EQ(buffer_.size(), holder_->buffer_.size());
-      DCHECK_NULL(holder_->old_buffer_);
-
-      DCHECK_LT(size(), new_size);
-
-      holder_->old_buffer_ = std::move(holder_->buffer_);
-      holder_->buffer_ = OwnedVector<uint8_t>::New(new_size);
-      return base::make_unique<View>(holder_->buffer_.as_vector(), holder_);
-    }
-
-   private:
-    const Vector<uint8_t> buffer_;
-    WasmInstructionBufferImpl* const holder_;
-  };
-
-  std::unique_ptr<AssemblerBuffer> CreateView() {
-    DCHECK_NOT_NULL(buffer_);
-    return base::make_unique<View>(buffer_.as_vector(), this);
-  }
-
-  std::unique_ptr<uint8_t[]> ReleaseBuffer() {
-    DCHECK_NULL(old_buffer_);
-    DCHECK_NOT_NULL(buffer_);
-    return buffer_.ReleaseData();
-  }
-
-  bool released() const { return buffer_ == nullptr; }
-
- private:
-  // The current buffer used to emit code.
-  OwnedVector<uint8_t> buffer_ =
-      OwnedVector<uint8_t>::New(AssemblerBase::kMinimalBufferSize);
-
-  // While the buffer is grown, we need to temporarily also keep the old
-  // buffer alive.
-  OwnedVector<uint8_t> old_buffer_;
-};
-
-WasmInstructionBufferImpl* Impl(WasmInstructionBuffer* buf) {
-  return reinterpret_cast<WasmInstructionBufferImpl*>(buf);
-}
-
 }  // namespace
 
-// PIMPL interface WasmInstructionBuffer for WasmInstBufferImpl
-WasmInstructionBuffer::~WasmInstructionBuffer() {
-  Impl(this)->~WasmInstructionBufferImpl();
-}
-
-std::unique_ptr<AssemblerBuffer> WasmInstructionBuffer::CreateView() {
-  return Impl(this)->CreateView();
-}
-
-std::unique_ptr<uint8_t[]> WasmInstructionBuffer::ReleaseBuffer() {
-  return Impl(this)->ReleaseBuffer();
-}
-
-// static
-std::unique_ptr<WasmInstructionBuffer> WasmInstructionBuffer::New() {
-  return std::unique_ptr<WasmInstructionBuffer>{
-      reinterpret_cast<WasmInstructionBuffer*>(
-          new WasmInstructionBufferImpl())};
-}
-// End of PIMPL interface WasmInstructionBuffer for WasmInstBufferImpl
-
 // static
 ExecutionTier WasmCompilationUnit::GetDefaultExecutionTier(
     const WasmModule* module) {
@@ -136,8 +52,8 @@ WasmCompilationUnit::WasmCompilationUnit(WasmEngine* wasm_engine, int index,
 // {TurbofanWasmCompilationUnit} can be opaque in the header file.
 WasmCompilationUnit::~WasmCompilationUnit() = default;
 
-WasmCompilationResult WasmCompilationUnit::ExecuteCompilation(
-    CompilationEnv* env,
+void WasmCompilationUnit::ExecuteCompilation(
+    CompilationEnv* env, NativeModule* native_module,
     const std::shared_ptr<WireBytesStorage>& wire_bytes_storage,
     Counters* counters, WasmFeatures* detected) {
   auto* func = &env->module->functions[func_index_];
@@ -157,57 +73,24 @@ WasmCompilationResult WasmCompilationUnit::ExecuteCompilation(
            GetExecutionTierAsString(tier_));
   }
 
-  WasmCompilationResult result;
   switch (tier_) {
     case ExecutionTier::kBaseline:
-      result =
-          liftoff_unit_->ExecuteCompilation(env, func_body, counters, detected);
-      if (result.succeeded()) break;
+      if (liftoff_unit_->ExecuteCompilation(env, native_module, func_body,
+                                            counters, detected)) {
+        break;
+      }
       // Otherwise, fall back to turbofan.
       SwitchTier(ExecutionTier::kOptimized);
       // TODO(wasm): We could actually stop or remove the tiering unit for this
       // function to avoid compiling it twice with TurboFan.
       V8_FALLTHROUGH;
     case ExecutionTier::kOptimized:
-      result = turbofan_unit_->ExecuteCompilation(env, func_body, counters,
-                                                  detected);
+      turbofan_unit_->ExecuteCompilation(env, native_module, func_body,
+                                         counters, detected);
       break;
     case ExecutionTier::kInterpreter:
       UNREACHABLE();  // TODO(titzer): compile interpreter entry stub.
   }
-
-  if (result.succeeded()) {
-    counters->wasm_generated_code_size()->Increment(
-        result.code_desc.instr_size);
-    counters->wasm_reloc_size()->Increment(result.code_desc.reloc_size);
-  }
-
-  return result;
-}
-
-WasmCode* WasmCompilationUnit::Publish(WasmCompilationResult result,
-                                       NativeModule* native_module) {
-  if (!result.succeeded()) {
-    native_module->compilation_state()->SetError(func_index_,
-                                                 std::move(result.error));
-    return nullptr;
-  }
-
-  // The {tier} argument specifies the requested tier, which can differ from the
-  // actually executed tier stored in {unit->tier()}.
-  DCHECK(result.succeeded());
-  WasmCode::Tier code_tier = tier_ == ExecutionTier::kBaseline
-                                 ? WasmCode::kLiftoff
-                                 : WasmCode::kTurbofan;
-  DCHECK_EQ(result.code_desc.buffer, result.instr_buffer.get());
-  WasmCode* code = native_module->AddCode(
-      func_index_, result.code_desc, result.frame_slot_count,
-      result.safepoint_table_offset, result.handler_table_offset,
-      std::move(result.protected_instructions),
-      std::move(result.source_positions), WasmCode::kFunction, code_tier);
-  // TODO(clemensh): Merge this into {AddCode}?
-  native_module->PublishCode(code);
-  return code;
 }
 
 void WasmCompilationUnit::SwitchTier(ExecutionTier new_tier) {
@@ -245,10 +128,21 @@ void WasmCompilationUnit::CompileWasmFunction(Isolate* isolate,
 
   WasmCompilationUnit unit(isolate->wasm_engine(), function->func_index, tier);
   CompilationEnv env = native_module->CreateCompilationEnv();
-  WasmCompilationResult result = unit.ExecuteCompilation(
-      &env, native_module->compilation_state()->GetWireBytesStorage(),
+  unit.ExecuteCompilation(
+      &env, native_module,
+      native_module->compilation_state()->GetWireBytesStorage(),
       isolate->counters(), detected);
-  unit.Publish(std::move(result), native_module);
+}
+
+void WasmCompilationUnit::SetResult(WasmCode* code, Counters* counters) {
+  DCHECK_NULL(result_);
+  result_ = code;
+  code->native_module()->PublishCode(code);
+
+  counters->wasm_generated_code_size()->Increment(
+      static_cast<int>(code->instructions().size()));
+  counters->wasm_reloc_size()->Increment(
+      static_cast<int>(code->reloc_info().size()));
 }
 
 }  // namespace wasm

src/wasm/function-compiler.h

@@ -5,7 +5,6 @@
 #ifndef V8_WASM_FUNCTION_COMPILER_H_
 #define V8_WASM_FUNCTION_COMPILER_H_
 
-#include "src/trap-handler/trap-handler.h"
 #include "src/wasm/compilation-environment.h"
 #include "src/wasm/function-body-decoder.h"
 #include "src/wasm/wasm-limits.h"
@@ -15,11 +14,9 @@
 namespace v8 {
 namespace internal {
 
-class AssemblerBuffer;
 class Counters;
 
 namespace compiler {
-class Pipeline;
 class TurbofanWasmCompilationUnit;
 }  // namespace compiler
 
@@ -28,46 +25,9 @@ namespace wasm {
 class LiftoffCompilationUnit;
 class NativeModule;
 class WasmCode;
-class WasmCompilationUnit;
 class WasmEngine;
 struct WasmFunction;
 
-class WasmInstructionBuffer final {
- public:
-  ~WasmInstructionBuffer();
-  std::unique_ptr<AssemblerBuffer> CreateView();
-  std::unique_ptr<uint8_t[]> ReleaseBuffer();
-
-  static std::unique_ptr<WasmInstructionBuffer> New();
-
- private:
-  WasmInstructionBuffer() = delete;
-  DISALLOW_COPY_AND_ASSIGN(WasmInstructionBuffer);
-};
-
-struct WasmCompilationResult {
- public:
-  MOVE_ONLY_WITH_DEFAULT_CONSTRUCTORS(WasmCompilationResult);
-
-  explicit WasmCompilationResult(WasmError error) : error(std::move(error)) {}
-
-  bool succeeded() const {
-    DCHECK_EQ(code_desc.buffer != nullptr, error.empty());
-    return error.empty();
-  }
-  operator bool() const { return succeeded(); }
-
-  CodeDesc code_desc;
-  std::unique_ptr<uint8_t[]> instr_buffer;
-  uint32_t frame_slot_count = 0;
-  size_t safepoint_table_offset = 0;
-  size_t handler_table_offset = 0;
-  OwnedVector<byte> source_positions;
-  OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions;
-
-  WasmError error;
-};
-
 class WasmCompilationUnit final {
  public:
   static ExecutionTier GetDefaultExecutionTier(const WasmModule*);
@@ -81,13 +41,12 @@ class WasmCompilationUnit final {
 
   ~WasmCompilationUnit();
 
-  WasmCompilationResult ExecuteCompilation(
-      CompilationEnv*, const std::shared_ptr<WireBytesStorage>&, Counters*,
-      WasmFeatures* detected);
-
-  WasmCode* Publish(WasmCompilationResult, NativeModule*);
+  void ExecuteCompilation(CompilationEnv*, NativeModule*,
+                          const std::shared_ptr<WireBytesStorage>&, Counters*,
+                          WasmFeatures* detected);
 
   ExecutionTier tier() const { return tier_; }
+  WasmCode* result() const { return result_; }
 
   static void CompileWasmFunction(Isolate*, NativeModule*,
                                   WasmFeatures* detected, const WasmFunction*,
@@ -109,6 +68,9 @@ class WasmCompilationUnit final {
 
   void SwitchTier(ExecutionTier new_tier);
 
+  // Called from {ExecuteCompilation} to set the result of compilation.
+  void SetResult(WasmCode*, Counters*);
+
   DISALLOW_COPY_AND_ASSIGN(WasmCompilationUnit);
 };
 

src/wasm/module-compiler.cc

@@ -347,15 +347,15 @@ WasmCode* LazyCompileFunction(Isolate* isolate, NativeModule* native_module,
                          module_start + func->code.offset(),
                          module_start + func->code.end_offset()};
 
-  ExecutionTier tier =
-      WasmCompilationUnit::GetDefaultExecutionTier(native_module->module());
-  WasmCompilationUnit unit(isolate->wasm_engine(), func_index, tier);
+  WasmCompilationUnit unit(
+      isolate->wasm_engine(), func_index,
+      WasmCompilationUnit::GetDefaultExecutionTier(native_module->module()));
   CompilationEnv env = native_module->CreateCompilationEnv();
-  WasmCompilationResult result = unit.ExecuteCompilation(
-      &env, native_module->compilation_state()->GetWireBytesStorage(),
+  unit.ExecuteCompilation(
+      &env, native_module,
+      native_module->compilation_state()->GetWireBytesStorage(),
       isolate->counters(),
       Impl(native_module->compilation_state())->detected_features());
-  WasmCode* code = unit.Publish(std::move(result), native_module);
 
   // During lazy compilation, we should never get compilation errors. The module
   // was verified before starting execution with lazy compilation.
@@ -364,6 +364,8 @@ WasmCode* LazyCompileFunction(Isolate* isolate, NativeModule* native_module,
   // module creation time, and return a function that always traps here.
   CHECK(!native_module->compilation_state()->failed());
 
+  WasmCode* code = unit.result();
+
   if (WasmCode::ShouldBeLogged(isolate)) code->LogCode(isolate);
 
   int64_t func_size =
@@ -488,11 +490,10 @@ bool FetchAndExecuteCompilationUnit(CompilationEnv* env,
   // Get the tier before starting compilation, as compilation can switch tiers
   // if baseline bails out.
   ExecutionTier tier = unit->tier();
-  WasmCompilationResult result = unit->ExecuteCompilation(
-      env, compilation_state->GetWireBytesStorage(), counters, detected);
-
-  WasmCode* code = unit->Publish(std::move(result), native_module);
-  compilation_state->OnFinishedUnit(tier, code);
+  unit->ExecuteCompilation(env, native_module,
+                           compilation_state->GetWireBytesStorage(), counters,
+                           detected);
+  compilation_state->OnFinishedUnit(tier, unit->result());
 
   return true;
 }

test/cctest/wasm/wasm-run-utils.cc

@@ -421,12 +421,13 @@ void WasmFunctionCompiler::Build(const byte* start, const byte* end) {
   WasmCompilationUnit unit(isolate()->wasm_engine(), function_->func_index,
                            tier);
   WasmFeatures unused_detected_features;
-  WasmCompilationResult result = unit.ExecuteCompilation(
-      &env, native_module->compilation_state()->GetWireBytesStorage(),
+  unit.ExecuteCompilation(
+      &env, native_module,
+      native_module->compilation_state()->GetWireBytesStorage(),
       isolate()->counters(), &unused_detected_features);
-  WasmCode* code = unit.Publish(std::move(result), native_module);
-  DCHECK_NOT_NULL(code);
-  if (WasmCode::ShouldBeLogged(isolate())) code->LogCode(isolate());
+  WasmCode* result = unit.result();
+  DCHECK_NOT_NULL(result);
+  if (WasmCode::ShouldBeLogged(isolate())) result->LogCode(isolate());
 }
 
 WasmFunctionCompiler::WasmFunctionCompiler(Zone* zone, FunctionSig* sig,