Revert "[dict-proto] TF support for constants in dictionary mode protos, pt. 3"

This reverts commit b1883dc3.

Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Mac64%20GC%20Stress/17269/overview

Original change's description:
> [dict-proto] TF support for constants in dictionary mode protos, pt. 3
>
> This CL is part of a  series that implements Turbofan support for
> property accesses satisfying the following conditions:
> 1. The holder is a dictionary mode object.
> 2. The holder is a prototype.
> 3. The access is a load.
>
> This feature will only be enabled if the build flag
> v8_dict_property_const_tracking is set.
>
> This particular CL implements support for the case that the property
> in question is an accesor, meaning that the given PropertyAccessInfo
> has kind kAccessorDictionaryProtoConstant.
>
> Bug: v8:11248
> Change-Id: Id082107edd45fa91a3f1d96aa9df345a60f46917
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2720300
> Reviewed-by: Georg Neis <neis@chromium.org>
> Commit-Queue: Frank Emrich <emrich@google.com>
> Cr-Commit-Position: refs/heads/master@{#73607}

Bug: v8:11248
Change-Id: Id753354a5ccddd1a05ecf9aec3267f152ef713c5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2780299Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73612}

src/compiler/access-info.cc

@@ -135,7 +135,7 @@ PropertyAccessInfo PropertyAccessInfo::FastAccessorConstant(
     Zone* zone, Handle<Map> receiver_map, Handle<Object> constant,
     MaybeHandle<JSObject> holder) {
   return PropertyAccessInfo(zone, kFastAccessorConstant, holder, constant,
-                            MaybeHandle<Name>(), {{receiver_map}, zone});
+                            {{receiver_map}, zone});
 }
 
 // static
@@ -143,7 +143,7 @@ PropertyAccessInfo PropertyAccessInfo::ModuleExport(Zone* zone,
                                                     Handle<Map> receiver_map,
                                                     Handle<Cell> cell) {
   return PropertyAccessInfo(zone, kModuleExport, MaybeHandle<JSObject>(), cell,
-                            MaybeHandle<Name>{}, {{receiver_map}, zone});
+                            {{receiver_map}, zone});
 }
 
 // static
@@ -164,9 +164,9 @@ PropertyAccessInfo PropertyAccessInfo::DictionaryProtoDataConstant(
 // static
 PropertyAccessInfo PropertyAccessInfo::DictionaryProtoAccessorConstant(
     Zone* zone, Handle<Map> receiver_map, MaybeHandle<JSObject> holder,
-    Handle<Object> constant, Handle<Name> property_name) {
+    Handle<Object> constant) {
   return PropertyAccessInfo(zone, kDictionaryProtoAccessorConstant, holder,
-                            constant, property_name, {{receiver_map}, zone});
+                            constant, {{receiver_map}, zone});
 }
 
 // static
@@ -204,8 +204,7 @@ PropertyAccessInfo::PropertyAccessInfo(
 
 PropertyAccessInfo::PropertyAccessInfo(
     Zone* zone, Kind kind, MaybeHandle<JSObject> holder,
-    Handle<Object> constant, MaybeHandle<Name> property_name,
-    ZoneVector<Handle<Map>>&& lookup_start_object_maps)
+    Handle<Object> constant, ZoneVector<Handle<Map>>&& lookup_start_object_maps)
     : kind_(kind),
       lookup_start_object_maps_(lookup_start_object_maps),
       constant_(constant),
@@ -213,11 +212,7 @@ PropertyAccessInfo::PropertyAccessInfo(
       unrecorded_dependencies_(zone),
       field_representation_(Representation::None()),
       field_type_(Type::Any()),
-      dictionary_index_(InternalIndex::NotFound()),
-      name_(property_name) {
-  DCHECK_IMPLIES(kind == kDictionaryProtoAccessorConstant,
-                 !property_name.is_null());
-}
+      dictionary_index_(InternalIndex::NotFound()) {}
 PropertyAccessInfo::PropertyAccessInfo(
     Kind kind, MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map,
     FieldIndex field_index, Representation field_representation,
@@ -325,7 +320,6 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
       return false;
     }
 
-    case kDictionaryProtoAccessorConstant:
     case kFastAccessorConstant: {
       // Check if we actually access the same constant.
       if (this->constant_.address() == that->constant_.address()) {
@@ -364,6 +358,10 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that,
     }
     case kModuleExport:
       return false;
+
+    case kDictionaryProtoAccessorConstant:
+      // TODO(v8:11248) Dealt with in follow-up CLs.
+      UNREACHABLE();
   }
 }
 
