[wasm] Merge PublishCode into AddCode

A small refactoring to reduce complexity. It also moves the call to
{RegisterTrapHandlerData} out of the mutex to reduce the time in this
critical section.

R=mstarzinger@chromium.org

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

src/wasm/function-compiler.cc

@@ -203,8 +203,6 @@ WasmCode* WasmCompilationUnit::Publish(WasmCompilationResult result,
       func_index_, result.code_desc, result.frame_slot_count,
       result.tagged_parameter_slots, 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;
 }
 

src/wasm/wasm-code-manager.cc

@@ -620,7 +620,7 @@ WasmCode* NativeModule::AddCode(
   const int handler_table_offset =
       desc.handler_table_size == 0 ? 0 : desc.handler_table_offset;
 
-  WasmCode* ret = AddOwnedCode(
+  WasmCode* code = AddOwnedCode(
       index, {desc.buffer, static_cast<size_t>(desc.instr_size)}, stack_slots,
       tagged_parameter_slots, safepoint_table_offset, handler_table_offset,
       desc.constant_pool_offset, desc.code_comments_offset, desc.instr_size,
@@ -628,12 +628,12 @@ WasmCode* NativeModule::AddCode(
       std::move(source_pos_table), kind, tier);
 
   // Apply the relocation delta by iterating over the RelocInfo.
-  intptr_t delta = ret->instructions().start() - desc.buffer;
+  intptr_t delta = code->instructions().start() - desc.buffer;
   int mode_mask = RelocInfo::kApplyMask |
                   RelocInfo::ModeMask(RelocInfo::WASM_CALL) |
                   RelocInfo::ModeMask(RelocInfo::WASM_STUB_CALL);
-  for (RelocIterator it(ret->instructions(), ret->reloc_info(),
-                        ret->constant_pool(), mode_mask);
+  for (RelocIterator it(code->instructions(), code->reloc_info(),
+                        code->constant_pool(), mode_mask);
        !it.done(); it.next()) {
     RelocInfo::Mode mode = it.rinfo()->rmode();
     if (RelocInfo::IsWasmCall(mode)) {
@@ -653,11 +653,23 @@ WasmCode* NativeModule::AddCode(
 
   // Flush the i-cache here instead of in AddOwnedCode, to include the changes
   // made while iterating over the RelocInfo above.
-  FlushInstructionCache(ret->instructions().start(),
-                        ret->instructions().size());
-  ret->MaybePrint();
-  ret->Validate();
-  return ret;
+  FlushInstructionCache(code->instructions().start(),
+                        code->instructions().size());
+  code->MaybePrint();
+  code->Validate();
+
+  if (!code->protected_instructions_.is_empty()) {
+    code->RegisterTrapHandlerData();
+  }
+
+  base::MutexGuard lock(&allocation_mutex_);
+  // Skip publishing code if there is an active redirection to the interpreter
+  // for the given function index, in order to preserve the redirection.
+  if (!code->IsAnonymous() && !has_interpreter_redirection(code->index())) {
+    InstallCode(code);
+  }
+
+  return code;
 }
 
 WasmCode* NativeModule::AddDeserializedCode(
@@ -685,18 +697,6 @@ WasmCode* NativeModule::AddDeserializedCode(
   return code;
 }
 
-void NativeModule::PublishCode(WasmCode* code) {
-  base::MutexGuard lock(&allocation_mutex_);
-  // Skip publishing code if there is an active redirection to the interpreter
-  // for the given function index, in order to preserve the redirection.
-  if (has_interpreter_redirection(code->index())) return;
-
-  if (!code->protected_instructions_.is_empty()) {
-    code->RegisterTrapHandlerData();
-  }
-  InstallCode(code);
-}
-
 void NativeModule::PublishInterpreterEntry(WasmCode* code,
                                            uint32_t func_index) {
   code->index_ = func_index;

src/wasm/wasm-code-manager.h

@@ -231,6 +231,8 @@ class V8_EXPORT_PRIVATE NativeModule final {
 
   // {AddCode} is thread safe w.r.t. other calls to {AddCode} or methods adding
   // code below, i.e. it can be called concurrently from background threads.
+  // {AddCode} also makes the code available to the system by entering it into
+  // the code table and patching the jump table.
   WasmCode* AddCode(uint32_t index, const CodeDesc& desc, uint32_t stack_slots,
                     uint32_t tagged_parameter_slots,
                     OwnedVector<trap_handler::ProtectedInstructionData>
@@ -261,11 +263,6 @@ class V8_EXPORT_PRIVATE NativeModule final {
   // other WasmCode so that runtime stub ids can be resolved during relocation.
   void SetRuntimeStubs(Isolate* isolate);
 
-  // Makes the code available to the system (by entering it into the code table
-  // and patching the jump table). Callers have to take care not to race with
-  // threads executing the old code.
-  void PublishCode(WasmCode* code);
-
   // Switch a function to an interpreter entry wrapper. When adding interpreter
   // wrappers, we do not insert them in the code_table, however, we let them
   // self-identify as the {index} function.

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

@@ -3473,10 +3473,9 @@ TEST(Liftoff_tier_up) {
     memcpy(buffer.get(), sub_code->instructions().start(), sub_size);
     desc.buffer = buffer.get();
     desc.instr_size = static_cast<int>(sub_size);
-    WasmCode* code = native_module->AddCode(
-        add.function_index(), desc, 0, 0, {}, OwnedVector<byte>(),
-        WasmCode::kFunction, WasmCode::kOther);
-    native_module->PublishCode(code);
+    native_module->AddCode(add.function_index(), desc, 0, 0, {},
+                           OwnedVector<byte>(), WasmCode::kFunction,
+                           WasmCode::kOther);
 
     // Second run should now execute {sub}.
     CHECK_EQ(4, r.Call(11, 7));