[turbofan] Preprocess feedback for ElementAccessInfo computation.

When --concurrent_inlining is on, precompute (during serialization)
the list of receiver maps and the transition matrix of element
accesses.

Bug: v8:7790
Change-Id: I257eaea630f33831ab6600851ccdf297e17e35ca
Reviewed-on: https://chromium-review.googlesource.com/c/1475769
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59863}

src/compiler/access-info.cc

@@ -24,21 +24,6 @@ namespace compiler {
 
 namespace {
 
-bool CanInlineElementAccess(Handle<Map> map) {
-  if (!map->IsJSObjectMap()) return false;
-  if (map->is_access_check_needed()) return false;
-  if (map->has_indexed_interceptor()) return false;
-  ElementsKind const elements_kind = map->elements_kind();
-  if (IsFastElementsKind(elements_kind)) return true;
-  if (IsFixedTypedArrayElementsKind(elements_kind) &&
-      elements_kind != BIGUINT64_ELEMENTS &&
-      elements_kind != BIGINT64_ELEMENTS) {
-    return true;
-  }
-  return false;
-}
-
-
 bool CanInlinePropertyAccess(Handle<Map> map) {
   // We can inline property access to prototypes of all primitives, except
   // the special Oddball ones that have no wrapper counterparts (i.e. Null,
@@ -267,62 +252,49 @@ bool AccessInfoFactory::ComputeElementAccessInfo(
   return true;
 }
 
-typedef std::vector<std::pair<Handle<Map>, Handle<Map>>> TransitionList;
-
-namespace {
-void ProcessFeedbackMaps(Isolate* isolate, MapHandles const& maps,
-                         MapHandles* receiver_maps,
-                         TransitionList* transitions) {
-  DCHECK(receiver_maps->empty());
-  DCHECK(transitions->empty());
-
-  // Collect possible transition targets.
-  MapHandles possible_transition_targets;
-  possible_transition_targets.reserve(maps.size());
-  for (Handle<Map> map : maps) {
-    if (CanInlineElementAccess(map) &&
-        IsFastElementsKind(map->elements_kind()) &&
-        GetInitialFastElementsKind() != map->elements_kind()) {
-      possible_transition_targets.push_back(map);
-    }
-  }
-
-  // Separate the actual receiver maps and the possible transition sources.
-  for (Handle<Map> map : maps) {
-    // Don't generate elements kind transitions from stable maps.
-    Map transition_target = map->is_stable()
-                                ? Map()
-                                : map->FindElementsKindTransitionedMap(
-                                      isolate, possible_transition_targets);
-    if (transition_target.is_null()) {
-      receiver_maps->push_back(map);
+bool AccessInfoFactory::ComputeElementAccessInfos(
+    FeedbackNexus nexus, MapHandles const& maps, AccessMode access_mode,
+    ZoneVector<ElementAccessInfo>* access_infos) const {
+  ProcessedFeedback processed(broker()->zone());
+  ProcessFeedbackMapsForElementAccess(isolate(), maps, &processed);
+
+  if (FLAG_concurrent_inlining) {
+    if (broker()->HasFeedback(nexus)) {
+      // We have already processed the feedback for this nexus during
+      // serialization. Use that data instead of the data computed above.
+      ProcessedFeedback const& preprocessed =
+          broker()->GetOrCreateFeedback(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;
     } else {
-      transitions->emplace_back(map, handle(transition_target, isolate));
+      TRACE_BROKER(broker(),
+                   "ComputeElementAccessInfos: missing preprocessed feedback "
+                       << "(slot " << nexus.slot() << " of "
+                       << Brief(*nexus.vector_handle()) << ").\n");
     }
   }
-}
-}  // namespace
 
-bool AccessInfoFactory::ComputeElementAccessInfos(
-    MapHandles const& maps, AccessMode access_mode,
-    ZoneVector<ElementAccessInfo>* access_infos) const {
   if (access_mode == AccessMode::kLoad) {
     // For polymorphic loads of similar elements kinds (i.e. all tagged or all
     // double), always use the "worst case" code without a transition.  This is
     // 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(maps, &access_info)) {
+    if (ConsolidateElementLoad(processed, &access_info)) {
       access_infos->push_back(access_info);
       return true;
     }
   }
 
-  MapHandles receiver_maps;
-  TransitionList transitions;
-  ProcessFeedbackMaps(isolate(), maps, &receiver_maps, &transitions);
-
-  for (Handle<Map> receiver_map : 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)) {
@@ -330,7 +302,7 @@ bool AccessInfoFactory::ComputeElementAccessInfos(
     }
 
     // Collect the possible transitions for the {receiver_map}.
-    for (auto transition : transitions) {
+    for (auto transition : processed.transitions) {
       if (transition.second.is_identical_to(receiver_map)) {
         access_info.AddTransitionSource(transition.first);
       }
@@ -633,11 +605,15 @@ Maybe<ElementsKind> GeneralizeElementsKind(ElementsKind this_kind,
 }  // namespace
 
 bool AccessInfoFactory::ConsolidateElementLoad(
-    MapHandles const& maps, ElementAccessInfo* access_info) const {
-  CHECK(!maps.empty());
-  InstanceType instance_type = maps.front()->instance_type();
-  ElementsKind elements_kind = maps.front()->elements_kind();
-  for (Handle<Map> map : maps) {
+    ProcessedFeedback const& processed, ElementAccessInfo* access_info) const {
+  CHECK(!processed.receiver_maps.empty());
+
+  // We want to look at each map but the maps are split across
+  // {processed.receiver_maps} and {processed.transitions}.
+
+  InstanceType instance_type = processed.receiver_maps.front()->instance_type();
+  ElementsKind elements_kind = processed.receiver_maps.front()->elements_kind();
+  auto processMap = [&](Handle<Map> map) {
     if (!CanInlineElementAccess(map) || map->instance_type() != instance_type) {
       return false;
     }
@@ -645,7 +621,22 @@ bool AccessInfoFactory::ConsolidateElementLoad(
              .To(&elements_kind)) {
       return false;
     }
+    return true;
+  };
+
+  for (Handle<Map> map : processed.receiver_maps) {
+    if (!processMap(map)) return false;
   }
+
+  MapHandles maps(processed.receiver_maps.begin(),
+                  processed.receiver_maps.end());
+  for (auto& pair : processed.transitions) {
+    if (!processMap(pair.first) || !processMap(pair.second)) return false;
+    maps.push_back(pair.first);
+    maps.push_back(pair.second);
+  }
+  // {maps} may now contain duplicate entries, but that shouldn't matter.
+
   *access_info = ElementAccessInfo(maps, elements_kind);
   return true;
 }

src/compiler/access-info.h

@@ -8,6 +8,7 @@
 #include <iosfwd>
 
 #include "src/compiler/types.h"
+#include "src/feedback-vector.h"
 #include "src/field-index.h"
 #include "src/machine-type.h"
 #include "src/objects.h"
@@ -26,6 +27,7 @@ namespace compiler {
 class CompilationDependencies;
 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.
@@ -150,7 +152,7 @@ class AccessInfoFactory final {
   bool ComputeElementAccessInfo(Handle<Map> map, AccessMode access_mode,
                                 ElementAccessInfo* access_info) const;
   bool ComputeElementAccessInfos(
-      MapHandles const& maps, AccessMode access_mode,
+      FeedbackNexus nexus, MapHandles const& maps, AccessMode access_mode,
       ZoneVector<ElementAccessInfo>* access_infos) const;
 
   bool ComputePropertyAccessInfo(Handle<Map> map, Handle<Name> name,
@@ -164,7 +166,7 @@ class AccessInfoFactory final {
       ZoneVector<PropertyAccessInfo>* access_infos) const;
 
  private:
-  bool ConsolidateElementLoad(MapHandles const& maps,
+  bool ConsolidateElementLoad(ProcessedFeedback const& processed,
                               ElementAccessInfo* access_info) const;
   bool LookupSpecialFieldAccessor(Handle<Map> map, Handle<Name> name,
                                   PropertyAccessInfo* access_info) const;

src/compiler/js-heap-broker.cc

@@ -1572,7 +1572,8 @@ JSHeapBroker::JSHeapBroker(Isolate* isolate, Zone* broker_zone)
       current_zone_(broker_zone),
       refs_(new (zone())
                 RefsMap(kMinimalRefsBucketCount, AddressMatcher(), zone())),
-      array_and_object_prototypes_(zone()) {
+      array_and_object_prototypes_(zone()),
+      feedback_(zone()) {
   // Note that this initialization of the refs_ pointer with the minimal
   // initial capacity is redundant in the normal use case (concurrent
   // compilation enabled, standard objects to be serialized), as the map
@@ -2756,6 +2757,66 @@ void JSBoundFunctionRef::Serialize() {
   data()->AsJSBoundFunction()->Serialize(broker());
 }
 
+bool CanInlineElementAccess(Handle<Map> map) {
+  if (!map->IsJSObjectMap()) return false;
+  if (map->is_access_check_needed()) return false;
+  if (map->has_indexed_interceptor()) return false;
+  ElementsKind const elements_kind = map->elements_kind();
+  if (IsFastElementsKind(elements_kind)) return true;
+  if (IsFixedTypedArrayElementsKind(elements_kind) &&
+      elements_kind != BIGUINT64_ELEMENTS &&
+      elements_kind != BIGINT64_ELEMENTS) {
+    return true;
+  }
+  return false;
+}
+
+bool JSHeapBroker::HasFeedback(FeedbackNexus const& nexus) const {
+  return feedback_.find(nexus) != feedback_.end();
+}
+
+ProcessedFeedback& JSHeapBroker::GetOrCreateFeedback(
+    FeedbackNexus const& nexus) {
+  auto it = feedback_.find(nexus);
+  if (it != feedback_.end()) return it->second;
+  auto insertion = feedback_.insert({nexus, ProcessedFeedback(zone())});
+  CHECK(insertion.second);
+  return insertion.first->second;
+}
+
+void ProcessFeedbackMapsForElementAccess(Isolate* isolate,
+                                         MapHandles const& maps,
+                                         ProcessedFeedback* processed) {
+  DCHECK(processed->receiver_maps.empty());
+  DCHECK(processed->transitions.empty());
+
+  // Collect possible transition targets.
+  MapHandles possible_transition_targets;
+  possible_transition_targets.reserve(maps.size());
+  for (Handle<Map> map : maps) {
+    if (CanInlineElementAccess(map) &&
+        IsFastElementsKind(map->elements_kind()) &&
+        GetInitialFastElementsKind() != map->elements_kind()) {
+      possible_transition_targets.push_back(map);
+    }
+  }
+
+  // Separate the actual receiver maps and the possible transition sources.
+  for (Handle<Map> map : maps) {
+    // Don't generate elements kind transitions from stable maps.
+    Map transition_target = map->is_stable()
+                                ? Map()
+                                : map->FindElementsKindTransitionedMap(
+                                      isolate, possible_transition_targets);
+    if (transition_target.is_null()) {
+      processed->receiver_maps.push_back(map);
+    } else {
+      processed->transitions.emplace_back(map,
+                                          handle(transition_target, isolate));
+    }
+  }
+}
+
 #undef BIMODAL_ACCESSOR
 #undef BIMODAL_ACCESSOR_B
 #undef BIMODAL_ACCESSOR_C

src/compiler/js-heap-broker.h

@@ -8,6 +8,7 @@
 #include "src/base/compiler-specific.h"
 #include "src/base/optional.h"
 #include "src/compiler/refs-map.h"
+#include "src/feedback-vector.h"
 #include "src/function-kind.h"
 #include "src/globals.h"
 #include "src/handles.h"
@@ -615,9 +616,18 @@ class InternalizedStringRef : public StringRef {
   static const uint32_t kNotAnArrayIndex = -1;  // 2^32-1 is not a valid index.
 };
 
+struct ProcessedFeedback {
+  ZoneVector<Handle<Map>> receiver_maps;
+  ZoneVector<std::pair<Handle<Map>, Handle<Map>>> transitions;
+
+  explicit ProcessedFeedback(Zone* zone)
+      : receiver_maps(zone), transitions(zone) {}
+};
+
 class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
  public:
   JSHeapBroker(Isolate* isolate, Zone* broker_zone);
+
   void SetNativeContextRef();
   void SerializeStandardObjects();
 
@@ -644,8 +654,10 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
   // %ObjectPrototype%.
   bool IsArrayOrObjectPrototype(const JSObjectRef& object) const;
 
-  std::ostream& Trace();
+  bool HasFeedback(FeedbackNexus const& nexus) const;
+  ProcessedFeedback& GetOrCreateFeedback(FeedbackNexus const& nexus);
 
+  std::ostream& Trace();
   void IncrementTracingIndentation();
   void DecrementTracingIndentation();
 
@@ -657,6 +669,18 @@ 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().location(), 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_;
@@ -669,6 +693,9 @@ 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>
+      feedback_;
 
   static const size_t kMinimalRefsBucketCount = 8;     // must be power of 2
   static const size_t kInitialRefsBucketCount = 1024;  // must be power of 2
@@ -685,6 +712,13 @@ class Reduction;
 Reduction NoChangeBecauseOfMissingData(JSHeapBroker* broker,
                                        const char* function, int line);
 
+// Miscellaneous definitions that should be moved elsewhere once concurrent
+// compilation is finished.
+bool CanInlineElementAccess(Handle<Map> map);
+void ProcessFeedbackMapsForElementAccess(Isolate* isolate,
+                                         MapHandles const& maps,
+                                         ProcessedFeedback* processed);
+
 #define TRACE_BROKER(broker, x)                               \
   do {                                                        \
     if (FLAG_trace_heap_broker) broker->Trace() << x << '\n'; \

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

@@ -1500,9 +1500,9 @@ Reduction JSNativeContextSpecialization::ReduceElementAccessOnString(
 }
 
 Reduction JSNativeContextSpecialization::ReduceElementAccess(
-    Node* node, Node* index, Node* value, MapHandles const& receiver_maps,
-    AccessMode access_mode, KeyedAccessLoadMode load_mode,
-    KeyedAccessStoreMode store_mode) {
+    Node* node, Node* index, Node* value, FeedbackNexus const& nexus,
+    MapHandles const& receiver_maps, AccessMode access_mode,
+    KeyedAccessLoadMode load_mode, KeyedAccessStoreMode store_mode) {
   DCHECK(node->opcode() == IrOpcode::kJSLoadProperty ||
          node->opcode() == IrOpcode::kJSStoreProperty ||
          node->opcode() == IrOpcode::kJSStoreInArrayLiteral);
@@ -1520,8 +1520,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
   AccessInfoFactory access_info_factory(broker(), dependencies(),
                                         graph()->zone());
   ZoneVector<ElementAccessInfo> access_infos(zone());
-  if (!access_info_factory.ComputeElementAccessInfos(receiver_maps, access_mode,
-                                                     &access_infos)) {
+  if (!access_info_factory.ComputeElementAccessInfos(
+          nexus, receiver_maps, access_mode, &access_infos)) {
     return NoChange();
   }
 
@@ -1856,8 +1856,8 @@ Reduction JSNativeContextSpecialization::ReduceKeyedAccess(
   }
 
   // Try to lower the element access based on the {receiver_maps}.
-  return ReduceElementAccess(node, index, value, receiver_maps, access_mode,
-                             load_mode, store_mode);
+  return ReduceElementAccess(node, index, value, nexus, receiver_maps,
+                             access_mode, load_mode, store_mode);
 }
 
 Reduction JSNativeContextSpecialization::ReduceSoftDeoptimize(
@@ -2513,7 +2513,7 @@ Reduction JSNativeContextSpecialization::ReduceJSStoreInArrayLiteral(
   if (nexus.ic_state() == MEGAMORPHIC) return NoChange();
 
   // Try to lower the element access based on the {receiver_maps}.
-  return ReduceElementAccess(node, index, value, receiver_maps,
+  return ReduceElementAccess(node, index, value, nexus, receiver_maps,
                              AccessMode::kStoreInLiteral, STANDARD_LOAD,
                              store_mode);
 }

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

@@ -92,6 +92,7 @@ class V8_EXPORT_PRIVATE JSNativeContextSpecialization final
   Reduction ReduceJSToObject(Node* node);
 
   Reduction ReduceElementAccess(Node* node, Node* index, Node* value,
+                                FeedbackNexus const& nexus,
                                 MapHandles const& receiver_maps,
                                 AccessMode access_mode,
                                 KeyedAccessLoadMode load_mode,

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

@@ -683,6 +683,59 @@ void SerializerForBackgroundCompilation::VisitConstructWithSpread(
   ProcessCallOrConstruct(callee, new_target, arguments, slot, true);
 }
 
+void SerializerForBackgroundCompilation::ProcessFeedbackForKeyedPropertyAccess(
+    BytecodeArrayIterator* iterator) {
+  interpreter::Bytecode bytecode = iterator->current_bytecode();
+  DCHECK(bytecode == interpreter::Bytecode::kLdaKeyedProperty ||
+         bytecode == interpreter::Bytecode::kStaKeyedProperty ||
+         bytecode == interpreter::Bytecode::kStaInArrayLiteral);
+
+  if (environment()->function().feedback_vector.is_null()) return;
+
+  FeedbackSlot slot = iterator->GetSlotOperand(
+      bytecode == interpreter::Bytecode::kLdaKeyedProperty ? 1 : 2);
+  if (slot.IsInvalid()) return;
+
+  FeedbackNexus nexus(environment()->function().feedback_vector, slot);
+  if (broker()->HasFeedback(nexus)) return;
+
+  Handle<Name> name(nexus.GetName(), broker()->isolate());
+  CHECK_IMPLIES(nexus.GetKeyType() == ELEMENT, name->is_null());
+  if (!name->is_null() || nexus.GetKeyType() == PROPERTY) {
+    CHECK_NE(bytecode, interpreter::Bytecode::kStaInArrayLiteral);
+    return;  // TODO(neis): Support named access.
+  }
+  if (nexus.ic_state() == MEGAMORPHIC) {
+    return;
+  }
+
+  ProcessedFeedback& processed = broker()->GetOrCreateFeedback(nexus);
+  MapHandles maps;
+  nexus.ExtractMaps(&maps);
+  ProcessFeedbackMapsForElementAccess(broker()->isolate(), maps, &processed);
+
+  // TODO(neis): Have something like MapRef::SerializeForElementStore() and call
+  // it for every receiver map in case of an element store.
+}
+
+void SerializerForBackgroundCompilation::VisitLdaKeyedProperty(
+    BytecodeArrayIterator* iterator) {
+  environment()->accumulator_hints().Clear();
+  ProcessFeedbackForKeyedPropertyAccess(iterator);
+}
+
+void SerializerForBackgroundCompilation::VisitStaKeyedProperty(
+    BytecodeArrayIterator* iterator) {
+  environment()->accumulator_hints().Clear();
+  ProcessFeedbackForKeyedPropertyAccess(iterator);
+}
+
+void SerializerForBackgroundCompilation::VisitStaInArrayLiteral(
+    BytecodeArrayIterator* iterator) {
+  environment()->accumulator_hints().Clear();
+  ProcessFeedbackForKeyedPropertyAccess(iterator);
+}
+
 #define DEFINE_CLEAR_ENVIRONMENT(name, ...)             \
   void SerializerForBackgroundCompilation::Visit##name( \
       BytecodeArrayIterator* iterator) {                \

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

@@ -57,7 +57,6 @@ namespace compiler {
   V(LdaGlobalInsideTypeof)          \
   V(LdaImmutableContextSlot)        \
   V(LdaImmutableCurrentContextSlot) \
-  V(LdaKeyedProperty)               \
   V(LdaNamedProperty)               \
   V(LdaNamedPropertyNoFeedback)
 
@@ -122,6 +121,7 @@ namespace compiler {
   V(ExtraWide)                     \
   V(Illegal)                       \
   V(LdaConstant)                   \
+  V(LdaKeyedProperty)              \
   V(LdaNull)                       \
   V(Ldar)                          \
   V(LdaSmi)                        \
@@ -130,6 +130,8 @@ namespace compiler {
   V(Mov)                           \
   V(Return)                        \
   V(StackCheck)                    \
+  V(StaInArrayLiteral)             \
+  V(StaKeyedProperty)              \
   V(Star)                          \
   V(Wide)                          \
   CLEAR_ENVIRONMENT_LIST(V)        \
@@ -241,6 +243,9 @@ class SerializerForBackgroundCompilation {
                            base::Optional<Hints> new_target,
                            const HintsVector& arguments, bool with_spread);
 
+  void ProcessFeedbackForKeyedPropertyAccess(
+      interpreter::BytecodeArrayIterator* iterator);
+
   JSHeapBroker* broker() const { return broker_; }
   Zone* zone() const { return zone_; }
   Environment* environment() const { return environment_; }