@@ -517,57 +515,54 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
   UNREACHABLE();
 }
 
-namespace {
-using AccessorsObjectGetter = std::function<Handle<Object>()>;
-
-PropertyAccessInfo AccessorAccessInfoHelper(
-    Isolate* isolate, Zone* zone, JSHeapBroker* broker,
-    const AccessInfoFactory* ai_factory, Handle<Map> receiver_map,
-    Handle<Name> name, Handle<Map> map, MaybeHandle<JSObject> holder,
-    AccessMode access_mode, AccessorsObjectGetter get_accessors) {
+PropertyAccessInfo AccessInfoFactory::ComputeAccessorDescriptorAccessInfo(
+    Handle<Map> receiver_map, Handle<Name> name, Handle<Map> map,
+    MaybeHandle<JSObject> holder, InternalIndex descriptor,
+    AccessMode access_mode) const {
+  DCHECK(descriptor.is_found());
+  Handle<DescriptorArray> descriptors(map->instance_descriptors(isolate()),
+                                      isolate());
+  SLOW_DCHECK(descriptor == 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);
+                                     isolate());
     Handle<JSModuleNamespace> module_namespace(
-        JSModuleNamespace::cast(proto_info->module_namespace()), isolate);
+        JSModuleNamespace::cast(proto_info->module_namespace()), isolate());
     Handle<Cell> cell(Cell::cast(module_namespace->module().exports().Lookup(
-                          isolate, name, Smi::ToInt(name->GetHash()))),
-                      isolate);
-    if (cell->value().IsTheHole(isolate)) {
+                          isolate(), name, Smi::ToInt(name->GetHash()))),
+                      isolate());
+    if (cell->value().IsTheHole(isolate())) {
       // This module has not been fully initialized yet.
-      return PropertyAccessInfo::Invalid(zone);
+      return PropertyAccessInfo::Invalid(zone());
     }
-    return PropertyAccessInfo::ModuleExport(zone, receiver_map, cell);
+    return PropertyAccessInfo::ModuleExport(zone(), receiver_map, cell);
   }
   if (access_mode == AccessMode::kHas) {
-    // kHas is not supported for dictionary mode objects.
-    DCHECK(!map->is_dictionary_map());
-
     // HasProperty checks don't call getter/setters, existence is sufficient.
-    return PropertyAccessInfo::FastAccessorConstant(zone, receiver_map,
+    return PropertyAccessInfo::FastAccessorConstant(zone(), receiver_map,
                                                     Handle<Object>(), holder);
   }
-  Handle<Object> accessors = get_accessors();
+  Handle<Object> accessors(descriptors->GetStrongValue(descriptor), isolate());
   if (!accessors->IsAccessorPair()) {
-    return PropertyAccessInfo::Invalid(zone);
+    return PropertyAccessInfo::Invalid(zone());
   }
   Handle<Object> accessor(access_mode == AccessMode::kLoad
                               ? Handle<AccessorPair>::cast(accessors)->getter()
                               : Handle<AccessorPair>::cast(accessors)->setter(),
-                          isolate);
+                          isolate());
   if (!accessor->IsJSFunction()) {
-    CallOptimization optimization(isolate, accessor);
+    CallOptimization optimization(isolate(), accessor);
     if (!optimization.is_simple_api_call() ||
         optimization.IsCrossContextLazyAccessorPair(
-            *broker->target_native_context().object(), *map)) {
-      return PropertyAccessInfo::Invalid(zone);
+            *broker()->target_native_context().object(), *map)) {
+      return PropertyAccessInfo::Invalid(zone());
     }
 
     CallOptimization::HolderLookup lookup;
     holder = optimization.LookupHolderOfExpectedType(receiver_map, &lookup);
     if (lookup == CallOptimization::kHolderNotFound) {
-      return PropertyAccessInfo::Invalid(zone);
+      return PropertyAccessInfo::Invalid(zone());
     }
     DCHECK_IMPLIES(lookup == CallOptimization::kHolderIsReceiver,
                    holder.is_null());
