[runtime] Better encapsulation of dictionary objects handling in lookup iterator.

Now LookupIterator follows the same pattern of prepare transition, apply transition
and write value when adding new properties to dictionary objects.

JSGlobalObject case:
* Prepare transition phase ensures that there is a "transition" property cell
  prepared for receiving a value.
* Apply transition phase does nothing.
* Prepare for data property phase ensures that the existing property cell can
  receive the value.
* Write value phase writes value directly to the current property cell.

JSObject case:
* Prepare transition phase prepares the object for receiving a data value (which
  could switch an object to dictionary mode).
* Apply transition phase migrates object to a transition map. If the map happened
  to be a dictionary mode object's map then an uninitialized entry added to the
  properties dictionary.
* Prepare for data property phase does nothing.
* Write value phase just puts value to the properties dictionary.

BUG=chromium:576312

Review-Url: https://codereview.chromium.org/2127583002
Cr-Commit-Position: refs/heads/master@{#37585}

src/ic/arm/handler-compiler-arm.cc

@@ -203,7 +203,8 @@ void NamedLoadHandlerCompiler::GenerateLoadFunctionPrototype(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/arm64/handler-compiler-arm64.cc

@@ -109,7 +109,8 @@ void NamedLoadHandlerCompiler::GenerateLoadFunctionPrototype(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/ia32/handler-compiler-ia32.cc

@@ -236,7 +236,8 @@ void PropertyHandlerCompiler::GenerateApiAccessorCall(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/ic.cc

@@ -1755,11 +1755,10 @@ Handle<Code> StoreIC::CompileHandler(LookupIterator* lookup,
     case LookupIterator::TRANSITION: {
       auto store_target = lookup->GetStoreTarget();
       if (store_target->IsJSGlobalObject()) {
-        // TODO(dcarney): this currently just deopts. Use the transition cell.
         TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobalTransition);
-        auto cell = isolate()->factory()->NewPropertyCell();
+        Handle<PropertyCell> cell = lookup->transition_cell();
         cell->set_value(*value);
-        auto code = PropertyCellStoreHandler(
+        Handle<Code> code = PropertyCellStoreHandler(
             isolate(), store_target, Handle<JSGlobalObject>::cast(store_target),
             lookup->name(), cell, PropertyCellType::kConstant);
         cell->set_value(isolate()->heap()->the_hole_value());

src/ic/mips/handler-compiler-mips.cc

@@ -195,7 +195,8 @@ void NamedLoadHandlerCompiler::GenerateLoadFunctionPrototype(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/mips64/handler-compiler-mips64.cc

@@ -195,7 +195,8 @@ void NamedLoadHandlerCompiler::GenerateLoadFunctionPrototype(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/ppc/handler-compiler-ppc.cc

@@ -198,7 +198,8 @@ void NamedLoadHandlerCompiler::GenerateLoadFunctionPrototype(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/s390/handler-compiler-s390.cc

@@ -187,7 +187,8 @@ void NamedLoadHandlerCompiler::GenerateLoadFunctionPrototype(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/x64/handler-compiler-x64.cc

@@ -219,7 +219,8 @@ void PropertyHandlerCompiler::GenerateApiAccessorCall(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/ic/x87/handler-compiler-x87.cc

@@ -236,7 +236,8 @@ void PropertyHandlerCompiler::GenerateApiAccessorCall(
 void PropertyHandlerCompiler::GenerateCheckPropertyCell(
     MacroAssembler* masm, Handle<JSGlobalObject> global, Handle<Name> name,
     Register scratch, Label* miss) {
-  Handle<PropertyCell> cell = JSGlobalObject::EnsurePropertyCell(global, name);
+  Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+      global, name, PropertyCellType::kInvalidated);
   Isolate* isolate = masm->isolate();
   DCHECK(cell->value()->IsTheHole(isolate));
   Handle<WeakCell> weak_cell = isolate->factory()->NewWeakCell(cell);

src/lookup.cc

@@ -220,6 +220,16 @@ void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
     return;
   }
 
+  if (holder->IsJSGlobalObject()) {
+    Handle<GlobalDictionary> dictionary(holder->global_dictionary());
+    Handle<PropertyCell> cell(
+        PropertyCell::cast(dictionary->ValueAt(dictionary_entry())));
+    DCHECK(!cell->IsTheHole(isolate_));
+    property_details_ = cell->property_details();
+    PropertyCell::PrepareForValue(dictionary, dictionary_entry(), value,
+                                  property_details_);
+    return;
+  }
   if (!holder->HasFastProperties()) return;
 
   Handle<Map> old_map(holder->map(), isolate_);
@@ -252,20 +262,34 @@ void LookupIterator::ReconfigureDataProperty(Handle<Object> value,
     holder->GetElementsAccessor()->Reconfigure(holder, elements, number_, value,
                                                attributes);
     ReloadPropertyInformation<true>();
+  } else if (holder->HasFastProperties()) {
+    Handle<Map> old_map(holder->map(), isolate_);
+    Handle<Map> new_map = Map::ReconfigureExistingProperty(
+        old_map, descriptor_number(), i::kData, attributes);
+    new_map = Map::PrepareForDataProperty(new_map, descriptor_number(), value);
+    JSObject::MigrateToMap(holder, new_map);
+    ReloadPropertyInformation<false>();
   } else {
-    if (!holder->HasFastProperties()) {
-      PropertyDetails details(attributes, v8::internal::DATA, 0,
-                              PropertyCellType::kMutable);
-      JSObject::SetNormalizedProperty(holder, name(), value, details);
+    PropertyDetails details(attributes, v8::internal::DATA, 0,
+                            PropertyCellType::kMutable);
+    if (holder->IsJSGlobalObject()) {
+      Handle<GlobalDictionary> dictionary(holder->global_dictionary());
+
+      Handle<PropertyCell> cell = PropertyCell::PrepareForValue(
+          dictionary, dictionary_entry(), value, details);
+      cell->set_value(*value);
+      property_details_ = cell->property_details();
     } else {
-      Handle<Map> old_map(holder->map(), isolate_);
-      Handle<Map> new_map = Map::ReconfigureExistingProperty(
-          old_map, descriptor_number(), i::kData, attributes);
-      new_map =
-          Map::PrepareForDataProperty(new_map, descriptor_number(), value);
-      JSObject::MigrateToMap(holder, new_map);
+      Handle<NameDictionary> dictionary(holder->property_dictionary());
+      PropertyDetails original_details =
+          dictionary->DetailsAt(dictionary_entry());
+      int enumeration_index = original_details.dictionary_index();
+      DCHECK(enumeration_index > 0);
+      details = details.set_index(enumeration_index);
+      dictionary->SetEntry(dictionary_entry(), name(), value, details);
+      property_details_ = details;
     }
-    ReloadPropertyInformation<false>();
+    state_ = DATA;
   }
 
   WriteDataValue(value);
@@ -297,11 +321,31 @@ void LookupIterator::PrepareTransitionToDataProperty(
     state_ = TRANSITION;
     if (map->IsJSGlobalObjectMap()) {
       // Install a property cell.
-      auto cell = JSGlobalObject::EnsurePropertyCell(
-          Handle<JSGlobalObject>::cast(receiver), name());
+      Handle<JSGlobalObject> global = Handle<JSGlobalObject>::cast(receiver);
+      int entry;
+      Handle<PropertyCell> cell = JSGlobalObject::EnsureEmptyPropertyCell(
+          global, name(), PropertyCellType::kUninitialized, &entry);
+      Handle<GlobalDictionary> dictionary(global->global_dictionary(),
+                                          isolate_);
       DCHECK(cell->value()->IsTheHole(isolate_));
+      DCHECK(!value->IsTheHole(isolate_));
       transition_ = cell;
+      // Assign an enumeration index to the property and update
+      // SetNextEnumerationIndex.
+      int index = dictionary->NextEnumerationIndex();
+      dictionary->SetNextEnumerationIndex(index + 1);
+      property_details_ = PropertyDetails(attributes, i::DATA, index,
+                                          PropertyCellType::kUninitialized);
+      PropertyCellType new_type =
+          PropertyCell::UpdatedType(cell, value, property_details_);
+      property_details_ = property_details_.set_cell_type(new_type);
+      cell->set_property_details(property_details_);
+      number_ = entry;
+      has_property_ = true;
     } else {
+      // Don't set enumeration index (it will be set during value store).
+      property_details_ =
+          PropertyDetails(attributes, i::DATA, 0, PropertyCellType::kNoCell);
       transition_ = map;
     }
     return;
@@ -322,9 +366,11 @@ void LookupIterator::ApplyTransitionToDataProperty(Handle<JSObject> receiver) {
   DCHECK_EQ(TRANSITION, state_);
 
   DCHECK(receiver.is_identical_to(GetStoreTarget()));
-
-  if (receiver->IsJSGlobalObject()) return;
   holder_ = receiver;
+  if (receiver->IsJSGlobalObject()) {
+    state_ = DATA;
+    return;
+  }
   Handle<Map> transition = transition_map();
   bool simple_transition = transition->GetBackPointer() == receiver->map();
   JSObject::MigrateToMap(receiver, transition);
@@ -334,6 +380,20 @@ void LookupIterator::ApplyTransitionToDataProperty(Handle<JSObject> receiver) {
     number_ = static_cast<uint32_t>(number);
     property_details_ = transition->GetLastDescriptorDetails();
     state_ = DATA;
+  } else if (receiver->map()->is_dictionary_map()) {
+    Handle<NameDictionary> dictionary(receiver->property_dictionary(),
+                                      isolate_);
+    int entry;
+    dictionary = NameDictionary::Add(dictionary, name(),
+                                     isolate_->factory()->uninitialized_value(),
+                                     property_details_, &entry);
+    receiver->set_properties(*dictionary);
+    // Reload details containing proper enumeration index value.
+    property_details_ = dictionary->DetailsAt(entry);
+    number_ = entry;
+    has_property_ = true;
+    state_ = DATA;
+
   } else {
     ReloadPropertyInformation<false>();
   }
@@ -572,11 +632,10 @@ Handle<FieldType> LookupIterator::GetFieldType() const {
 
 Handle<PropertyCell> LookupIterator::GetPropertyCell() const {
   DCHECK(!IsElement());
-  Handle<JSObject> holder = GetHolder<JSObject>();
-  Handle<JSGlobalObject> global = Handle<JSGlobalObject>::cast(holder);
-  Object* value = global->global_dictionary()->ValueAt(dictionary_entry());
+  Handle<JSGlobalObject> holder = GetHolder<JSGlobalObject>();
+  Object* value = holder->global_dictionary()->ValueAt(dictionary_entry());
   DCHECK(value->IsPropertyCell());
-  return handle(PropertyCell::cast(value));
+  return handle(PropertyCell::cast(value), isolate_);
 }
 
 
@@ -608,13 +667,13 @@ void LookupIterator::WriteDataValue(Handle<Object> value) {
       DCHECK_EQ(v8::internal::DATA_CONSTANT, property_details_.type());
     }
   } else if (holder->IsJSGlobalObject()) {
-    Handle<GlobalDictionary> property_dictionary =
-        handle(JSObject::cast(*holder)->global_dictionary());
-    PropertyCell::UpdateCell(property_dictionary, dictionary_entry(), value,
-                             property_details_);
+    GlobalDictionary* dictionary = JSObject::cast(*holder)->global_dictionary();
+    Object* cell = dictionary->ValueAt(dictionary_entry());
+    DCHECK(cell->IsPropertyCell());
+    PropertyCell::cast(cell)->set_value(*value);
   } else {
-    NameDictionary* property_dictionary = holder->property_dictionary();
-    property_dictionary->ValueAtPut(dictionary_entry(), *value);
+    NameDictionary* dictionary = holder->property_dictionary();
+    dictionary->ValueAtPut(dictionary_entry(), *value);
   }
 }
 

src/lookup.h

@@ -190,6 +190,10 @@ class LookupIterator final BASE_EMBEDDED {
     DCHECK_EQ(TRANSITION, state_);
     return Handle<Map>::cast(transition_);
   }
+  Handle<PropertyCell> transition_cell() const {
+    DCHECK_EQ(TRANSITION, state_);
+    return Handle<PropertyCell>::cast(transition_);
+  }
   template <class T>
   Handle<T> GetHolder() const {
     DCHECK(IsFound());

src/objects-printer.cc

@@ -995,6 +995,46 @@ void PropertyCell::PropertyCellPrint(std::ostream& os) {  // NOLINT
   HeapObject::PrintHeader(os, "PropertyCell");
   os << "\n - value: " << Brief(value());
   os << "\n - details: " << property_details();
+  PropertyCellType cell_type = property_details().cell_type();
+  os << "\n - cell_type: ";
+  if (value()->IsTheHole(GetIsolate())) {
+    switch (cell_type) {
+      case PropertyCellType::kUninitialized:
+        os << "Uninitialized";
+        break;
+      case PropertyCellType::kInvalidated:
+        os << "Invalidated";
+        break;
+      default:
+        os << "??? " << static_cast<int>(cell_type);
+        break;
+    }
+  } else {
+    switch (cell_type) {
+      case PropertyCellType::kUndefined:
+        os << "Undefined";
+        break;
+      case PropertyCellType::kConstant:
+        os << "Constant";
+        break;
+      case PropertyCellType::kConstantType:
+        os << "ConstantType"
+           << " (";
+        switch (GetConstantType()) {
+          case PropertyCellConstantType::kSmi:
+            os << "Smi";
+            break;
+          case PropertyCellConstantType::kStableMap:
+            os << "StableMap";
+            break;
+        }
+        os << ")";
+        break;
+      case PropertyCellType::kMutable:
+        os << "Mutable";
+        break;
+    }
+  }
   os << "\n";
 }
 

src/objects.h

@@ -2483,12 +2483,6 @@ class JSObject: public JSReceiver {
   MUST_USE_RESULT static Maybe<bool> SetPropertyWithFailedAccessCheck(
       LookupIterator* it, Handle<Object> value, ShouldThrow should_throw);
 
-  // Add a property to a slow-case object.
-  static void AddSlowProperty(Handle<JSObject> object,
-                              Handle<Name> name,
-                              Handle<Object> value,
-                              PropertyAttributes attributes);
-
   MUST_USE_RESULT static Maybe<bool> DeletePropertyWithInterceptor(
       LookupIterator* it, ShouldThrow should_throw);
 
@@ -3518,11 +3512,10 @@ class Dictionary: public HashTable<Derived, Shape, Key> {
                        Handle<Object> value,
                        PropertyDetails details);
 
-  MUST_USE_RESULT static Handle<Derived> Add(
-      Handle<Derived> dictionary,
-      Key key,
-      Handle<Object> value,
-      PropertyDetails details);
+  MUST_USE_RESULT static Handle<Derived> Add(Handle<Derived> dictionary,
+                                             Key key, Handle<Object> value,
+                                             PropertyDetails details,
+                                             int* entry_out = nullptr);
 
   // Returns iteration indices array for the |dictionary|.
   // Values are direct indices in the |HashTable| array.
@@ -3536,13 +3529,9 @@ class Dictionary: public HashTable<Derived, Shape, Key> {
       Key key,
       Handle<Object> value);
 
-  // Add entry to dictionary.
-  static void AddEntry(
-      Handle<Derived> dictionary,
-      Key key,
-      Handle<Object> value,
-      PropertyDetails details,
-      uint32_t hash);
+  // Add entry to dictionary. Returns entry value.
+  static int AddEntry(Handle<Derived> dictionary, Key key, Handle<Object> value,
+                      PropertyDetails details, uint32_t hash);
 
   // Generate new enumeration indices to avoid enumeration index overflow.
   // Returns iteration indices array for the |dictionary|.
@@ -7870,8 +7859,9 @@ class JSGlobalObject : public JSObject {
   static void InvalidatePropertyCell(Handle<JSGlobalObject> object,
                                      Handle<Name> name);
   // Ensure that the global object has a cell for the given property name.
-  static Handle<PropertyCell> EnsurePropertyCell(Handle<JSGlobalObject> global,
-                                                 Handle<Name> name);
+  static Handle<PropertyCell> EnsureEmptyPropertyCell(
+      Handle<JSGlobalObject> global, Handle<Name> name,
+      PropertyCellType cell_type, int* entry_out = nullptr);
 
   DECLARE_CAST(JSGlobalObject)
 
@@ -9721,8 +9711,12 @@ class PropertyCell : public HeapObject {
   static PropertyCellType UpdatedType(Handle<PropertyCell> cell,
                                       Handle<Object> value,
                                       PropertyDetails details);
-  static void UpdateCell(Handle<GlobalDictionary> dictionary, int entry,
-                         Handle<Object> value, PropertyDetails details);
+  // Prepares property cell at given entry for receiving given value.
+  // As a result the old cell could be invalidated and/or dependent code could
+  // be deoptimized. Returns the prepared property cell.
+  static Handle<PropertyCell> PrepareForValue(
+      Handle<GlobalDictionary> dictionary, int entry, Handle<Object> value,
+      PropertyDetails details);
 
   static Handle<PropertyCell> InvalidateEntry(
       Handle<GlobalDictionary> dictionary, int entry);

src/property-details.h

@@ -209,7 +209,6 @@ static const int kMaxNumberOfDescriptors =
 static const int kInvalidEnumCacheSentinel =
     (1 << kDescriptorIndexBitCount) - 1;
 
-
 enum class PropertyCellType {
   // Meaningful when a property cell does not contain the hole.
   kUndefined,     // The PREMONOMORPHIC of property cells.
@@ -219,13 +218,13 @@ enum class PropertyCellType {
 
   // Meaningful when a property cell contains the hole.
   kUninitialized = kUndefined,  // Cell has never been initialized.
-  kInvalidated = kConstant,     // Cell has been deleted or invalidated.
+  kInvalidated = kConstant,     // Cell has been deleted, invalidated or never
+                                // existed.
 
   // For dictionaries not holding cells.
   kNoCell = kMutable,
 };
 
-
 enum class PropertyCellConstantType {
   kSmi,
   kStableMap,
@@ -265,8 +264,9 @@ class PropertyDetails BASE_EMBEDDED {
              FieldIndexField::encode(field_index);
   }
 
-  static PropertyDetails Empty() {
-    return PropertyDetails(NONE, DATA, 0, PropertyCellType::kNoCell);
+  static PropertyDetails Empty(
+      PropertyCellType cell_type = PropertyCellType::kNoCell) {
+    return PropertyDetails(NONE, DATA, 0, cell_type);
   }
 
   int pointer() const { return DescriptorPointer::decode(value_); }