[wasm] Remove WasmCompilationUnit::failed

Compilation failures are already stored in the {CompilationState}. We
never use the information which individual compilation unit failed.
Hence remove that getter, and only check for failure of the overall
compilation.

R=ahaas@chromium.org

Bug: v8:7921, v8:8343
Change-Id: Ibf90be233c9ff576ec8a3413ba5abefe2fdb645e
Reviewed-on: https://chromium-review.googlesource.com/c/1373783Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58195}

src/runtime/runtime-test.cc

@@ -1087,11 +1087,10 @@ RUNTIME_FUNCTION(Runtime_WasmTierUpFunction) {
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(WasmInstanceObject, instance, 0);
   CONVERT_SMI_ARG_CHECKED(function_index, 1);
-  if (!isolate->wasm_engine()->CompileFunction(
-          isolate, instance->module_object()->native_module(), function_index,
-          wasm::ExecutionTier::kOptimized)) {
-    return ReadOnlyRoots(isolate).exception();
-  }
+  auto* native_module = instance->module_object()->native_module();
+  isolate->wasm_engine()->CompileFunction(
+      isolate, native_module, function_index, wasm::ExecutionTier::kOptimized);
+  CHECK(!native_module->compilation_state()->failed());
   return ReadOnlyRoots(isolate).undefined_value();
 }
 

src/wasm/compilation-environment.h