@@ -575,39 +570,15 @@ PropertyAccessInfo AccessorAccessInfoHelper(
   }
   if (access_mode == AccessMode::kLoad) {
     Handle<Name> cached_property_name;
-    if (FunctionTemplateInfo::TryGetCachedPropertyName(isolate, accessor)
+    if (FunctionTemplateInfo::TryGetCachedPropertyName(isolate(), accessor)
             .ToHandle(&cached_property_name)) {
-      PropertyAccessInfo access_info = ai_factory->ComputePropertyAccessInfo(
-          map, cached_property_name, access_mode);
+      PropertyAccessInfo access_info =
+          ComputePropertyAccessInfo(map, cached_property_name, access_mode);
       if (!access_info.IsInvalid()) return access_info;
     }
   }
-  if (map->is_dictionary_map()) {
-    return PropertyAccessInfo::DictionaryProtoAccessorConstant(
-        zone, receiver_map, holder, accessor, name);
-  } else {
-    return PropertyAccessInfo::FastAccessorConstant(zone, receiver_map,
-                                                    accessor, holder);
-  }
-}
-
-}  // namespace
-
-PropertyAccessInfo AccessInfoFactory::ComputeAccessorDescriptorAccessInfo(
-    Handle<Map> receiver_map, Handle<Name> name, Handle<Map> holder_map,
-    MaybeHandle<JSObject> holder, InternalIndex descriptor,
-    AccessMode access_mode) const {
-  DCHECK(descriptor.is_found());
-  Handle<DescriptorArray> descriptors(
-      holder_map->instance_descriptors(kRelaxedLoad), isolate());
-  SLOW_DCHECK(descriptor == descriptors->Search(*name, *holder_map));
-
-  auto get_accessors = [&]() {
-    return handle(descriptors->GetStrongValue(descriptor), isolate());
-  };
-  return AccessorAccessInfoHelper(isolate(), zone(), broker(), this,
-                                  receiver_map, name, holder_map, holder,
-                                  access_mode, get_accessors);
+  return PropertyAccessInfo::FastAccessorConstant(zone(), receiver_map,
+                                                  accessor, holder);
 }
 
 PropertyAccessInfo AccessInfoFactory::ComputeDictionaryProtoAccessInfo(
@@ -628,13 +599,8 @@ PropertyAccessInfo AccessInfoFactory::ComputeDictionaryProtoAccessInfo(
         zone(), receiver_map, holder, dictionary_index, name);
   }
 
-  auto get_accessors = [&]() {
-    return JSObject::DictionaryPropertyAt(holder, dictionary_index);
-  };
-  Handle<Map> holder_map = handle(holder->map(), isolate());
-  return AccessorAccessInfoHelper(isolate(), zone(), broker(), this,
-                                  receiver_map, name, holder_map, holder,
-                                  access_mode, get_accessors);
+  // TODO(v8:11248) Support for accessors is implemented a in follow-up CL.
+  return PropertyAccessInfo::Invalid(zone());
 }
 
 MinimorphicLoadPropertyAccessInfo AccessInfoFactory::ComputePropertyAccessInfo(

src/compiler/access-info.h

@@ -105,7 +105,7 @@ class PropertyAccessInfo final {
       InternalIndex dict_index, Handle<Name> name);
   static PropertyAccessInfo DictionaryProtoAccessorConstant(
       Zone* zone, Handle<Map> receiver_map, MaybeHandle<JSObject> holder,
-      Handle<Object> constant, Handle<Name> name);
+      Handle<Object> constant);
 
   bool Merge(PropertyAccessInfo const* that, AccessMode access_mode,
              Zone* zone) V8_WARN_UNUSED_RESULT;
