Revert "Reimplement Object.entries/values as CSA to optimize performance."

This reverts commit d30a8fa9.

Reason for revert: no-snap test failures here
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap%20-%20debug/builds/17068
You need to update the whitelist in src/debug/debug-evaluate.cc.
I'm a bit surprised this only happens in no-snap builds.


Original change's description:
> Reimplement Object.entries/values as CSA to optimize performance.
> 
> This implementation based on runtime implementation.
> 
> Bug: v8:6804
> Change-Id: Ib8bfcc4648e44a999789237effc0275c5e4d9936
> Reviewed-on: https://chromium-review.googlesource.com/810504
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50477}

TBR=cbruni@chromium.org,jgruber@chromium.org,ishell@chromium.org,brn@b6n.ch

Change-Id: I1a0c8e3c054a57ca4d15f7a064ff4b28ca133b16
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6804
Reviewed-on: https://chromium-review.googlesource.com/859937Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50478}

AUTHORS

@@ -135,7 +135,6 @@ Sanjoy Das <sanjoy@playingwithpointers.com>
 Seo Sanghyeon <sanxiyn@gmail.com>
 Stefan Penner <stefan.penner@gmail.com>
 Sylvestre Ledru <sledru@mozilla.com>
-Taketoshi Aono <brn@b6n.ch>
 Tiancheng "Timothy" Gu <timothygu99@gmail.com>
 Tobias Burnus <burnus@net-b.de>
 Victor Costan <costan@gmail.com>

src/bootstrapper.cc

