[maglev] Towards concurrent compilation

Add LocalIsolate use, broker()->GetFeedbackForPropertyAccess, and
generating persistent/canonical handles to fix maglev concurrent
compilation.

Bug: v8:7700
Change-Id: Ifd1156c72710047b5f2930837a04709419b23bc3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3578546
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79918}

src/maglev/maglev-compilation-info.cc

@@ -69,11 +69,17 @@ MaglevCompilationInfo::MaglevCompilationInfo(Isolate* isolate,
       zone()->New<compiler::CompilationDependencies>(broker(), zone());
   USE(deps);  // The deps register themselves in the heap broker.
 
+  // Heap broker initialization may already use IsPendingAllocation.
+  isolate->heap()->PublishPendingAllocations();
+
   broker()->SetTargetNativeContextRef(
       handle(function->native_context(), isolate));
   broker()->InitializeAndStartSerializing();
   broker()->StopSerializing();
 
+  // Serialization may have allocated.
+  isolate->heap()->PublishPendingAllocations();
+
   toplevel_compilation_unit_ =
       MaglevCompilationUnit::New(zone(), this, function);
 }

src/maglev/maglev-compilation-unit.h

@@ -30,6 +30,7 @@ class MaglevCompilationUnit : public ZoneObject {
   MaglevCompilationInfo* info() const { return info_; }
   compiler::JSHeapBroker* broker() const;
   Isolate* isolate() const;
+  LocalIsolate* local_isolate() const;
   Zone* zone() const;
   int register_count() const { return register_count_; }
   int parameter_count() const { return parameter_count_; }

src/maglev/maglev-compiler.cc

@@ -138,8 +138,9 @@ class UseMarkingProcessor {
 };
 
 // static
-void MaglevCompiler::Compile(MaglevCompilationUnit* toplevel_compilation_unit) {
-  MaglevCompiler compiler(toplevel_compilation_unit);
+void MaglevCompiler::Compile(LocalIsolate* local_isolate,
+                             MaglevCompilationUnit* toplevel_compilation_unit) {
+  MaglevCompiler compiler(local_isolate, toplevel_compilation_unit);
   compiler.Compile();
 }
 
@@ -153,7 +154,7 @@ void MaglevCompiler::Compile() {
         new MaglevGraphLabeller());
   }
 
-  MaglevGraphBuilder graph_builder(toplevel_compilation_unit_);
+  MaglevGraphBuilder graph_builder(local_isolate(), toplevel_compilation_unit_);
 
   graph_builder.Build();
 

src/maglev/maglev-compiler.h

@@ -24,7 +24,8 @@ class Graph;
 class MaglevCompiler {
  public:
   // May be called from any thread.
-  static void Compile(MaglevCompilationUnit* toplevel_compilation_unit);
+  static void Compile(LocalIsolate* local_isolate,
+                      MaglevCompilationUnit* toplevel_compilation_unit);
 
   // Called on the main thread after Compile has completed.
   // TODO(v8:7700): Move this to a different class?
@@ -32,8 +33,10 @@ class MaglevCompiler {
       MaglevCompilationUnit* toplevel_compilation_unit);
 
  private:
-  explicit MaglevCompiler(MaglevCompilationUnit* toplevel_compilation_unit)
-      : toplevel_compilation_unit_(toplevel_compilation_unit) {}
+  explicit MaglevCompiler(LocalIsolate* local_isolate,
+                          MaglevCompilationUnit* toplevel_compilation_unit)
+      : local_isolate_(local_isolate),
+        toplevel_compilation_unit_(toplevel_compilation_unit) {}
 
   void Compile();
 
@@ -41,8 +44,9 @@ class MaglevCompiler {
     return toplevel_compilation_unit_->broker();
   }
   Zone* zone() { return toplevel_compilation_unit_->zone(); }
-  Isolate* isolate() { return toplevel_compilation_unit_->isolate(); }
+  LocalIsolate* local_isolate() { return local_isolate_; }
 
+  LocalIsolate* const local_isolate_;
   MaglevCompilationUnit* const toplevel_compilation_unit_;
 };
 

src/maglev/maglev-concurrent-dispatcher.cc

@@ -101,7 +101,8 @@ CompilationJob::Status MaglevCompilationJob::PrepareJobImpl(Isolate* isolate) {
 CompilationJob::Status MaglevCompilationJob::ExecuteJobImpl(
     RuntimeCallStats* stats, LocalIsolate* local_isolate) {
   LocalIsolateScope scope{info(), local_isolate};
-  maglev::MaglevCompiler::Compile(info()->toplevel_compilation_unit());
+  maglev::MaglevCompiler::Compile(local_isolate,
+                                  info()->toplevel_compilation_unit());
   // TODO(v8:7700): Actual return codes.
   return CompilationJob::SUCCEEDED;
 }

src/maglev/maglev-graph-builder.cc

@@ -21,8 +21,22 @@ namespace internal {
 
 namespace maglev {
 
-MaglevGraphBuilder::MaglevGraphBuilder(MaglevCompilationUnit* compilation_unit)
-    : compilation_unit_(compilation_unit),
+namespace {
+
+int LoadSimpleFieldHandler(FieldIndex field_index) {
+  int config = LoadHandler::KindBits::encode(LoadHandler::Kind::kField) |
+               LoadHandler::IsInobjectBits::encode(field_index.is_inobject()) |
+               LoadHandler::IsDoubleBits::encode(field_index.is_double()) |
+               LoadHandler::FieldIndexBits::encode(field_index.index());
+  return config;
+}
+
+}  // namespace
+
+MaglevGraphBuilder::MaglevGraphBuilder(LocalIsolate* local_isolate,
+                                       MaglevCompilationUnit* compilation_unit)
+    : local_isolate_(local_isolate),
+      compilation_unit_(compilation_unit),
       iterator_(bytecode().object()),
       jump_targets_(zone()->NewArray<BasicBlockRef>(bytecode().length())),
       // Overallocate merge_states_ by one to allow always looking up the
@@ -160,7 +174,7 @@ void MaglevGraphBuilder::VisitUnaryOperation() {
 
 template <Operation kOperation>
 void MaglevGraphBuilder::VisitBinaryOperation() {
-  FeedbackNexus nexus = feedback_nexus(1);
+  FeedbackNexus nexus = FeedbackNexusForOperand(1);
 
   if (nexus.ic_state() == InlineCacheState::MONOMORPHIC) {
     if (nexus.kind() == FeedbackSlotKind::kBinaryOp) {
@@ -185,7 +199,7 @@ void MaglevGraphBuilder::VisitBinaryOperation() {
 
 template <Operation kOperation>
 void MaglevGraphBuilder::VisitBinarySmiOperation() {
-  FeedbackNexus nexus = feedback_nexus(1);
+  FeedbackNexus nexus = FeedbackNexusForOperand(1);
 
   if (nexus.ic_state() == InlineCacheState::MONOMORPHIC) {
     if (nexus.kind() == FeedbackSlotKind::kBinaryOp) {
@@ -253,11 +267,9 @@ void MaglevGraphBuilder::VisitLdaCurrentContextSlot() {
   // TODO(leszeks): Passing a LoadHandler to LoadField here is a bit of
   // a hack, maybe we should have a LoadRawOffset or similar.
   SetAccumulator(AddNewNode<LoadField>(
-      {context}, LoadHandler::LoadField(
-                     isolate(), FieldIndex::ForInObjectOffset(
-                                    Context::OffsetOfElementAt(slot_index),
-                                    FieldIndex::kTagged))
-                     ->value()));
+      {context},
+      LoadSimpleFieldHandler(FieldIndex::ForInObjectOffset(
+          Context::OffsetOfElementAt(slot_index), FieldIndex::kTagged))));
 }
 void MaglevGraphBuilder::VisitLdaImmutableCurrentContextSlot() {
   // TODO(leszeks): Consider context specialising.
@@ -333,10 +345,8 @@ void MaglevGraphBuilder::BuildPropertyCellAccess(
   // a hack, maybe we should have a LoadRawOffset or similar.
   SetAccumulator(AddNewNode<LoadField>(
       {property_cell_node},
-      LoadHandler::LoadField(
-          isolate(), FieldIndex::ForInObjectOffset(PropertyCell::kValueOffset,
-                                                   FieldIndex::kTagged))
-          ->value()));
+      LoadSimpleFieldHandler(FieldIndex::ForInObjectOffset(
+          PropertyCell::kValueOffset, FieldIndex::kTagged))));
 }
 
 void MaglevGraphBuilder::VisitLdaGlobal() {
@@ -381,35 +391,50 @@ MAGLEV_UNIMPLEMENTED_BYTECODE(StaLookupSlot)
 void MaglevGraphBuilder::VisitGetNamedProperty() {
   // GetNamedProperty <object> <name_index> <slot>
   ValueNode* object = LoadRegister(0);
-  FeedbackSlot slot_index = GetSlotOperand(2);
-  FeedbackNexus nexus(feedback().object(), slot_index);
-
-  if (nexus.ic_state() == InlineCacheState::UNINITIALIZED) {
-    AddNewNode<EagerDeopt>({});
-    return;
-  } else if (nexus.ic_state() == InlineCacheState::MONOMORPHIC) {
-    std::vector<MapAndHandler> maps_and_handlers;
-    nexus.ExtractMapsAndHandlers(&maps_and_handlers);
-    DCHECK_EQ(maps_and_handlers.size(), 1);
-    MapAndHandler& map_and_handler = maps_and_handlers[0];
-    if (map_and_handler.second->IsSmi()) {
-      int handler = map_and_handler.second->ToSmi().value();
-      LoadHandler::Kind kind = LoadHandler::KindBits::decode(handler);
-      if (kind == LoadHandler::Kind::kField &&
-          !LoadHandler::IsWasmStructBits::decode(handler)) {
-        AddNewNode<CheckMaps>({object},
-                              MakeRef(broker(), map_and_handler.first));
-        SetAccumulator(AddNewNode<LoadField>({object}, handler));
-        return;
+  compiler::NameRef name = GetRefOperand<Name>(1);
+  FeedbackSlot slot = GetSlotOperand(2);
+  compiler::FeedbackSource feedback_source{feedback(), slot};
+
+  const compiler::ProcessedFeedback& processed_feedback =
+      broker()->GetFeedbackForPropertyAccess(feedback_source,
+                                             compiler::AccessMode::kLoad, name);
+
+  switch (processed_feedback.kind()) {
+    case compiler::ProcessedFeedback::kInsufficient:
+      AddNewNode<EagerDeopt>({});
+      return;
+
+    case compiler::ProcessedFeedback::kNamedAccess: {
+      const compiler::NamedAccessFeedback& named_feedback =
+          processed_feedback.AsNamedAccess();
+      if (named_feedback.maps().size() == 1) {
+        // Monomorphic load, check the handler.
+        // TODO(leszeks): Make GetFeedbackForPropertyAccess read the handler.
+        MaybeObjectHandle handler =
+            FeedbackNexusForSlot(slot).FindHandlerForMap(
+                named_feedback.maps()[0].object());
+        if (!handler.is_null() && handler->IsSmi()) {
+          // Smi handler, emit a map check and LoadField.
+          int smi_handler = handler->ToSmi().value();
+          LoadHandler::Kind kind = LoadHandler::KindBits::decode(smi_handler);
+          if (kind == LoadHandler::Kind::kField &&
+              !LoadHandler::IsWasmStructBits::decode(smi_handler)) {
+            AddNewNode<CheckMaps>({object}, named_feedback.maps()[0]);
+            SetAccumulator(AddNewNode<LoadField>({object}, smi_handler));
+            return;
+          }
+        }
       }
-    }
+    } break;
+
+    default:
+      break;
   }
 
+  // Create a generic load in the fallthrough.
   ValueNode* context = GetContext();
-  compiler::NameRef name = GetRefOperand<Name>(1);
-  SetAccumulator(AddNewNode<LoadNamedGeneric>(
-      {context, object}, name,
-      compiler::FeedbackSource{feedback(), slot_index}));
+  SetAccumulator(
+      AddNewNode<LoadNamedGeneric>({context, object}, name, feedback_source));
 }
 
 MAGLEV_UNIMPLEMENTED_BYTECODE(GetNamedPropertyFromSuper)
@@ -420,27 +445,43 @@ MAGLEV_UNIMPLEMENTED_BYTECODE(StaModuleVariable)
 void MaglevGraphBuilder::VisitSetNamedProperty() {
   // SetNamedProperty <object> <name_index> <slot>
   ValueNode* object = LoadRegister(0);
-  FeedbackNexus nexus = feedback_nexus(2);
-
-  if (nexus.ic_state() == InlineCacheState::UNINITIALIZED) {
-    AddNewNode<EagerDeopt>({});
-    return;
-  } else if (nexus.ic_state() == InlineCacheState::MONOMORPHIC) {
-    std::vector<MapAndHandler> maps_and_handlers;
-    nexus.ExtractMapsAndHandlers(&maps_and_handlers);
-    DCHECK_EQ(maps_and_handlers.size(), 1);
-    MapAndHandler& map_and_handler = maps_and_handlers[0];
-    if (map_and_handler.second->IsSmi()) {
-      int handler = map_and_handler.second->ToSmi().value();
-      StoreHandler::Kind kind = StoreHandler::KindBits::decode(handler);
-      if (kind == StoreHandler::Kind::kField) {
-        AddNewNode<CheckMaps>({object},
-                              MakeRef(broker(), map_and_handler.first));
-        ValueNode* value = GetAccumulator();
-        AddNewNode<StoreField>({object, value}, handler);
-        return;
+  compiler::NameRef name = GetRefOperand<Name>(1);
+  FeedbackSlot slot = GetSlotOperand(2);
+  compiler::FeedbackSource feedback_source{feedback(), slot};
+
+  const compiler::ProcessedFeedback& processed_feedback =
+      broker()->GetFeedbackForPropertyAccess(
+          feedback_source, compiler::AccessMode::kStore, name);
+
+  switch (processed_feedback.kind()) {
+    case compiler::ProcessedFeedback::kInsufficient:
+      AddNewNode<EagerDeopt>({});
+      return;
+
+    case compiler::ProcessedFeedback::kNamedAccess: {
+      const compiler::NamedAccessFeedback& named_feedback =
+          processed_feedback.AsNamedAccess();
+      if (named_feedback.maps().size() == 1) {
+        // Monomorphic store, check the handler.
+        // TODO(leszeks): Make GetFeedbackForPropertyAccess read the handler.
+        MaybeObjectHandle handler =
+            FeedbackNexusForSlot(slot).FindHandlerForMap(
+                named_feedback.maps()[0].object());
+        if (!handler.is_null() && handler->IsSmi()) {
+          int smi_handler = handler->ToSmi().value();
+          StoreHandler::Kind kind = StoreHandler::KindBits::decode(smi_handler);
+          if (kind == StoreHandler::Kind::kField) {
+            AddNewNode<CheckMaps>({object}, named_feedback.maps()[0]);
+            ValueNode* value = GetAccumulator();
+            AddNewNode<StoreField>({object, value}, smi_handler);
+            return;
+          }
+        }
       }
-    }
+    } break;
+
+    default:
+      break;
   }
 
   // TODO(victorgomes): Generic store.

src/maglev/maglev-graph-builder.h

@@ -25,7 +25,8 @@ namespace maglev {
 
 class MaglevGraphBuilder {
  public:
-  explicit MaglevGraphBuilder(MaglevCompilationUnit* compilation_unit);
+  explicit MaglevGraphBuilder(LocalIsolate* local_isolate,
+                              MaglevCompilationUnit* compilation_unit);
 
   void Build() {
     for (iterator_.Reset(); !iterator_.done(); iterator_.Advance()) {
@@ -182,9 +183,12 @@ class MaglevGraphBuilder {
   template <class T, typename = std::enable_if_t<
                          std::is_convertible<T*, Object*>::value>>
   typename compiler::ref_traits<T>::ref_type GetRefOperand(int operand_index) {
-    return MakeRef(broker(),
-                   Handle<T>::cast(iterator_.GetConstantForIndexOperand(
-                       operand_index, isolate())));
+    // The BytecodeArray itself was fetched by using a barrier so all reads
+    // from the constant pool are safe.
+    return MakeRefAssumeMemoryFence(
+        broker(), broker()->CanonicalPersistentHandle(
+                      Handle<T>::cast(iterator_.GetConstantForIndexOperand(
+                          operand_index, local_isolate()))));
   }
 
   ValueNode* GetConstant(const compiler::ObjectRef& ref) {
@@ -394,10 +398,14 @@ class MaglevGraphBuilder {
   const compiler::FeedbackVectorRef& feedback() const {
     return compilation_unit_->feedback();
   }
-  const FeedbackNexus feedback_nexus(int slot_operand_index) const {
-    // TODO(leszeks): Use JSHeapBroker here.
+  const FeedbackNexus FeedbackNexusForOperand(int slot_operand_index) const {
     return FeedbackNexus(feedback().object(),
-                         GetSlotOperand(slot_operand_index));
+                         GetSlotOperand(slot_operand_index),
+                         broker()->feedback_nexus_config());
+  }
+  const FeedbackNexus FeedbackNexusForSlot(FeedbackSlot slot) const {
+    return FeedbackNexus(feedback().object(), slot,
+                         broker()->feedback_nexus_config());
   }
   const compiler::BytecodeArrayRef& bytecode() const {
     return compilation_unit_->bytecode();
@@ -405,7 +413,7 @@ class MaglevGraphBuilder {
   const compiler::BytecodeAnalysis& bytecode_analysis() const {
     return compilation_unit_->bytecode_analysis();
   }
-  Isolate* isolate() const { return compilation_unit_->isolate(); }
+  LocalIsolate* local_isolate() const { return local_isolate_; }
   Zone* zone() const { return compilation_unit_->zone(); }
   int parameter_count() const { return compilation_unit_->parameter_count(); }
   int register_count() const { return compilation_unit_->register_count(); }
@@ -416,6 +424,7 @@ class MaglevGraphBuilder {
     return compilation_unit_->graph_labeller();
   }
 
+  LocalIsolate* const local_isolate_;
   MaglevCompilationUnit* const compilation_unit_;
   interpreter::BytecodeArrayIterator iterator_;
   uint32_t* predecessors_;

src/maglev/maglev-ir.cc

@@ -482,7 +482,7 @@ void CheckMaps::GenerateCode(MaglevCodeGenState* code_gen_state,
   __ Cmp(map_tmp, map().object());
 
   // TODO(leszeks): Encode as a bit on CheckMaps.
-  if (map().object()->is_migration_target()) {
+  if (map().is_migration_target()) {
     JumpToDeferredIf(
         not_equal, code_gen_state,
         [](MaglevCodeGenState* code_gen_state, Label* return_label,

src/maglev/maglev.cc

@@ -19,7 +19,7 @@ MaybeHandle<CodeT> Maglev::Compile(Isolate* isolate,
   // TODO(v8:7700): Support exceptions in maglev. We currently bail if exception
   // handler table is non-empty.
   if (unit->bytecode().handler_table_size() > 0) return {};
-  maglev::MaglevCompiler::Compile(unit);
+  maglev::MaglevCompiler::Compile(isolate->main_thread_local_isolate(), unit);
   return maglev::MaglevCompiler::GenerateCode(unit);
 }