Revert "Add fast path to ObjectGetOwnPropertyDescriptor"

This reverts commit e0b76c9a.

Reason for revert: Changes layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/18522

See also:
https://github.com/v8/v8/wiki/Blink-layout-tests

Original change's description:
> Add fast path to ObjectGetOwnPropertyDescriptor
> 
> Bug: v8:6557
> Change-Id: I384e9f36058c73d40be1faf5ae1bf8c01d068f0e
> Reviewed-on: https://chromium-review.googlesource.com/682059
> Commit-Queue: Maya Lekova <mslekova@google.com>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48166}

TBR=ishell@chromium.org,mslekova@google.com

Change-Id: I27aa52786f79dd617faea6336df684821c1720d2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6557
Reviewed-on: https://chromium-review.googlesource.com/685314Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48171}

src/builtins/builtins-object-gen.cc

@@ -27,13 +27,6 @@ class ObjectBuiltinsAssembler : public CodeStubAssembler {
   void AddToDictionaryIf(Node* condition, Node* name_dictionary,
                          Handle<Name> name, Node* value, Label* bailout);
   Node* FromPropertyDescriptor(Node* context, Node* desc);
-  Node* FromPropertyDetails(Node* context, Node* raw_value, Node* details,
-                            Label* if_bailout);
-  Node* ConstructAccessorDescriptor(Node* context, Node* getter, Node* setter,
-                                    Node* enumerable, Node* configurable);
-  Node* ConstructDataDescriptor(Node* context, Node* value, Node* writable,
-                                Node* enumerable, Node* configurable);
-  Node* GetAccessorOrUndefined(Node* accessor, Label* if_bailout);
 };
 
 void ObjectBuiltinsAssembler::ReturnToStringFormat(Node* context,
@@ -48,55 +41,6 @@ void ObjectBuiltinsAssembler::ReturnToStringFormat(Node* context,
                   rhs));
 }
 
-Node* ObjectBuiltinsAssembler::ConstructAccessorDescriptor(Node* context,
-                                                           Node* getter,
-                                                           Node* setter,
-                                                           Node* enumerable,
-                                                           Node* configurable) {
-  Node* native_context = LoadNativeContext(context);
-  Node* map = LoadContextElement(
-      native_context, Context::ACCESSOR_PROPERTY_DESCRIPTOR_MAP_INDEX);
-  Node* js_desc = AllocateJSObjectFromMap(map);
-
-  StoreObjectFieldNoWriteBarrier(
-      js_desc, JSAccessorPropertyDescriptor::kGetOffset, getter);
-  StoreObjectFieldNoWriteBarrier(
-      js_desc, JSAccessorPropertyDescriptor::kSetOffset, setter);
-  StoreObjectFieldNoWriteBarrier(
-      js_desc, JSAccessorPropertyDescriptor::kEnumerableOffset,
-      SelectBooleanConstant(enumerable));
-  StoreObjectFieldNoWriteBarrier(
-      js_desc, JSAccessorPropertyDescriptor::kConfigurableOffset,
-      SelectBooleanConstant(configurable));
-
-  return js_desc;
-}
-
-Node* ObjectBuiltinsAssembler::ConstructDataDescriptor(Node* context,
-                                                       Node* value,
-                                                       Node* writable,
-                                                       Node* enumerable,
-                                                       Node* configurable) {
-  Node* native_context = LoadNativeContext(context);
-  Node* map = LoadContextElement(native_context,
-                                 Context::DATA_PROPERTY_DESCRIPTOR_MAP_INDEX);
-  Node* js_desc = AllocateJSObjectFromMap(map);
-
-  StoreObjectFieldNoWriteBarrier(js_desc,
-                                 JSDataPropertyDescriptor::kValueOffset, value);
-  StoreObjectFieldNoWriteBarrier(js_desc,
-                                 JSDataPropertyDescriptor::kWritableOffset,
-                                 SelectBooleanConstant(writable));
-  StoreObjectFieldNoWriteBarrier(js_desc,
-                                 JSDataPropertyDescriptor::kEnumerableOffset,
-                                 SelectBooleanConstant(enumerable));
-  StoreObjectFieldNoWriteBarrier(js_desc,
-                                 JSDataPropertyDescriptor::kConfigurableOffset,
-                                 SelectBooleanConstant(configurable));
-
-  return js_desc;
-}
-
 TF_BUILTIN(ObjectPrototypeHasOwnProperty, ObjectBuiltinsAssembler) {
   Node* object = Parameter(Descriptor::kReceiver);
   Node* key = Parameter(Descriptor::kKey);
@@ -757,87 +701,27 @@ TF_BUILTIN(ObjectGetOwnPropertyDescriptor, ObjectBuiltinsAssembler) {
                              UndefinedConstant()));
 
   CodeStubArguments args(this, ChangeInt32ToIntPtr(argc));
