[turbofan] Preprocess feedback for global accesses (partially)

Main changes:
- Rename ProcessedFeedback to ElementAccessFeedback and introduce a base class
  with the old name ProcessedFeedback.
- Introduce another kind of ProcessedFeedback, namely GlobalAccessFeedback for
  the LoadGlobal/StoreGlobal IC. It's either a PropertyCell or a script context
  slot.
- Produce such processed feedback in the serializer, when visiting LdaGlobal and
  similar bytecodes.
- Consume it, and disallow heap access, in JSNativeContextSpecialization's
  ReduceJSLoadGlobal and ReduceJSStoreGlobal (for --concurrent-inlining).

Minor changes:
- Introduce a FeedbackSource class (pair of FeedbackVector and FeedbackSlot)
  that is used as the key of the processed feedback hash table. We already have
  two similar classes, FeedbackNexus and VectorSlotPair, but both are unsuitable
  for technical reasons (e.g. FeedbackNexus construction accesses the heap).
  Eventually we should remove VectorSlotPair.
- Processed feedback is now returned as a pointer, which is nullptr if the
  original feedback wasn't interesting (e.g. megamorphic).

The title says "partially" because the CL doesn't yet take into account named
accesses where the receiver happens to be the global proxy.

Bug: v8:7790
Change-Id: I4404d98636b91a8f2d5667115944bae4773a4770
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1518184
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60240}

src/compiler/access-info.cc