@@ -99,10 +99,12 @@ class CompilationState {
 
   void SetWireBytesStorage(std::shared_ptr<WireBytesStorage>);
 
-  std::shared_ptr<WireBytesStorage> GetWireBytesStorage();
+  std::shared_ptr<WireBytesStorage> GetWireBytesStorage() const;
 
   void AddCallback(callback_t);
 
+  bool failed() const;
+
  private:
   friend class NativeModule;
   CompilationState() = delete;

src/wasm/function-compiler.cc

@@ -125,7 +125,7 @@ void WasmCompilationUnit::SwitchTier(ExecutionTier new_tier) {
 }
 
 // static
-bool WasmCompilationUnit::CompileWasmFunction(Isolate* isolate,
+void WasmCompilationUnit::CompileWasmFunction(Isolate* isolate,
                                               NativeModule* native_module,
                                               WasmFeatures* detected,
                                               const WasmFunction* function,
@@ -141,7 +141,6 @@ bool WasmCompilationUnit::CompileWasmFunction(Isolate* isolate,
   unit.ExecuteCompilation(
       &env, native_module->compilation_state()->GetWireBytesStorage(),
       isolate->counters(), detected);
-  return !unit.failed();
 }
 
 void WasmCompilationUnit::SetResult(WasmCode* code, Counters* counters) {

src/wasm/function-compiler.h

@@ -47,10 +47,9 @@ class WasmCompilationUnit final {
 
   NativeModule* native_module() const { return native_module_; }
   ExecutionTier tier() const { return tier_; }
-  bool failed() const { return result_ == nullptr; }  // TODO(clemensh): Remove.
   WasmCode* result() const { return result_; }
 
-  static bool CompileWasmFunction(Isolate* isolate, NativeModule* native_module,
+  static void CompileWasmFunction(Isolate* isolate, NativeModule* native_module,
                                   WasmFeatures* detected,
                                   const WasmFunction* function,
                                   ExecutionTier = GetDefaultExecutionTier());

src/wasm/module-compiler.cc

@@ -157,7 +157,7 @@ class CompilationStateImpl {
     wire_bytes_storage_ = wire_bytes_storage;
   }
 
-  std::shared_ptr<WireBytesStorage> GetWireBytesStorage() {
+  std::shared_ptr<WireBytesStorage> GetWireBytesStorage() const {
     base::MutexGuard guard(&mutex_);
     return wire_bytes_storage_;
   }
@@ -436,12 +436,17 @@ class InstanceBuilder {
 CompilationStateImpl* Impl(CompilationState* compilation_state) {
   return reinterpret_cast<CompilationStateImpl*>(compilation_state);
 }
+const CompilationStateImpl* Impl(const CompilationState* compilation_state) {
+  return reinterpret_cast<const CompilationStateImpl*>(compilation_state);
+}
 
 }  // namespace
 
 //////////////////////////////////////////////////////
 // PIMPL implementation of {CompilationState}.
 
+CompilationState::~CompilationState() { Impl(this)->~CompilationStateImpl(); }
+
 void CompilationState::CancelAndWait() { Impl(this)->CancelAndWait(); }
 
 void CompilationState::SetError(uint32_t func_index,
@@ -454,7 +459,8 @@ void CompilationState::SetWireBytesStorage(
   Impl(this)->SetWireBytesStorage(std::move(wire_bytes_storage));
 }
 
-std::shared_ptr<WireBytesStorage> CompilationState::GetWireBytesStorage() {
+std::shared_ptr<WireBytesStorage> CompilationState::GetWireBytesStorage()
+    const {
   return Impl(this)->GetWireBytesStorage();
 }
 
@@ -462,7 +468,7 @@ void CompilationState::AddCallback(CompilationState::callback_t callback) {
   return Impl(this)->AddCallback(std::move(callback));
 }
 
-CompilationState::~CompilationState() { Impl(this)->~CompilationStateImpl(); }
+bool CompilationState::failed() const { return Impl(this)->failed(); }
 
 // static
 std::unique_ptr<CompilationState> CompilationState::New(
@@ -510,12 +516,12 @@ WasmCode* LazyCompileFunction(Isolate* isolate, NativeModule* native_module,
       isolate->counters(),
       Impl(native_module->compilation_state())->detected_features());
 
-  // If there is a pending error, something really went wrong. The module was
-  // verified before starting execution with lazy compilation.
+  // During lazy compilation, we should never get compilation errors. The module
+  // was verified before starting execution with lazy compilation.
   // This might be OOM, but then we cannot continue execution anyway.
   // TODO(clemensh): According to the spec, we can actually skip validation at
   // module creation time, and return a function that always traps here.
-  CHECK(!unit.failed());
+  CHECK(!native_module->compilation_state()->failed());
 
   WasmCode* code = unit.result();
 
@@ -652,7 +658,9 @@ bool FetchAndExecuteCompilationUnit(CompilationEnv* env,
   ExecutionTier tier = unit->tier();
   unit->ExecuteCompilation(env, compilation_state->GetSharedWireBytesStorage(),
                            counters, detected);
-  if (!unit->failed()) compilation_state->ScheduleCodeLogging(unit->result());
+  if (WasmCode* result = unit->result()) {
+    compilation_state->ScheduleCodeLogging(result);
+  }
   compilation_state->ScheduleUnitForFinishing(std::move(unit), tier);
 
   return true;
@@ -678,11 +686,6 @@ void FinishCompilationUnits(CompilationStateImpl* compilation_state) {
         compilation_state->GetNextExecutedUnit();
     if (unit == nullptr) break;
 
-    if (unit->failed()) {
-      compilation_state->Abort();
-      break;
-    }
-
     // Update the compilation state.
     compilation_state->OnFinishedUnit();
   }
@@ -783,16 +786,15 @@ void CompileSequentially(Isolate* isolate, NativeModule* native_module,
   ModuleWireBytes wire_bytes(native_module->wire_bytes());
   const WasmModule* module = native_module->module();
   WasmFeatures detected = kNoWasmFeatures;
-  for (uint32_t i = 0; i < module->functions.size(); ++i) {
-    const WasmFunction& func = module->functions[i];
+  auto* comp_state = Impl(native_module->compilation_state());
+  for (const WasmFunction& func : module->functions) {
     if (func.imported) continue;  // Imports are compiled at instantiation time.
 
     // Compile the function.
-    bool success = WasmCompilationUnit::CompileWasmFunction(
-        isolate, native_module, &detected, &func);
-    if (!success) {
-      thrower->CompileFailed(
-          Impl(native_module->compilation_state())->GetCompileError());
+    WasmCompilationUnit::CompileWasmFunction(isolate, native_module, &detected,
+                                             &func);
+    if (comp_state->failed()) {
+      thrower->CompileFailed(comp_state->GetCompileError());
       break;
     }
   }
@@ -912,8 +914,7 @@ class FinishCompileTask : public CancelableTask {
         break;
       }
 
-      DCHECK_IMPLIES(unit->failed(), compilation_state_->failed());
-      if (unit->failed()) break;
+      if (compilation_state_->failed()) break;
 
       // Update the compilation state, and possibly notify
       // threads waiting for events.

src/wasm/wasm-engine.cc

@@ -226,11 +226,11 @@ std::shared_ptr<StreamingDecoder> WasmEngine::StartStreamingCompilation(
   return job->CreateStreamingDecoder();
 }
 
-bool WasmEngine::CompileFunction(Isolate* isolate, NativeModule* native_module,
+void WasmEngine::CompileFunction(Isolate* isolate, NativeModule* native_module,
                                  uint32_t function_index, ExecutionTier tier) {
   // Note we assume that "one-off" compilations can discard detected features.
   WasmFeatures detected = kNoWasmFeatures;
-  return WasmCompilationUnit::CompileWasmFunction(
+  WasmCompilationUnit::CompileWasmFunction(
       isolate, native_module, &detected,
       &native_module->module()->functions[function_index], tier);
 }

src/wasm/wasm-engine.h

@@ -98,10 +98,10 @@ class V8_EXPORT_PRIVATE WasmEngine {
       Isolate* isolate, const WasmFeatures& enabled, Handle<Context> context,
       std::shared_ptr<CompilationResultResolver> resolver);
 
-  // Compiles the function with the given index at a specific compilation tier
-  // and returns true on success, false otherwise. This is mostly used for
-  // testing to force a function into a specific tier.
-  bool CompileFunction(Isolate* isolate, NativeModule* native_module,
+  // Compiles the function with the given index at a specific compilation tier.
+  // Errors are stored internally in the CompilationState.
+  // This is mostly used for testing to force a function into a specific tier.
+  void CompileFunction(Isolate* isolate, NativeModule* native_module,
                        uint32_t function_index, ExecutionTier tier);
 
   // Exports the sharable parts of the given module object so that they can be

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

@@ -425,8 +425,9 @@ void WasmFunctionCompiler::Build(const byte* start, const byte* end) {
   unit.ExecuteCompilation(
       &env, native_module->compilation_state()->GetWireBytesStorage(),
       isolate()->counters(), &unused_detected_features);
-  CHECK(!unit.failed());
-  if (WasmCode::ShouldBeLogged(isolate())) unit.result()->LogCode(isolate());
+  WasmCode* result = unit.result();
+  DCHECK_NOT_NULL(result);
+  if (WasmCode::ShouldBeLogged(isolate())) result->LogCode(isolate());
 }
 
 WasmFunctionCompiler::WasmFunctionCompiler(Zone* zone, FunctionSig* sig,