@@ -183,7 +183,7 @@ class PropertyAccessInfo final {
   PropertyAccessInfo(Zone* zone, Kind kind, MaybeHandle<JSObject> holder,
                      ZoneVector<Handle<Map>>&& lookup_start_object_maps);
   PropertyAccessInfo(Zone* zone, Kind kind, MaybeHandle<JSObject> holder,
-                     Handle<Object> constant, MaybeHandle<Name> name,
+                     Handle<Object> constant,
                      ZoneVector<Handle<Map>>&& lookup_start_object_maps);
   PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
                      MaybeHandle<Map> transition_map, FieldIndex field_index,

src/compiler/compilation-dependencies.cc

@@ -110,11 +110,10 @@ class ConstantInDictionaryPrototypeChainDependency final
  public:
   explicit ConstantInDictionaryPrototypeChainDependency(
       const MapRef receiver_map, const NameRef property_name,
-      const ObjectRef constant, PropertyKind kind)
+      const ObjectRef constant)
       : receiver_map_(receiver_map),
         property_name_{property_name},
-        constant_{constant},
-        kind_{kind} {
+        constant_{constant} {
     DCHECK(V8_DICT_PROPERTY_CONST_TRACKING_BOOL);
   }
 
@@ -167,26 +166,7 @@ class ConstantInDictionaryPrototypeChainDependency final
         return ValidationResult::kFoundIncorrect;
       }
 
-      Object dictionary_value = dictionary.ValueAt(entry);
-      Object value;
-      // We must be able to detect the case that the property |property_name_|
-      // of |holder_| was originally a plain function |constant_| (when creating
-      // this dependency) and has since become an accessor whose getter is
-      // |constant_|. Therefore, we cannot just look at the property kind of
-      // |details|, because that reflects the current situation, not the one
-      // when creating this dependency.
-      if (details.kind() != kind_) {
-        return ValidationResult::kFoundIncorrect;
-      }
-      if (kind_ == PropertyKind::kAccessor) {
-        // Only supporting loading at the moment, so we only ever want the
-        // getter.
-        CHECK(dictionary_value.IsAccessorPair());
-        value = AccessorPair::cast(dictionary_value)
-                    .get(AccessorComponent::ACCESSOR_GETTER);
-      } else {
-        value = dictionary_value;
-      }
+      Object value = dictionary.ValueAt(entry);
       return value == *constant_.object() ? ValidationResult::kFoundCorrect
                                           : ValidationResult::kFoundIncorrect;
     };
@@ -222,7 +202,6 @@ class ConstantInDictionaryPrototypeChainDependency final
   MapRef receiver_map_;
   NameRef property_name_;
   ObjectRef constant_;