-  Node* object = args.GetOptionalArgumentValue(0);
+  Node* obj = args.GetOptionalArgumentValue(0);
   Node* key = args.GetOptionalArgumentValue(1);
 
   // 1. Let obj be ? ToObject(O).
-  object = CallBuiltin(Builtins::kToObject, context, object);
+  Node* object = CallBuiltin(Builtins::kToObject, context, obj);
 
   // 2. Let key be ? ToPropertyKey(P).
-  key = ToName(context, key);
+  Node* name = ToName(context, key);
 
   // 3. Let desc be ? obj.[[GetOwnProperty]](key).
-  Label if_keyisindex(this), if_iskeyunique(this),
-      call_runtime(this, Label::kDeferred),
-      return_undefined(this, Label::kDeferred), if_notunique_name(this);
-  {
-    VARIABLE(var_index, MachineType::PointerRepresentation(),
-             IntPtrConstant(0));
-    VARIABLE(var_name, MachineRepresentation::kTagged);
-
-    TryToName(key, &if_keyisindex, &var_index, &if_iskeyunique, &var_name,
-              &call_runtime, &if_notunique_name);
-
-    BIND(&if_notunique_name);
-    {
-      Label not_in_string_table(this);
-      TryInternalizeString(key, &if_keyisindex, &var_index, &if_iskeyunique,
-                           &var_name, &not_in_string_table, &call_runtime);
-
-      BIND(&not_in_string_table);
-      {
-        // If the string was not found in the string table, then no regular
-        // object can have a property with that name, so return |undefined|.
-        Goto(&return_undefined);
-      }
-    }
-
-    BIND(&if_iskeyunique);
-    {
-      Label if_found_value(this), return_empty(this), if_not_found(this);
-
-      Node* map = LoadMap(object);
-      Node* instance_type = LoadInstanceType(object);
-      VARIABLE(var_value, MachineRepresentation::kTagged);
-      VARIABLE(var_details, MachineRepresentation::kWord32);
-      VARIABLE(var_raw_value, MachineRepresentation::kTagged);
-
-      TryGetOwnProperty(context, object, object, map, instance_type,
-                        var_name.value(), &if_found_value, &var_value,
-                        &var_details, &var_raw_value, &return_empty,
-                        &if_not_found, kReturnAccessorPair);
-
-      BIND(&if_found_value);
-      // 4. Return FromPropertyDescriptor(desc).
-      Node* js_desc = FromPropertyDetails(context, var_value.value(),
-                                          var_details.value(), &call_runtime);
-      args.PopAndReturn(js_desc);
-
-      BIND(&return_empty);
-      var_value.Bind(UndefinedConstant());
-      args.PopAndReturn(UndefinedConstant());
-
-      BIND(&if_not_found);
-      Goto(&call_runtime);
-    }
-  }
-
-  BIND(&if_keyisindex);
-  Goto(&call_runtime);
-
-  BIND(&call_runtime);
-  {
   Node* desc =
-        CallRuntime(Runtime::kGetOwnPropertyDescriptor, context, object, key);
+      CallRuntime(Runtime::kGetOwnPropertyDescriptor, context, object, name);
 
+  Label return_undefined(this, Label::kDeferred);
   GotoIf(IsUndefined(desc), &return_undefined);
 
   CSA_ASSERT(this, IsFixedArray(desc));
 
   // 4. Return FromPropertyDescriptor(desc).
-    Node* js_desc = FromPropertyDescriptor(context, desc);
-    args.PopAndReturn(js_desc);
-  }
+  args.PopAndReturn(FromPropertyDescriptor(context, desc));
+
   BIND(&return_undefined);
   args.PopAndReturn(UndefinedConstant());
 }
