[turbofan] Refactor ComputePropertyAccessInfo for readability

...mainly by moving two big chunks into separate methods.

Change-Id: I048a8d27c28bc171559190a915616efbebdfe6cc
Reviewed-on: https://chromium-review.googlesource.com/c/1462957Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59550}

src/compiler/access-info.cc

@@ -261,9 +261,9 @@ AccessInfoFactory::AccessInfoFactory(JSHeapBroker* broker,
   DCHECK(native_context->IsNativeContext());
 }
 
-
 bool AccessInfoFactory::ComputeElementAccessInfo(
-    Handle<Map> map, AccessMode access_mode, ElementAccessInfo* access_info) {
+    Handle<Map> map, AccessMode access_mode,
+    ElementAccessInfo* access_info) const {
   // Check if it is safe to inline element access for the {map}.
   if (!CanInlineElementAccess(map)) return false;
   ElementsKind const elements_kind = map->elements_kind();
@@ -273,7 +273,7 @@ bool AccessInfoFactory::ComputeElementAccessInfo(
 
 bool AccessInfoFactory::ComputeElementAccessInfos(
     MapHandles const& maps, AccessMode access_mode,
-    ZoneVector<ElementAccessInfo>* access_infos) {
+    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
@@ -335,24 +335,130 @@ bool AccessInfoFactory::ComputeElementAccessInfos(
   return true;
 }
 
+bool AccessInfoFactory::ComputeDataFieldAccessInfo(
+    Handle<Map> receiver_map, Handle<Map> map, MaybeHandle<JSObject> holder,
+    int number, AccessMode access_mode, PropertyAccessInfo* access_info) const {
+  DCHECK_NE(number, DescriptorArray::kNotFound);
+  Handle<DescriptorArray> descriptors(map->instance_descriptors(), isolate());
+  PropertyDetails const details = descriptors->GetDetails(number);
+  int index = descriptors->GetFieldIndex(number);
+  Representation details_representation = details.representation();
+  FieldIndex field_index =
+      FieldIndex::ForPropertyIndex(*map, index, details_representation);
+  Type field_type = Type::NonInternal();
+  MachineRepresentation field_representation = MachineRepresentation::kTagged;
+  MaybeHandle<Map> field_map;
+  if (details_representation.IsSmi()) {
+    field_type = Type::SignedSmall();
+    field_representation = MachineRepresentation::kTaggedSigned;
+  } else if (details_representation.IsDouble()) {
+    field_type = type_cache_->kFloat64;
+    field_representation = MachineRepresentation::kFloat64;
+  } else if (details_representation.IsHeapObject()) {
+    // Extract the field type from the property details (make sure its
+    // representation is TaggedPointer to reflect the heap object case).
+    field_representation = MachineRepresentation::kTaggedPointer;
+    Handle<FieldType> descriptors_field_type(descriptors->GetFieldType(number),
+                                             isolate());
+    if (descriptors_field_type->IsNone()) {
+      // Store is not safe if the field type was cleared.
+      if (access_mode == AccessMode::kStore) return false;
+
+      // The field type was cleared by the GC, so we don't know anything
+      // about the contents now.
+    } else if (descriptors_field_type->IsClass()) {
+      MapRef map_ref(broker(), map);
+      map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
+      dependencies()->DependOnFieldType(map_ref, number);
+      // Remember the field map, and try to infer a useful type.
+      Handle<Map> map(descriptors_field_type->AsClass(), isolate());
+      field_type = Type::For(MapRef(broker(), map));
+      field_map = MaybeHandle<Map>(map);
+    }
+  }
+  *access_info = PropertyAccessInfo::DataField(
+      details.constness(), MapHandles{receiver_map}, field_index,
+      field_representation, field_type, field_map, holder);
+  return true;
+}
+
+bool AccessInfoFactory::ComputeAccessorDescriptorAccessInfo(
+    Handle<Map> receiver_map, Handle<Name> name, Handle<Map> map,
+    MaybeHandle<JSObject> holder, int number, AccessMode access_mode,
+    PropertyAccessInfo* access_info) const {
+  DCHECK_NE(number, DescriptorArray::kNotFound);
+  Handle<DescriptorArray> descriptors(map->instance_descriptors(), isolate());
+  SLOW_DCHECK(number == descriptors->Search(*name, *map));
+  if (map->instance_type() == JS_MODULE_NAMESPACE_TYPE) {
+    DCHECK(map->is_prototype_map());
+    Handle<PrototypeInfo> proto_info(PrototypeInfo::cast(map->prototype_info()),
+                                     isolate());
+    Handle<JSModuleNamespace> module_namespace(
+        JSModuleNamespace::cast(proto_info->module_namespace()), isolate());
+    Handle<Cell> cell(
+        Cell::cast(module_namespace->module()->exports()->Lookup(
+            ReadOnlyRoots(isolate()), name, Smi::ToInt(name->GetHash()))),
+        isolate());
+    if (cell->value()->IsTheHole(isolate())) {
+      // This module has not been fully initialized yet.
+      return false;
+    }
+    *access_info =
+        PropertyAccessInfo::ModuleExport(MapHandles{receiver_map}, cell);
+    return true;
+  }
+  Handle<Object> accessors(descriptors->GetStrongValue(number), isolate());
+  if (!accessors->IsAccessorPair()) return false;
+  Handle<Object> accessor(access_mode == AccessMode::kLoad
+                              ? Handle<AccessorPair>::cast(accessors)->getter()
+                              : Handle<AccessorPair>::cast(accessors)->setter(),
+                          isolate());
+  if (!accessor->IsJSFunction()) {
+    CallOptimization optimization(isolate(), accessor);
+    if (!optimization.is_simple_api_call()) return false;
+    if (optimization.IsCrossContextLazyAccessorPair(*native_context_, *map)) {
+      return false;
+    }
+
+    CallOptimization::HolderLookup lookup;
+    holder = optimization.LookupHolderOfExpectedType(receiver_map, &lookup);
+    if (lookup == CallOptimization::kHolderNotFound) return false;
+    DCHECK_IMPLIES(lookup == CallOptimization::kHolderIsReceiver,
+                   holder.is_null());
+    DCHECK_IMPLIES(lookup == CallOptimization::kHolderFound, !holder.is_null());
+    if (V8_UNLIKELY(FLAG_runtime_stats)) return false;
+  }
+  if (access_mode == AccessMode::kLoad) {
+    Handle<Name> cached_property_name;
+    if (FunctionTemplateInfo::TryGetCachedPropertyName(isolate(), accessor)
+            .ToHandle(&cached_property_name)) {
+      if (ComputePropertyAccessInfo(map, cached_property_name, access_mode,
+                                    access_info)) {
+        return true;
+      }
+    }
+  }
+  *access_info = PropertyAccessInfo::AccessorConstant(MapHandles{receiver_map},
+                                                      accessor, holder);
+  return true;
+}
 
 bool AccessInfoFactory::ComputePropertyAccessInfo(
     Handle<Map> map, Handle<Name> name, AccessMode access_mode,
-    PropertyAccessInfo* access_info) {
+    PropertyAccessInfo* access_info) const {
   CHECK(name->IsUniqueName());
 
   // Check if it is safe to inline property access for the {map}.
   if (!CanInlinePropertyAccess(map)) return false;
 
-  // Compute the receiver type.
-  Handle<Map> receiver_map = map;
-
   // We support fast inline cases for certain JSObject getters.
   if (access_mode == AccessMode::kLoad &&
       LookupSpecialFieldAccessor(map, name, access_info)) {
     return true;
   }
 
+  // Remember the receiver map. We use {map} as loop variable.
+  Handle<Map> receiver_map = map;
   MaybeHandle<JSObject> holder;
   do {
     // Lookup the named property on the {map}.
@@ -366,62 +472,23 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
         if (details.IsReadOnly()) {
           return false;
         }
-        // Check for store to data property on a prototype.
         if (details.kind() == kData && !holder.is_null()) {
-          // Store to property not found on the receiver but on a prototype, we
-          // need to transition to a new data property.
-          // Implemented according to ES6 section 9.1.9 [[Set]] (P, V, Receiver)
+          // This is a store to a property not found on the receiver but on a
+          // prototype. According to ES6 section 9.1.9 [[Set]], we need to
+          // create a new data property on the receiver. We can still optimize
+          // if such a transition already exists.
           return LookupTransition(receiver_map, name, holder, access_info);
         }
       }
       if (details.location() == kField) {
         if (details.kind() == kData) {
-          int index = descriptors->GetFieldIndex(number);
-          Representation details_representation = details.representation();
-          FieldIndex field_index =
-              FieldIndex::ForPropertyIndex(*map, index, details_representation);
-          Type field_type = Type::NonInternal();
-          MachineRepresentation field_representation =
-              MachineRepresentation::kTagged;
-          MaybeHandle<Map> field_map;
-          if (details_representation.IsSmi()) {
-            field_type = Type::SignedSmall();
-            field_representation = MachineRepresentation::kTaggedSigned;
-          } else if (details_representation.IsDouble()) {
-            field_type = type_cache_->kFloat64;
-            field_representation = MachineRepresentation::kFloat64;
-          } else if (details_representation.IsHeapObject()) {
-            // Extract the field type from the property details (make sure its
-            // representation is TaggedPointer to reflect the heap object case).
-            field_representation = MachineRepresentation::kTaggedPointer;
-            Handle<FieldType> descriptors_field_type(
-                descriptors->GetFieldType(number), isolate());
-            if (descriptors_field_type->IsNone()) {
-              // Store is not safe if the field type was cleared.
-              if (access_mode == AccessMode::kStore) return false;
-
-              // The field type was cleared by the GC, so we don't know anything
-              // about the contents now.
-            } else if (descriptors_field_type->IsClass()) {
-              MapRef map_ref(broker(), map);
-              map_ref.SerializeOwnDescriptors();  // TODO(neis): Remove later.
-              dependencies()->DependOnFieldType(map_ref, number);
-              // Remember the field map, and try to infer a useful type.
-              Handle<Map> map(descriptors_field_type->AsClass(), isolate());
-              field_type = Type::For(MapRef(broker(), map));
-              field_map = MaybeHandle<Map>(map);
-            }
-          }
-          *access_info = PropertyAccessInfo::DataField(
-              details.constness(), MapHandles{receiver_map}, field_index,
-              field_representation, field_type, field_map, holder);
-          return true;
+          return ComputeDataFieldAccessInfo(receiver_map, map, holder, number,
+                                            access_mode, access_info);
         } else {
           DCHECK_EQ(kAccessor, details.kind());
           // TODO(turbofan): Add support for general accessors?
           return false;
         }
-
       } else {
         DCHECK_EQ(kDescriptor, details.location());
         if (details.kind() == kData) {
@@ -432,71 +499,16 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
           return true;
         } else {
           DCHECK_EQ(kAccessor, details.kind());
-          if (map->instance_type() == JS_MODULE_NAMESPACE_TYPE) {
-            DCHECK(map->is_prototype_map());
-            Handle<PrototypeInfo> proto_info(
-                PrototypeInfo::cast(map->prototype_info()), isolate());
-            Handle<JSModuleNamespace> module_namespace(
-                JSModuleNamespace::cast(proto_info->module_namespace()),
-                isolate());
-            Handle<Cell> cell(
-                Cell::cast(module_namespace->module()->exports()->Lookup(
-                    ReadOnlyRoots(isolate()), name,
-                    Smi::ToInt(name->GetHash()))),
-                isolate());
-            if (cell->value()->IsTheHole(isolate())) {
-              // This module has not been fully initialized yet.
-              return false;
-            }
-            *access_info = PropertyAccessInfo::ModuleExport(
-                MapHandles{receiver_map}, cell);
-            return true;
-          }
-          Handle<Object> accessors(descriptors->GetStrongValue(number),
-                                   isolate());
-          if (!accessors->IsAccessorPair()) return false;
-          Handle<Object> accessor(
-              access_mode == AccessMode::kLoad
-                  ? Handle<AccessorPair>::cast(accessors)->getter()
-                  : Handle<AccessorPair>::cast(accessors)->setter(),
-              isolate());
-          if (!accessor->IsJSFunction()) {
-            CallOptimization optimization(isolate(), accessor);
-            if (!optimization.is_simple_api_call()) return false;
-            if (optimization.IsCrossContextLazyAccessorPair(*native_context_,
-                                                            *map)) {
-              return false;
-            }
-
-            CallOptimization::HolderLookup lookup;
-            holder =
-                optimization.LookupHolderOfExpectedType(receiver_map, &lookup);
-            if (lookup == CallOptimization::kHolderNotFound) return false;
-            DCHECK_IMPLIES(lookup == CallOptimization::kHolderIsReceiver,
-                           holder.is_null());
-            DCHECK_IMPLIES(lookup == CallOptimization::kHolderFound,
-                           !holder.is_null());
-            if (V8_UNLIKELY(FLAG_runtime_stats)) return false;
-          }
-          if (access_mode == AccessMode::kLoad) {
-            Handle<Name> cached_property_name;
-            if (FunctionTemplateInfo::TryGetCachedPropertyName(isolate(),
-                                                               accessor)
-                    .ToHandle(&cached_property_name)) {
-              if (ComputePropertyAccessInfo(map, cached_property_name,
-                                            access_mode, access_info)) {
-                return true;
-              }
-            }
-          }
-          *access_info = PropertyAccessInfo::AccessorConstant(
-              MapHandles{receiver_map}, accessor, holder);
-          return true;
+          return ComputeAccessorDescriptorAccessInfo(receiver_map, name, map,
+                                                     holder, number,
+                                                     access_mode, access_info);
         }
       }
       UNREACHABLE();
     }
 
+    // The property wasn't found on {map}. Look on the prototype if appropriate.
+
     // Don't search on the prototype chain for special indices in case of
     // integer indexed exotic objects (see ES6 section 9.4.5).
     if (map->IsJSTypedArrayMap() && name->IsString() &&
@@ -504,7 +516,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
       return false;
     }
 
-    // Don't search on the prototype when storing in literals
+    // Don't search on the prototype when storing in literals.
     if (access_mode == AccessMode::kStoreInLiteral) {
       return LookupTransition(receiver_map, name, holder, access_info);
     }
@@ -552,7 +564,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
 
 bool AccessInfoFactory::ComputePropertyAccessInfo(
     MapHandles const& maps, Handle<Name> name, AccessMode access_mode,
-    PropertyAccessInfo* access_info) {
+    PropertyAccessInfo* access_info) const {
   ZoneVector<PropertyAccessInfo> access_infos(zone());
   if (ComputePropertyAccessInfos(maps, name, access_mode, &access_infos) &&
       access_infos.size() == 1) {
@@ -564,7 +576,7 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
 
 bool AccessInfoFactory::ComputePropertyAccessInfos(
     MapHandles const& maps, Handle<Name> name, AccessMode access_mode,
-    ZoneVector<PropertyAccessInfo>* access_infos) {
+    ZoneVector<PropertyAccessInfo>* access_infos) const {
   ZoneVector<PropertyAccessInfo> infos(zone());
   infos.reserve(maps.size());
   for (Handle<Map> map : maps) {
@@ -612,8 +624,8 @@ Maybe<ElementsKind> GeneralizeElementsKind(ElementsKind this_kind,
 
 }  // namespace
 
-bool AccessInfoFactory::ConsolidateElementLoad(MapHandles const& maps,
-                                               ElementAccessInfo* access_info) {
+bool AccessInfoFactory::ConsolidateElementLoad(
+    MapHandles const& maps, ElementAccessInfo* access_info) const {
   MapHandles updated_maps;
   updated_maps.reserve(maps.size());
   for (Handle<Map> map : maps) {
@@ -639,7 +651,7 @@ bool AccessInfoFactory::ConsolidateElementLoad(MapHandles const& maps,
 }
 
 bool AccessInfoFactory::LookupSpecialFieldAccessor(
-    Handle<Map> map, Handle<Name> name, PropertyAccessInfo* access_info) {
+    Handle<Map> map, Handle<Name> name, PropertyAccessInfo* access_info) const {
   // Check for String::length field accessor.
   if (map->IsStringMap()) {
     if (Name::Equals(isolate(), name, factory()->length_string())) {
@@ -679,10 +691,9 @@ bool AccessInfoFactory::LookupSpecialFieldAccessor(
   return false;
 }
 
-
-bool AccessInfoFactory::LookupTransition(Handle<Map> map, Handle<Name> name,
-                                         MaybeHandle<JSObject> holder,
-                                         PropertyAccessInfo* access_info) {
+bool AccessInfoFactory::LookupTransition(
+    Handle<Map> map, Handle<Name> name, MaybeHandle<JSObject> holder,
+    PropertyAccessInfo* access_info) const {
   // Check if the {map} has a data transition with the given {name}.
   Map transition =
       TransitionsAccessor(isolate(), map).SearchTransition(*name, kData, NONE);
@@ -738,7 +749,6 @@ bool AccessInfoFactory::LookupTransition(Handle<Map> map, Handle<Name> name,
   return true;
 }
 
-
 Factory* AccessInfoFactory::factory() const { return isolate()->factory(); }
 
 }  // namespace compiler

src/compiler/access-info.h

@@ -145,31 +145,40 @@ class PropertyAccessInfo final {
 class AccessInfoFactory final {
  public:
   AccessInfoFactory(JSHeapBroker* broker, CompilationDependencies* dependencies,
-
                     Handle<Context> native_context, Zone* zone);
 
   bool ComputeElementAccessInfo(Handle<Map> map, AccessMode access_mode,
-                                ElementAccessInfo* access_info);
-  bool ComputeElementAccessInfos(MapHandles const& maps, AccessMode access_mode,
-                                 ZoneVector<ElementAccessInfo>* access_infos);
+                                ElementAccessInfo* access_info) const;
+  bool ComputeElementAccessInfos(
+      MapHandles const& maps, AccessMode access_mode,
+      ZoneVector<ElementAccessInfo>* access_infos) const;
+
   bool ComputePropertyAccessInfo(Handle<Map> map, Handle<Name> name,
                                  AccessMode access_mode,
-                                 PropertyAccessInfo* access_info);
+                                 PropertyAccessInfo* access_info) const;
   bool ComputePropertyAccessInfo(MapHandles const& maps, Handle<Name> name,
                                  AccessMode access_mode,
-                                 PropertyAccessInfo* access_info);
-  bool ComputePropertyAccessInfos(MapHandles const& maps, Handle<Name> name,
-                                  AccessMode access_mode,
-                                  ZoneVector<PropertyAccessInfo>* access_infos);
+                                 PropertyAccessInfo* access_info) const;
+  bool ComputePropertyAccessInfos(
+      MapHandles const& maps, Handle<Name> name, AccessMode access_mode,
+      ZoneVector<PropertyAccessInfo>* access_infos) const;
 
  private:
   bool ConsolidateElementLoad(MapHandles const& maps,
-                              ElementAccessInfo* access_info);
+                              ElementAccessInfo* access_info) const;
   bool LookupSpecialFieldAccessor(Handle<Map> map, Handle<Name> name,
-                                  PropertyAccessInfo* access_info);
+                                  PropertyAccessInfo* access_info) const;
   bool LookupTransition(Handle<Map> map, Handle<Name> name,
                         MaybeHandle<JSObject> holder,
-                        PropertyAccessInfo* access_info);
+                        PropertyAccessInfo* access_info) const;
+  bool ComputeDataFieldAccessInfo(Handle<Map> receiver_map, Handle<Map> map,
+                                  MaybeHandle<JSObject> holder, int number,
+                                  AccessMode access_mode,
+                                  PropertyAccessInfo* access_info) const;
+  bool ComputeAccessorDescriptorAccessInfo(
+      Handle<Map> receiver_map, Handle<Name> name, Handle<Map> map,
+      MaybeHandle<JSObject> holder, int number, AccessMode access_mode,
+      PropertyAccessInfo* access_info) const;
 
   CompilationDependencies* dependencies() const { return dependencies_; }
   JSHeapBroker* broker() const { return broker_; }
@@ -182,7 +191,7 @@ class AccessInfoFactory final {
   CompilationDependencies* const dependencies_;
   Handle<Context> const native_context_;
   Isolate* const isolate_;
-  TypeCache const* type_cache_;
+  TypeCache const* const type_cache_;
   Zone* const zone_;
 
   DISALLOW_COPY_AND_ASSIGN(AccessInfoFactory);