@@ -260,38 +260,19 @@ bool AccessInfoFactory::ComputeElementAccessInfo(
 bool AccessInfoFactory::ComputeElementAccessInfos(
     FeedbackNexus nexus, MapHandles const& maps, AccessMode access_mode,
     ZoneVector<ElementAccessInfo>* access_infos) const {
-  ProcessedFeedback processed(broker()->zone());
-
+  ElementAccessFeedback const* processed;
   if (FLAG_concurrent_inlining) {
-    // TODO(neis): When concurrent inlining is ready,
-    // - change the printing below to not look into the heap,
-    // - remove the call to ProcessFeedbackMapsForElementAccess,
-    // - remove the Allow* scopes,
-    AllowCodeDependencyChange dependency_change_;
-    AllowHandleAllocation handle_allocation_;
-    AllowHandleDereference handle_dereference_;
-    AllowHeapAllocation heap_allocation_;
-
-    // We have already processed the feedback for this nexus during
-    // serialization. Use that data! We still process the incoming {maps} (even
-    // though we don't use them) so that we can print a comparison.
-    ProcessFeedbackMapsForElementAccess(broker(), maps, &processed);
-    ProcessedFeedback const& preprocessed = broker()->GetFeedback(nexus);
     TRACE_BROKER(broker(),
                  "ComputeElementAccessInfos: using preprocessed feedback "
                      << "(slot " << nexus.slot() << " of "
-                     << Brief(*nexus.vector_handle()) << "; "
-                     << preprocessed.receiver_maps.size() << "/"
-                     << preprocessed.transitions.size() << " vs "
-                     << processed.receiver_maps.size() << "/"
-                     << processed.transitions.size() << ").\n");
-    processed.receiver_maps = preprocessed.receiver_maps;
-    processed.transitions = preprocessed.transitions;
+                     << "feedback vector handle "
+                     << nexus.vector_handle().address() << ").\n");
+    processed = broker()->GetElementAccessFeedback(FeedbackSource(nexus));
   } else {
-    ProcessFeedbackMapsForElementAccess(broker(), maps, &processed);
+    processed = broker()->ProcessFeedbackMapsForElementAccess(maps);
   }
 
-  if (processed.receiver_maps.empty()) return false;
+  if (processed == nullptr) return false;
 
   if (access_mode == AccessMode::kLoad || access_mode == AccessMode::kHas) {
     // For polymorphic loads of similar elements kinds (i.e. all tagged or all
@@ -299,13 +280,13 @@ bool AccessInfoFactory::ComputeElementAccessInfos(
     // much faster than transitioning the elements to the worst case, trading a
     // TransitionElementsKind for a CheckMaps, avoiding mutation of the array.
     ElementAccessInfo access_info;
-    if (ConsolidateElementLoad(processed, &access_info)) {
+    if (ConsolidateElementLoad(*processed, &access_info)) {
       access_infos->push_back(access_info);
       return true;
     }
   }
 
-  for (Handle<Map> receiver_map : processed.receiver_maps) {
+  for (Handle<Map> receiver_map : processed->receiver_maps) {
     // Compute the element access information.
     ElementAccessInfo access_info;
     if (!ComputeElementAccessInfo(receiver_map, access_mode, &access_info)) {
@@ -313,7 +294,7 @@ bool AccessInfoFactory::ComputeElementAccessInfos(
     }
 
     // Collect the possible transitions for the {receiver_map}.
-    for (auto transition : processed.transitions) {
+    for (auto transition : processed->transitions) {
       if (transition.second.equals(receiver_map)) {
         access_info.AddTransitionSource(transition.first);
       }
@@ -624,8 +605,9 @@ Maybe<ElementsKind> GeneralizeElementsKind(ElementsKind this_kind,
 }  // namespace
 
 bool AccessInfoFactory::ConsolidateElementLoad(
-    ProcessedFeedback const& processed, ElementAccessInfo* access_info) const {
-  ProcessedFeedback::MapIterator it = processed.all_maps(broker());
+    ElementAccessFeedback const& processed,
+    ElementAccessInfo* access_info) const {
+  ElementAccessFeedback::MapIterator it = processed.all_maps(broker());
   MapRef first_map = it.current();
   InstanceType instance_type = first_map.instance_type();
   ElementsKind elements_kind = first_map.elements_kind();

src/compiler/access-info.h

@@ -25,9 +25,9 @@ namespace compiler {
 
 // Forward declarations.
 class CompilationDependencies;
+class ElementAccessFeedback;
 class Type;
 class TypeCache;
-struct ProcessedFeedback;
 
 // Whether we are loading a property or storing to a property.
 // For a store during literal creation, do not walk up the prototype chain.
@@ -166,7 +166,7 @@ class AccessInfoFactory final {
       ZoneVector<PropertyAccessInfo>* access_infos) const;
 
  private:
-  bool ConsolidateElementLoad(ProcessedFeedback const& processed,
+  bool ConsolidateElementLoad(ElementAccessFeedback const& processed,
                               ElementAccessInfo* access_info) const;
   bool LookupSpecialFieldAccessor(Handle<Map> map, Handle<Name> name,
                                   PropertyAccessInfo* access_info) const;

src/compiler/js-heap-broker.h

@@ -21,6 +21,7 @@ namespace v8 {
 namespace internal {
 
 class BytecodeArray;
+class VectorSlotPair;
 class FixedDoubleArray;
 class HeapNumber;
 class InternalizedString;
@@ -186,6 +187,8 @@ class PropertyCellRef : public HeapObjectRef {
   Handle<PropertyCell> object() const;
 
   PropertyDetails property_details() const;
+
+  void Serialize();
   ObjectRef value() const;
 };
 
@@ -622,8 +625,40 @@ class InternalizedStringRef : public StringRef {
   static const uint32_t kNotAnArrayIndex = -1;  // 2^32-1 is not a valid index.
 };
 
-struct ProcessedFeedback {
-  explicit ProcessedFeedback(Zone* zone);
+class ProcessedFeedback : public ZoneObject {
+ public:
+  enum Kind { kElementAccess, kGlobalAccess };
+  Kind kind() const { return kind_; }
+
+ protected:
+  explicit ProcessedFeedback(Kind kind) : kind_(kind) {}
+
+ private:
+  Kind const kind_;
+};
+
+class GlobalAccessFeedback : public ProcessedFeedback {
+ public:
+  explicit GlobalAccessFeedback(PropertyCellRef cell);
+  GlobalAccessFeedback(ContextRef script_context, int slot_index,
+                       bool immutable);
+
+  bool IsPropertyCell() const;
+  PropertyCellRef property_cell() const;
+
+  bool IsScriptContextSlot() const { return !IsPropertyCell(); }
+  ContextRef script_context() const;
+  int slot_index() const;
+  bool immutable() const;
+
+ private:
+  ObjectRef const cell_or_context_;
+  int const index_and_immutable_;
+};
+
+class ElementAccessFeedback : public ProcessedFeedback {
+ public:
+  explicit ElementAccessFeedback(Zone* zone);
 
   // No transition sources appear in {receiver_maps}.
   // All transition targets appear in {receiver_maps}.
@@ -637,12 +672,12 @@ struct ProcessedFeedback {
     MapRef current() const;
 
    private:
-    friend struct ProcessedFeedback;
+    friend class ElementAccessFeedback;
 
-    explicit MapIterator(ProcessedFeedback const& processed,
+    explicit MapIterator(ElementAccessFeedback const& processed,
                          JSHeapBroker* broker);
 
-    ProcessedFeedback const& processed_;
+    ElementAccessFeedback const& processed_;
     JSHeapBroker* const broker_;
     size_t index_ = 0;
   };
@@ -651,6 +686,29 @@ struct ProcessedFeedback {
   MapIterator all_maps(JSHeapBroker* broker) const;
 };
 
+struct FeedbackSource {
+  FeedbackSource(Handle<FeedbackVector> vector_, FeedbackSlot slot_)
+      : vector(vector_), slot(slot_) {}
+  explicit FeedbackSource(FeedbackNexus const& nexus);
+  explicit FeedbackSource(VectorSlotPair const& pair);
+
+  Handle<FeedbackVector> const vector;
+  FeedbackSlot const slot;
+
+  struct Hash {
+    size_t operator()(FeedbackSource const& source) const {
+      return base::hash_combine(source.vector.address(), source.slot);
+    }
+  };
+
+  struct Equal {
+    bool operator()(FeedbackSource const& lhs,
+                    FeedbackSource const& rhs) const {
+      return lhs.vector.equals(rhs.vector) && lhs.slot == rhs.slot;
+    }
+  };
+};
+
 class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
  public:
   JSHeapBroker(Isolate* isolate, Zone* broker_zone);
@@ -681,9 +739,24 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   // %ObjectPrototype%.
   bool IsArrayOrObjectPrototype(const JSObjectRef& object) const;
 
-  ProcessedFeedback& CreateEmptyFeedback(FeedbackNexus const& nexus);
-  bool HasFeedback(FeedbackNexus const& nexus) const;
-  ProcessedFeedback& GetFeedback(FeedbackNexus const& nexus);
+  bool HasFeedback(FeedbackSource const& source) const;
+  // The processed {feedback} can be {nullptr}, indicating that the original
+  // feedback didn't contain information relevant for Turbofan.
+  void SetFeedback(FeedbackSource const& source,
+                   ProcessedFeedback const* feedback);
+  ProcessedFeedback const* GetFeedback(FeedbackSource const& source) const;
+
+  // Convenience wrappers around GetFeedback.
+  ElementAccessFeedback const* GetElementAccessFeedback(
+      FeedbackSource const& source) const;
+  GlobalAccessFeedback const* GetGlobalAccessFeedback(
+      FeedbackSource const& source) const;
+
+  // TODO(neis): Move these into serializer when we're always in the background.
+  ElementAccessFeedback const* ProcessFeedbackMapsForElementAccess(
+      MapHandles const& maps);
+  GlobalAccessFeedback const* ProcessFeedbackForGlobalAccess(
+      FeedbackSource const& source);
 
   std::ostream& Trace();
   void IncrementTracingIndentation();
@@ -697,18 +770,6 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   void SerializeShareableObjects();
   void CollectArrayAndObjectPrototypes();
 
-  struct FeedbackNexusHash {
-    size_t operator()(FeedbackNexus const& nexus) const {
-      return base::hash_combine(nexus.vector_handle().address(), nexus.slot());
-    }
-  };
-  struct FeedbackNexusEqual {
-    bool operator()(FeedbackNexus const& lhs, FeedbackNexus const& rhs) const {
-      return lhs.vector_handle().equals(rhs.vector_handle()) &&
-             lhs.slot() == rhs.slot();
-    }
-  };
-
   Isolate* const isolate_;
   Zone* const broker_zone_;
   Zone* current_zone_;
@@ -721,8 +782,8 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   StdoutStream trace_out_;
   unsigned trace_indentation_ = 0;
   PerIsolateCompilerCache* compiler_cache_;
-  ZoneUnorderedMap<FeedbackNexus, ProcessedFeedback, FeedbackNexusHash,
-                   FeedbackNexusEqual>
+  ZoneUnorderedMap<FeedbackSource, ProcessedFeedback const*,
+                   FeedbackSource::Hash, FeedbackSource::Equal>
       feedback_;
 
   static const size_t kMinimalRefsBucketCount = 8;     // must be power of 2
@@ -743,9 +804,6 @@ Reduction NoChangeBecauseOfMissingData(JSHeapBroker* broker,
 // Miscellaneous definitions that should be moved elsewhere once concurrent
 // compilation is finished.
 bool CanInlineElementAccess(MapRef const& map);
-void ProcessFeedbackMapsForElementAccess(JSHeapBroker* broker,
-                                         MapHandles const& maps,
-                                         ProcessedFeedback* processed);
 
 #define TRACE_BROKER(broker, x)                               \
   do {                                                        \

src/compiler/js-native-context-specialization.h

@@ -116,7 +116,8 @@ class V8_EXPORT_PRIVATE JSNativeContextSpecialization final
                                Node* index = nullptr);
   Reduction ReduceGlobalAccess(Node* node, Node* receiver, Node* value,
                                Handle<Name> name, AccessMode access_mode,
-                               Node* index, Handle<PropertyCell> property_cell);
+                               Node* index,
+                               PropertyCellRef const& property_cell);
   Reduction ReduceKeyedLoadFromHeapConstant(Node* node, Node* index,
                                             FeedbackNexus const& nexus,
                                             AccessMode access_mode,

src/compiler/serializer-for-background-compilation.cc

@@ -683,13 +683,59 @@ void SerializerForBackgroundCompilation::VisitConstructWithSpread(
   ProcessCallOrConstruct(callee, new_target, arguments, slot, true);
 }
 
+void SerializerForBackgroundCompilation::ProcessFeedbackForGlobalAccess(
+    FeedbackSlot slot) {
+  if (slot.IsInvalid()) return;
+  if (environment()->function().feedback_vector.is_null()) return;
+  FeedbackSource source(environment()->function().feedback_vector, slot);
+  if (!broker()->HasFeedback(source)) {
+    broker()->SetFeedback(source,
+                          broker()->ProcessFeedbackForGlobalAccess(source));
+  }
+  // TODO(neis, mvstanton): In the case of an immutable script context slot, we
+  // must also serialize that slot such that ContextRef::get can retrieve the
+  // value.
+}
+
+void SerializerForBackgroundCompilation::VisitLdaGlobal(
+    BytecodeArrayIterator* iterator) {
+  FeedbackSlot slot = iterator->GetSlotOperand(1);
+  ProcessFeedbackForGlobalAccess(slot);
+  // TODO(neis, mvstanton): In the case of an immutable script context slot, add
+  // the value as constant hint here and below
+  environment()->accumulator_hints().Clear();
+}
+
+void SerializerForBackgroundCompilation::VisitLdaGlobalInsideTypeof(
+    BytecodeArrayIterator* iterator) {
+  VisitLdaGlobal(iterator);
+}
+
+void SerializerForBackgroundCompilation::VisitLdaLookupGlobalSlot(
+    BytecodeArrayIterator* iterator) {
+  VisitLdaGlobal(iterator);
+}
+
+void SerializerForBackgroundCompilation::VisitLdaLookupGlobalSlotInsideTypeof(
+    BytecodeArrayIterator* iterator) {
+  VisitLdaGlobal(iterator);
+}
+
+void SerializerForBackgroundCompilation::VisitStaGlobal(
+    BytecodeArrayIterator* iterator) {
+  FeedbackSlot slot = iterator->GetSlotOperand(1);
+  ProcessFeedbackForGlobalAccess(slot);
+}
+
+// Note: We never use the same feeedback slot for multiple access modes.
 void SerializerForBackgroundCompilation::ProcessFeedbackForKeyedPropertyAccess(
     FeedbackSlot slot, AccessMode mode) {
   if (slot.IsInvalid()) return;
   if (environment()->function().feedback_vector.is_null()) return;
 
   FeedbackNexus nexus(environment()->function().feedback_vector, slot);
-  if (broker()->HasFeedback(nexus)) return;
+  FeedbackSource source(nexus);
+  if (broker()->HasFeedback(source)) return;
 
   if (nexus.ic_state() == MEGAMORPHIC) return;
 
@@ -702,10 +748,12 @@ void SerializerForBackgroundCompilation::ProcessFeedbackForKeyedPropertyAccess(
 
   MapHandles maps;
   nexus.ExtractMaps(&maps);
-  ProcessedFeedback& processed = broker()->CreateEmptyFeedback(nexus);
-  ProcessFeedbackMapsForElementAccess(broker(), maps, &processed);
+  ElementAccessFeedback const* processed =
+      broker()->ProcessFeedbackMapsForElementAccess(maps);
+  broker()->SetFeedback(source, processed);
+  if (processed == nullptr) return;
 
-  for (ProcessedFeedback::MapIterator it = processed.all_maps(broker());
+  for (ElementAccessFeedback::MapIterator it = processed->all_maps(broker());
        !it.done(); it.advance()) {
     switch (mode) {
       case AccessMode::kHas:

src/compiler/serializer-for-background-compilation.h

@@ -55,8 +55,6 @@ namespace compiler {
   V(CreateUnmappedArguments)        \
   V(LdaContextSlot)                 \
   V(LdaCurrentContextSlot)          \
-  V(LdaGlobal)                      \
-  V(LdaGlobalInsideTypeof)          \
   V(LdaImmutableContextSlot)        \
   V(LdaImmutableCurrentContextSlot) \
   V(LdaNamedProperty)               \
@@ -104,42 +102,47 @@ namespace compiler {
   V(ThrowSuperNotCalledIfHole)   \
   V(ThrowSuperAlreadyCalledIfNotHole)
 
-#define SUPPORTED_BYTECODE_LIST(V) \
-  V(CallAnyReceiver)               \
-  V(CallNoFeedback)                \
-  V(CallProperty)                  \
-  V(CallProperty0)                 \
-  V(CallProperty1)                 \
-  V(CallProperty2)                 \
-  V(CallUndefinedReceiver)         \
-  V(CallUndefinedReceiver0)        \
-  V(CallUndefinedReceiver1)        \
-  V(CallUndefinedReceiver2)        \
-  V(CallWithSpread)                \
-  V(Construct)                     \
-  V(ConstructWithSpread)           \
-  V(CreateClosure)                 \
-  V(ExtraWide)                     \
-  V(Illegal)                       \
-  V(LdaConstant)                   \
-  V(LdaKeyedProperty)              \
-  V(LdaNull)                       \
-  V(Ldar)                          \
-  V(LdaSmi)                        \
-  V(LdaUndefined)                  \
-  V(LdaZero)                       \
-  V(Mov)                           \
-  V(Return)                        \
-  V(StackCheck)                    \
-  V(StaInArrayLiteral)             \
-  V(StaKeyedProperty)              \
-  V(Star)                          \
-  V(TestIn)                        \
-  V(Wide)                          \
-  CLEAR_ENVIRONMENT_LIST(V)        \
-  CLEAR_ACCUMULATOR_LIST(V)        \
-  CONDITIONAL_JUMPS_LIST(V)        \
-  UNCONDITIONAL_JUMPS_LIST(V)      \
+#define SUPPORTED_BYTECODE_LIST(V)   \
+  V(CallAnyReceiver)                 \
+  V(CallNoFeedback)                  \
+  V(CallProperty)                    \
+  V(CallProperty0)                   \
+  V(CallProperty1)                   \
+  V(CallProperty2)                   \
+  V(CallUndefinedReceiver)           \
+  V(CallUndefinedReceiver0)          \
+  V(CallUndefinedReceiver1)          \
+  V(CallUndefinedReceiver2)          \
+  V(CallWithSpread)                  \
+  V(Construct)                       \
+  V(ConstructWithSpread)             \
+  V(CreateClosure)                   \
+  V(ExtraWide)                       \
+  V(Illegal)                         \
+  V(LdaConstant)                     \
+  V(LdaGlobal)                       \
+  V(LdaGlobalInsideTypeof)           \
+  V(LdaKeyedProperty)                \
+  V(LdaLookupGlobalSlot)             \
+  V(LdaLookupGlobalSlotInsideTypeof) \
+  V(LdaNull)                         \
+  V(Ldar)                            \
+  V(LdaSmi)                          \
+  V(LdaUndefined)                    \
+  V(LdaZero)                         \
+  V(Mov)                             \
+  V(Return)                          \
+  V(StackCheck)                      \
+  V(StaGlobal)                       \
+  V(StaInArrayLiteral)               \
+  V(StaKeyedProperty)                \
+  V(Star)                            \
+  V(TestIn)                          \
+  V(Wide)                            \
+  CLEAR_ENVIRONMENT_LIST(V)          \
+  CLEAR_ACCUMULATOR_LIST(V)          \
+  CONDITIONAL_JUMPS_LIST(V)          \
+  UNCONDITIONAL_JUMPS_LIST(V)        \
   INGORED_BYTECODE_LIST(V)
 
 class JSHeapBroker;
@@ -245,6 +248,7 @@ class SerializerForBackgroundCompilation {
                            base::Optional<Hints> new_target,
                            const HintsVector& arguments, bool with_spread);
 
+  void ProcessFeedbackForGlobalAccess(FeedbackSlot slot);
   void ProcessFeedbackForKeyedPropertyAccess(FeedbackSlot slot,
                                              AccessMode mode);