@@ -880,21 +764,54 @@ Node* ObjectBuiltinsAssembler::FromPropertyDescriptor(Node* context,
 
   BIND(&if_accessor_desc);
   {
-    js_descriptor.Bind(ConstructAccessorDescriptor(
-        context, LoadObjectField(desc, PropertyDescriptorObject::kGetOffset),
-        LoadObjectField(desc, PropertyDescriptorObject::kSetOffset),
-        IsSetWord32<PropertyDescriptorObject::IsEnumerableBit>(flags),
+    Node* native_context = LoadNativeContext(context);
+    Node* map = LoadContextElement(
+        native_context, Context::ACCESSOR_PROPERTY_DESCRIPTOR_MAP_INDEX);
+    Node* js_desc = AllocateJSObjectFromMap(map);
+
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSAccessorPropertyDescriptor::kGetOffset,
+        LoadObjectField(desc, PropertyDescriptorObject::kGetOffset));
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSAccessorPropertyDescriptor::kSetOffset,
+        LoadObjectField(desc, PropertyDescriptorObject::kSetOffset));
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSAccessorPropertyDescriptor::kEnumerableOffset,
+        SelectBooleanConstant(
+            IsSetWord32<PropertyDescriptorObject::IsEnumerableBit>(flags)));
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSAccessorPropertyDescriptor::kConfigurableOffset,
+        SelectBooleanConstant(
             IsSetWord32<PropertyDescriptorObject::IsConfigurableBit>(flags)));
+
+    js_descriptor.Bind(js_desc);
     Goto(&return_desc);
   }
 
   BIND(&if_data_desc);
   {
-    js_descriptor.Bind(ConstructDataDescriptor(
-        context, LoadObjectField(desc, PropertyDescriptorObject::kValueOffset),
-        IsSetWord32<PropertyDescriptorObject::IsWritableBit>(flags),
-        IsSetWord32<PropertyDescriptorObject::IsEnumerableBit>(flags),
+    Node* native_context = LoadNativeContext(context);
+    Node* map = LoadContextElement(native_context,
+                                   Context::DATA_PROPERTY_DESCRIPTOR_MAP_INDEX);
+    Node* js_desc = AllocateJSObjectFromMap(map);
+
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSDataPropertyDescriptor::kValueOffset,
+        LoadObjectField(desc, PropertyDescriptorObject::kValueOffset));
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSDataPropertyDescriptor::kWritableOffset,
+        SelectBooleanConstant(
+            IsSetWord32<PropertyDescriptorObject::IsWritableBit>(flags)));
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSDataPropertyDescriptor::kEnumerableOffset,
+        SelectBooleanConstant(
+            IsSetWord32<PropertyDescriptorObject::IsEnumerableBit>(flags)));
+    StoreObjectFieldNoWriteBarrier(
+        js_desc, JSDataPropertyDescriptor::kConfigurableOffset,
+        SelectBooleanConstant(
             IsSetWord32<PropertyDescriptorObject::IsConfigurableBit>(flags)));
+
+    js_descriptor.Bind(js_desc);
     Goto(&return_desc);
   }
 
@@ -952,60 +869,5 @@ Node* ObjectBuiltinsAssembler::FromPropertyDescriptor(Node* context,
   BIND(&return_desc);
   return js_descriptor.value();
 }