@@ -1506,9 +1506,9 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
         object_function, "keys", Builtins::kObjectKeys, 1, true);
     native_context()->set_object_keys(*object_keys);
     SimpleInstallFunction(object_function, factory->entries_string(),
-                          Builtins::kObjectEntries, 1, true);
+                          Builtins::kObjectEntries, 1, false);
     SimpleInstallFunction(object_function, factory->values_string(),
-                          Builtins::kObjectValues, 1, true);
+                          Builtins::kObjectValues, 1, false);
 
     SimpleInstallFunction(isolate->initial_object_prototype(),
                           "__defineGetter__", Builtins::kObjectDefineGetter, 2,

src/builtins/builtins-definitions.h

@@ -754,7 +754,7 @@ namespace internal {
   CPP(ObjectDefineProperties)                                                  \
   CPP(ObjectDefineProperty)                                                    \
   CPP(ObjectDefineSetter)                                                      \
-  TFJ(ObjectEntries, 1, kObject)                                               \
+  CPP(ObjectEntries)                                                           \
   CPP(ObjectFreeze)                                                            \
   TFJ(ObjectGetOwnPropertyDescriptor,                                          \
       SharedFunctionInfo::kDontAdaptArgumentsSentinel)                         \
@@ -784,7 +784,7 @@ namespace internal {
   /* ES #sec-object.prototype.tolocalestring */                                \
   TFJ(ObjectPrototypeToLocaleString, 0)                                        \
   CPP(ObjectSeal)                                                              \
-  TFJ(ObjectValues, 1, kObject)                                                \
+  CPP(ObjectValues)                                                            \
                                                                                \
   /* instanceof */                                                             \
   TFC(OrdinaryHasInstance, Compare, 1)                                         \

src/builtins/builtins-object.cc

@@ -395,6 +395,31 @@ BUILTIN(ObjectIsSealed) {
   return isolate->heap()->ToBoolean(result.FromJust());
 }
 
+BUILTIN(ObjectValues) {
+  HandleScope scope(isolate);
+  Handle<Object> object = args.atOrUndefined(isolate, 1);
+  Handle<JSReceiver> receiver;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
+                                     Object::ToObject(isolate, object));
+  Handle<FixedArray> values;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, values, JSReceiver::GetOwnValues(receiver, ENUMERABLE_STRINGS));
+  return *isolate->factory()->NewJSArrayWithElements(values);
+}
+
+BUILTIN(ObjectEntries) {
+  HandleScope scope(isolate);
+  Handle<Object> object = args.atOrUndefined(isolate, 1);
+  Handle<JSReceiver> receiver;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
+                                     Object::ToObject(isolate, object));
+  Handle<FixedArray> entries;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, entries,
+      JSReceiver::GetOwnEntries(receiver, ENUMERABLE_STRINGS));
+  return *isolate->factory()->NewJSArrayWithElements(entries);
+}
+
 BUILTIN(ObjectGetOwnPropertyDescriptors) {
   HandleScope scope(isolate);
   Handle<Object> object = args.atOrUndefined(isolate, 1);

src/code-stub-assembler.cc

@@ -4019,6 +4019,19 @@ Node* CodeStubAssembler::InstanceTypeEqual(Node* instance_type, int type) {
   return Word32Equal(instance_type, Int32Constant(type));
 }
 
+Node* CodeStubAssembler::IsSpecialReceiverMap(Node* map) {
+  CSA_SLOW_ASSERT(this, IsMap(map));
+  Node* is_special = IsSpecialReceiverInstanceType(LoadMapInstanceType(map));
+  uint32_t mask =
+      Map::HasNamedInterceptorBit::kMask | Map::IsAccessCheckNeededBit::kMask;
+  USE(mask);
+  // Interceptors or access checks imply special receiver.
+  CSA_ASSERT(this,
+             SelectConstant(IsSetWord32(LoadMapBitField(map), mask), is_special,
+                            Int32Constant(1), MachineRepresentation::kWord32));
+  return is_special;
+}
+
 TNode<BoolT> CodeStubAssembler::IsDictionaryMap(SloppyTNode<Map> map) {
   CSA_SLOW_ASSERT(this, IsMap(map));
   Node* bit_field3 = LoadMapBitField3(map);
@@ -6367,38 +6380,36 @@ Node* CodeStubAssembler::DescriptorArrayNumberOfEntries(Node* descriptors) {
       descriptors, IntPtrConstant(DescriptorArray::kDescriptorLengthIndex));
 }
 
-Node* CodeStubAssembler::DescriptorNumberToIndex(
-    SloppyTNode<Uint32T> descriptor_number) {
-  Node* descriptor_size = Int32Constant(DescriptorArray::kEntrySize);
-  Node* index = Int32Mul(descriptor_number, descriptor_size);
-  return ChangeInt32ToIntPtr(index);
+namespace {
+
+Node* DescriptorNumberToIndex(CodeStubAssembler* a, Node* descriptor_number) {
+  Node* descriptor_size = a->Int32Constant(DescriptorArray::kEntrySize);
+  Node* index = a->Int32Mul(descriptor_number, descriptor_size);
+  return a->ChangeInt32ToIntPtr(index);
 }
 
+}  // namespace
+
 Node* CodeStubAssembler::DescriptorArrayToKeyIndex(Node* descriptor_number) {
   return IntPtrAdd(IntPtrConstant(DescriptorArray::ToKeyIndex(0)),
-                   DescriptorNumberToIndex(descriptor_number));
+                   DescriptorNumberToIndex(this, descriptor_number));
 }
 
 Node* CodeStubAssembler::DescriptorArrayGetSortedKeyIndex(
     Node* descriptors, Node* descriptor_number) {
-  Node* details = DescriptorArrayGetDetails(
-      TNode<DescriptorArray>::UncheckedCast(descriptors),
-      TNode<Uint32T>::UncheckedCast(descriptor_number));
+  const int details_offset = DescriptorArray::ToDetailsIndex(0) * kPointerSize;
+  Node* details = LoadAndUntagToWord32FixedArrayElement(
+      descriptors, DescriptorNumberToIndex(this, descriptor_number),
+      details_offset);
   return DecodeWord32<PropertyDetails::DescriptorPointer>(details);
 }
 
 Node* CodeStubAssembler::DescriptorArrayGetKey(Node* descriptors,
                                                Node* descriptor_number) {
   const int key_offset = DescriptorArray::ToKeyIndex(0) * kPointerSize;
-  return LoadFixedArrayElement(
-      descriptors, DescriptorNumberToIndex(descriptor_number), key_offset);
-}
-
-TNode<Uint32T> CodeStubAssembler::DescriptorArrayGetDetails(
-    TNode<DescriptorArray> descriptors, TNode<Uint32T> descriptor_number) {
-  const int details_offset = DescriptorArray::ToDetailsIndex(0) * kPointerSize;
-  return TNode<Uint32T>::UncheckedCast(LoadAndUntagToWord32FixedArrayElement(
-      descriptors, DescriptorNumberToIndex(descriptor_number), details_offset));
+  return LoadFixedArrayElement(descriptors,
+                               DescriptorNumberToIndex(this, descriptor_number),
+                               key_offset);
 }
 
 void CodeStubAssembler::DescriptorLookupBinary(Node* unique_name,
@@ -6597,22 +6608,12 @@ void CodeStubAssembler::LoadPropertyFromFastObject(Node* object, Node* map,
                                                    Variable* var_value) {
   DCHECK_EQ(MachineRepresentation::kWord32, var_details->rep());
   DCHECK_EQ(MachineRepresentation::kTagged, var_value->rep());
+  Comment("[ LoadPropertyFromFastObject");
 
   Node* details =
       LoadDetailsByKeyIndex<DescriptorArray>(descriptors, name_index);
   var_details->Bind(details);
 
-  LoadPropertyFromFastObject(object, map, descriptors, name_index, details,
-                             var_value);
-}
-
-void CodeStubAssembler::LoadPropertyFromFastObject(Node* object, Node* map,
-                                                   Node* descriptors,
-                                                   Node* name_index,
-                                                   Node* details,
-                                                   Variable* var_value) {
-  Comment("[ LoadPropertyFromFastObject");
-
   Node* location = DecodeWord32<PropertyDetails::LocationField>(details);
 
   Label if_in_field(this), if_in_descriptor(this), done(this);

src/code-stub-assembler.h

@@ -1114,6 +1114,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* IsSequentialStringInstanceType(Node* instance_type);
   Node* IsShortExternalStringInstanceType(Node* instance_type);
   Node* IsSpecialReceiverInstanceType(Node* instance_type);
+  Node* IsSpecialReceiverMap(Node* map);
   Node* IsSpeciesProtectorCellInvalid();
   Node* IsStringInstanceType(Node* instance_type);
   Node* IsString(Node* object);
@@ -1571,10 +1572,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
                                   Node* name_index, Variable* var_details,
                                   Variable* var_value);
 
-  void LoadPropertyFromFastObject(Node* object, Node* map, Node* descriptors,
-                                  Node* name_index, Node* details,
-                                  Variable* var_value);
-
   void LoadPropertyFromNameDictionary(Node* dictionary, Node* entry,
                                       Variable* var_details,
                                       Variable* var_value);
@@ -1897,15 +1894,11 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   void DescriptorLookupBinary(Node* unique_name, Node* descriptors, Node* nof,
                               Label* if_found, Variable* var_name_index,
                               Label* if_not_found);
-  Node* DescriptorNumberToIndex(SloppyTNode<Uint32T> descriptor_number);
   // Implements DescriptorArray::ToKeyIndex.
   // Returns an untagged IntPtr.
   Node* DescriptorArrayToKeyIndex(Node* descriptor_number);
   // Implements DescriptorArray::GetKey.
   Node* DescriptorArrayGetKey(Node* descriptors, Node* descriptor_number);
-  // Implements DescriptorArray::GetKey.
-  TNode<Uint32T> DescriptorArrayGetDetails(TNode<DescriptorArray> descriptors,
-                                           TNode<Uint32T> descriptor_number);
 
   Node* CallGetterIfAccessor(Node* value, Node* details, Node* context,
                              Node* receiver, Label* if_bailout,

src/objects.cc

@@ -8791,10 +8791,9 @@ MUST_USE_RESULT Maybe<bool> FastGetOwnValuesOrEntries(
 MaybeHandle<FixedArray> GetOwnValuesOrEntries(Isolate* isolate,
                                               Handle<JSReceiver> object,
                                               PropertyFilter filter,
-                                              bool try_fast_path,
                                               bool get_entries) {
   Handle<FixedArray> values_or_entries;
-  if (try_fast_path && filter == ENUMERABLE_STRINGS) {
+  if (filter == ENUMERABLE_STRINGS) {
     Maybe<bool> fast_values_or_entries = FastGetOwnValuesOrEntries(
         isolate, object, get_entries, &values_or_entries);
     if (fast_values_or_entries.IsNothing()) return MaybeHandle<FixedArray>();
@@ -8847,17 +8846,13 @@ MaybeHandle<FixedArray> GetOwnValuesOrEntries(Isolate* isolate,
 }
 
 MaybeHandle<FixedArray> JSReceiver::GetOwnValues(Handle<JSReceiver> object,
-                                                 PropertyFilter filter,
-                                                 bool try_fast_path) {
-  return GetOwnValuesOrEntries(object->GetIsolate(), object, filter,
-                               try_fast_path, false);
+                                                 PropertyFilter filter) {
+  return GetOwnValuesOrEntries(object->GetIsolate(), object, filter, false);
 }
 
 MaybeHandle<FixedArray> JSReceiver::GetOwnEntries(Handle<JSReceiver> object,
-                                                  PropertyFilter filter,
-                                                  bool try_fast_path) {
-  return GetOwnValuesOrEntries(object->GetIsolate(), object, filter,
-                               try_fast_path, true);
+                                                  PropertyFilter filter) {
+  return GetOwnValuesOrEntries(object->GetIsolate(), object, filter, true);
 }
 
 bool Map::DictionaryElementsInPrototypeChainOnly() {

src/objects.h

@@ -2182,12 +2182,10 @@ class JSReceiver: public HeapObject {
       Handle<JSReceiver> object);
 
   MUST_USE_RESULT static MaybeHandle<FixedArray> GetOwnValues(
-      Handle<JSReceiver> object, PropertyFilter filter,
-      bool try_fast_path = true);
+      Handle<JSReceiver> object, PropertyFilter filter);
 
   MUST_USE_RESULT static MaybeHandle<FixedArray> GetOwnEntries(
-      Handle<JSReceiver> object, PropertyFilter filter,
-      bool try_fast_path = true);
+      Handle<JSReceiver> object, PropertyFilter filter);
 
   static const int kHashMask = PropertyArray::HashField::kMask;
 

src/runtime/runtime-object.cc

@@ -439,61 +439,6 @@ RUNTIME_FUNCTION(Runtime_OptimizeObjectForAddingMultipleProperties) {
   return *object;
 }
 
-RUNTIME_FUNCTION(Runtime_ObjectValues) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-
-  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
-
-  Handle<FixedArray> values;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, values,
-      JSReceiver::GetOwnValues(receiver, PropertyFilter::ENUMERABLE_STRINGS,
-                               true));
-  return *isolate->factory()->NewJSArrayWithElements(values);
-}
-
-RUNTIME_FUNCTION(Runtime_ObjectValuesSkipFastPath) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-
-  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
-
-  Handle<FixedArray> value;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, value,
-      JSReceiver::GetOwnValues(receiver, PropertyFilter::ENUMERABLE_STRINGS,
-                               false));
-  return *isolate->factory()->NewJSArrayWithElements(value);
-}
-
-RUNTIME_FUNCTION(Runtime_ObjectEntries) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-
-  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
-
-  Handle<FixedArray> entries;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, entries,
-      JSReceiver::GetOwnEntries(receiver, PropertyFilter::ENUMERABLE_STRINGS,
-                                true));
-  return *isolate->factory()->NewJSArrayWithElements(entries);
-}
-
-RUNTIME_FUNCTION(Runtime_ObjectEntriesSkipFastPath) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-
-  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
-
-  Handle<FixedArray> entries;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, entries,
-      JSReceiver::GetOwnEntries(receiver, PropertyFilter::ENUMERABLE_STRINGS,
-                                false));
-  return *isolate->factory()->NewJSArrayWithElements(entries);
-}
 
 RUNTIME_FUNCTION(Runtime_GetProperty) {
   HandleScope scope(isolate);

src/runtime/runtime.h

@@ -392,10 +392,6 @@ namespace internal {
   F(ObjectCreate, 2, 1)                                         \
   F(InternalSetPrototype, 2, 1)                                 \
   F(OptimizeObjectForAddingMultipleProperties, 2, 1)            \
-  F(ObjectValues, 1, 1)                                         \
-  F(ObjectValuesSkipFastPath, 1, 1)                             \
-  F(ObjectEntries, 1, 1)                                        \
-  F(ObjectEntriesSkipFastPath, 1, 1)                            \
   F(GetProperty, 2, 1)                                          \
   F(KeyedGetProperty, 2, 1)                                     \
   F(AddNamedProperty, 4, 1)                                     \