-  PropertyKind kind_;
 };
 
 class TransitionDependency final : public CompilationDependency {
@@ -525,9 +504,9 @@ void CompilationDependencies::DependOnStableMap(const MapRef& map) {
 
 void CompilationDependencies::DependOnConstantInDictionaryPrototypeChain(
     const MapRef& receiver_map, const NameRef& property_name,
-    const ObjectRef& constant, PropertyKind kind) {
+    const ObjectRef& constant) {
   RecordDependency(zone_->New<ConstantInDictionaryPrototypeChainDependency>(
-      receiver_map, property_name, constant, kind));
+      receiver_map, property_name, constant));
 }
 
 AllocationType CompilationDependencies::DependOnPretenureMode(

src/compiler/compilation-dependencies.h

@@ -51,13 +51,9 @@ class V8_EXPORT_PRIVATE CompilationDependencies : public ZoneObject {
   // of the objects between receiver and |holder| on the prototype chain, b) any
   // of the objects on the prototype chain up to |holder| change prototypes, or
   // c) the value of |property_name| in |holder| changes.
-  // If PropertyKind is kData, |constant| is the value of the property in
-  // question. In case of PropertyKind::kAccessor, |constant| is the accessor
-  // function (i.e., getter or setter) itself, not the overall AccessorPair.
   void DependOnConstantInDictionaryPrototypeChain(const MapRef& receiver_map,
                                                   const NameRef& property_name,
-                                                  const ObjectRef& constant,
-                                                  PropertyKind kind);
+                                                  const ObjectRef& constant);
 
   // Return the pretenure mode of {site} and record the assumption that it does
   // not change.

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

@@ -2224,16 +2224,6 @@ Node* JSNativeContextSpecialization::InlinePropertyGetterCall(
     Node* frame_state, Node** effect, Node** control,
     ZoneVector<Node*>* if_exceptions, PropertyAccessInfo const& access_info) {
   ObjectRef constant(broker(), access_info.constant());
-
-  if (access_info.IsDictionaryProtoAccessorConstant()) {
-    // For fast mode holders we recorded dependencies in BuildPropertyLoad.
-    for (const Handle<Map> map : access_info.lookup_start_object_maps()) {
-      dependencies()->DependOnConstantInDictionaryPrototypeChain(
-          MapRef{broker(), map}, NameRef{broker(), access_info.name()},
-          constant, PropertyKind::kAccessor);
-    }
-  }
-
   Node* target = jsgraph()->Constant(constant);
   // Introduce the call to the getter function.
   Node* value;
@@ -2369,8 +2359,7 @@ JSNativeContextSpecialization::BuildPropertyLoad(
   Node* value;
   if (access_info.IsNotFound()) {
     value = jsgraph()->UndefinedConstant();
-  } else if (access_info.IsFastAccessorConstant() ||
-             access_info.IsDictionaryProtoAccessorConstant()) {
+  } else if (access_info.IsFastAccessorConstant()) {
     ConvertReceiverMode receiver_mode =
         receiver == lookup_start_object
             ? ConvertReceiverMode::kNotNullOrUndefined

src/compiler/property-access-builder.cc

@@ -160,7 +160,7 @@ Node* PropertyAccessBuilder::FoldLoadDictPrototypeConstant(
   for (const Handle<Map> map : access_info.lookup_start_object_maps()) {
     dependencies()->DependOnConstantInDictionaryPrototypeChain(
         MapRef{broker(), map}, NameRef{broker(), access_info.name()},
-        value.value(), PropertyKind::kData);
+        value.value());
   }
 
   return jsgraph()->Constant(value.value());

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

@@ -3012,8 +3012,7 @@ SerializerForBackgroundCompilation::ProcessMapForNamedPropertyAccess(
 
   // For JSNativeContextSpecialization::InlinePropertySetterCall
   // and InlinePropertyGetterCall.
-  if ((access_info.IsFastAccessorConstant() ||
-       access_info.IsDictionaryProtoAccessorConstant()) &&
+  if (access_info.IsFastAccessorConstant() &&
       !access_info.constant().is_null()) {
     if (access_info.constant()->IsJSFunction()) {
       JSFunctionRef function(broker(), access_info.constant());

test/mjsunit/const-dict-tracking.js

@@ -372,38 +372,6 @@ function testbench(o, proto, update_proto, check_constness) {
   }
 })();
 
-// Test inlining of accessor.
-(function() {
-  var proto = Object.create(null);
-  proto.x_val = 1;
-  Object.defineProperty(proto, "x", {
-    get : function () {return this.x_val;}
-  });
-
-  var o = Object.create(proto);
-  assertFalse(%HasFastProperties(proto))
-
-  function read_x(arg_o) {
-    return arg_o.x;
-  }
-
-  %PrepareFunctionForOptimization(read_x);
-  assertEquals(1, read_x(o));
-  %OptimizeFunctionOnNextCall(read_x);
-  assertEquals(1, read_x(o));
-  assertOptimized(read_x);
-
-  // Test that we inlined the access:
-  var dummy = {x : 123};
-  read_x(dummy);
-
-  if (%IsDictPropertyConstTrackingEnabled()) {
-    assertTrue(%HasFastProperties(o));
-    assertFalse(%HasFastProperties(proto));
-    assertUnoptimized(read_x);
-  }
-})();
-
 // Invalidation by adding same property to receiver.
 (function() {
   var proto = Object.create(null);