-
-Node* ObjectBuiltinsAssembler::FromPropertyDetails(Node* context,
-                                                   Node* raw_value,
-                                                   Node* details,
-                                                   Label* if_bailout) {
-  VARIABLE(js_descriptor, MachineRepresentation::kTagged);
-
-  Label if_accessor_desc(this), if_data_desc(this), return_desc(this);
-  BranchIfAccessorPair(raw_value, &if_accessor_desc, &if_data_desc);
-
-  BIND(&if_accessor_desc);
-  {
-    Node* getter = LoadObjectField(raw_value, AccessorPair::kGetterOffset);
-    Node* setter = LoadObjectField(raw_value, AccessorPair::kSetterOffset);
-    js_descriptor.Bind(ConstructAccessorDescriptor(
-        context, GetAccessorOrUndefined(getter, if_bailout),
-        GetAccessorOrUndefined(setter, if_bailout),
-        IsNotSetWord32(details, PropertyDetails::kAttributesDontEnumMask),
-        IsNotSetWord32(details, PropertyDetails::kAttributesDontDeleteMask)));
-    Goto(&return_desc);
-  }
-
-  BIND(&if_data_desc);
-  {
-    js_descriptor.Bind(ConstructDataDescriptor(
-        context, raw_value,
-        IsNotSetWord32(details, PropertyDetails::kAttributesReadOnlyMask),
-        IsNotSetWord32(details, PropertyDetails::kAttributesDontEnumMask),
-        IsNotSetWord32(details, PropertyDetails::kAttributesDontDeleteMask)));
-    Goto(&return_desc);
-  }
-
-  BIND(&return_desc);
-  return js_descriptor.value();
-}
-
-Node* ObjectBuiltinsAssembler::GetAccessorOrUndefined(Node* accessor,
-                                                      Label* if_bailout) {
-  Label bind_undefined(this, Label::kDeferred), return_result(this);
-  VARIABLE(result, MachineRepresentation::kTagged);
-
-  GotoIf(IsNull(accessor), &bind_undefined);
-  result.Bind(accessor);
-  Node* map = LoadMap(accessor);
-  // TODO(ishell): probe template instantiations cache.
-  GotoIf(IsFunctionTemplateInfoMap(map), if_bailout);
-  Goto(&return_result);
-
-  BIND(&bind_undefined);
-  result.Bind(UndefinedConstant());
-  Goto(&return_result);
-
-  BIND(&return_result);
-  return result.value();
-}
 }  // namespace internal
 }  // namespace v8

src/builtins/builtins-proxy-gen.h

@@ -16,6 +16,12 @@ class ProxiesCodeStubAssembler : public CodeStubAssembler {
   explicit ProxiesCodeStubAssembler(compiler::CodeAssemblerState* state)
       : CodeStubAssembler(state) {}
 
+  void BranchIfAccessorPair(Node* value, Label* if_accessor_pair,
+                            Label* if_not_accessor_pair) {
+    GotoIf(TaggedIsSmi(value), if_not_accessor_pair);
+    Branch(IsAccessorPair(value), if_accessor_pair, if_not_accessor_pair);
+  }
+
   // ES6 section 9.5.8 [[Get]] ( P, Receiver )
   // name should not be an index.
   Node* ProxyGetProperty(Node* context, Node* proxy, Node* name,

src/code-stub-assembler.cc

@@ -6115,8 +6115,7 @@ void CodeStubAssembler::LoadPropertyFromGlobalDictionary(Node* dictionary,
 // Returns either the original value, or the result of the getter call.
 Node* CodeStubAssembler::CallGetterIfAccessor(Node* value, Node* details,
                                               Node* context, Node* receiver,
-                                              Label* if_bailout,
-                                              GetOwnPropertyMode mode) {
+                                              Label* if_bailout) {
   VARIABLE(var_value, MachineRepresentation::kTagged, value);
   Label done(this), if_accessor_info(this, Label::kDeferred);
 
@@ -6128,15 +6127,13 @@ Node* CodeStubAssembler::CallGetterIfAccessor(Node* value, Node* details,
 
   // AccessorPair case.
   {
-    if (mode == kCallJSGetter) {
     Node* accessor_pair = value;
-      Node* getter =
-          LoadObjectField(accessor_pair, AccessorPair::kGetterOffset);
+    Node* getter = LoadObjectField(accessor_pair, AccessorPair::kGetterOffset);
     Node* getter_map = LoadMap(getter);
     Node* instance_type = LoadMapInstanceType(getter_map);
     // FunctionTemplateInfo getters are not supported yet.
-      GotoIf(Word32Equal(instance_type,
-                         Int32Constant(FUNCTION_TEMPLATE_INFO_TYPE)),
+    GotoIf(
+        Word32Equal(instance_type, Int32Constant(FUNCTION_TEMPLATE_INFO_TYPE)),
         if_bailout);
 
     // Return undefined if the {getter} is not callable.
@@ -6147,7 +6144,6 @@ Node* CodeStubAssembler::CallGetterIfAccessor(Node* value, Node* details,
     Callable callable = CodeFactory::Call(isolate());
     Node* result = CallJS(callable, context, getter, receiver);
     var_value.Bind(result);
-    }
     Goto(&done);
   }
 
@@ -6224,7 +6220,7 @@ void CodeStubAssembler::TryGetOwnProperty(
     Node* context, Node* receiver, Node* object, Node* map, Node* instance_type,
     Node* unique_name, Label* if_found_value, Variable* var_value,
     Variable* var_details, Variable* var_raw_value, Label* if_not_found,
-    Label* if_bailout, GetOwnPropertyMode mode) {
+    Label* if_bailout) {
   DCHECK_EQ(MachineRepresentation::kTagged, var_value->rep());
   Comment("TryGetOwnProperty");
 
@@ -6271,12 +6267,11 @@ void CodeStubAssembler::TryGetOwnProperty(
   // Here we have details and value which could be an accessor.
   BIND(&if_found);
   {
-    // TODO(ishell): Execute C++ accessor in case of accessor info
     if (var_raw_value) {
       var_raw_value->Bind(var_value->value());
     }
     Node* value = CallGetterIfAccessor(var_value->value(), var_details->value(),
-                                       context, receiver, if_bailout, mode);
+                                       context, receiver, if_bailout);
     var_value->Bind(value);
     Goto(if_found_value);
   }

src/code-stub-assembler.h

@@ -1102,12 +1102,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                           Int32Constant(0));
   }
 
-  // Returns true if none of the mask's bits in given |word32| are set.
-  TNode<BoolT> IsNotSetWord32(SloppyTNode<Word32T> word32, uint32_t mask) {
-    return Word32Equal(Word32And(word32, Int32Constant(mask)),
-                       Int32Constant(0));
-  }
-
   // Returns true if any of the |T|'s bits in given |word| are set.
   template <typename T>
   Node* IsSetWord(Node* word) {
@@ -1320,9 +1314,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                          Node* unique_name, Label* if_found,
                          Label* if_not_found, Label* if_bailout);
 
-  // Operating mode for TryGetOwnProperty and CallGetterIfAccessor
-  // kReturnAccessorPair is used when we're only getting the property descriptor
-  enum GetOwnPropertyMode { kCallJSGetter, kReturnAccessorPair };
   // Tries to get {object}'s own {unique_name} property value. If the property
   // is an accessor then it also calls a getter. If the property is a double
   // field it re-wraps value in an immutable heap number.
@@ -1334,8 +1325,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                          Node* instance_type, Node* unique_name,
                          Label* if_found, Variable* var_value,
                          Variable* var_details, Variable* var_raw_value,
-                         Label* if_not_found, Label* if_bailout,
-                         GetOwnPropertyMode mode = kCallJSGetter);
+                         Label* if_not_found, Label* if_bailout);
 
   Node* GetProperty(Node* context, Node* receiver, Handle<Name> name) {
     return GetProperty(context, receiver, HeapConstant(name));
@@ -1578,12 +1568,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                                            Node* lhs, Node* rhs, Label* if_true,
                                            Label* if_false);
 
-  void BranchIfAccessorPair(Node* value, Label* if_accessor_pair,
-                            Label* if_not_accessor_pair) {
-    GotoIf(TaggedIsSmi(value), if_not_accessor_pair);
-    Branch(IsAccessorPair(value), if_accessor_pair, if_not_accessor_pair);
-  }
-
   void GotoIfNumberGreaterThanOrEqual(Node* lhs, Node* rhs, Label* if_false);
 
   Node* Equal(Node* lhs, Node* rhs, Node* context,
@@ -1680,8 +1664,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* DescriptorArrayGetKey(Node* descriptors, Node* descriptor_number);
 
   Node* CallGetterIfAccessor(Node* value, Node* details, Node* context,
-                             Node* receiver, Label* if_bailout,
-                             GetOwnPropertyMode mode = kCallJSGetter);
+                             Node* receiver, Label* if_bailout);
 
   Node* TryToIntptr(Node* key, Label* miss);
 

src/property-details.h

@@ -342,8 +342,6 @@ class PropertyDetails BASE_EMBEDDED {
       (READ_ONLY << AttributesField::kShift);
   static const int kAttributesDontDeleteMask =
       (DONT_DELETE << AttributesField::kShift);
-  static const int kAttributesDontEnumMask =
-      (DONT_ENUM << AttributesField::kShift);
 
   // Bit fields for normalized objects.
   class PropertyCellTypeField