Reland "[ic] Migrate StoreGlobal to data handler"

The problem was that transitioning element stores had a similar shape to
the new StoreGlobal case.

The problem was fixed by https://chromium-review.googlesource.com/c/457341/

BUG=v8:5561

Change-Id: If996e9b37809ba8edf6dcb228b116b77021ce7bc
Reviewed-on: https://chromium-review.googlesource.com/457324Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43992}

src/code-stub-assembler.cc

@@ -2960,7 +2960,7 @@ Node* CodeStubAssembler::IsJSArray(Node* object) {
 }
 
 Node* CodeStubAssembler::IsWeakCell(Node* object) {
-  return HasInstanceType(object, WEAK_CELL_TYPE);
+  return IsWeakCellMap(LoadMap(object));
 }
 
 Node* CodeStubAssembler::IsBoolean(Node* object) {

src/code-stub-assembler.h

@@ -48,7 +48,8 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol };
   V(TrueValue, True)                                  \
   V(Tuple2Map, Tuple2Map)                             \
   V(Tuple3Map, Tuple3Map)                             \
-  V(UndefinedValue, Undefined)
+  V(UndefinedValue, Undefined)                        \
+  V(WeakCellMap, WeakCellMap)
 
 // Provides JavaScript-specific "macro-assembler" functionality on top of the
 // CodeAssembler. By factoring the JavaScript-isms out of the CodeAssembler,

src/code-stubs.cc

@@ -1490,88 +1490,6 @@ void SubStringStub::GenerateAssembly(
                             assembler.Parameter(Descriptor::kContext)));
 }
 
-void StoreGlobalStub::GenerateAssembly(
-    compiler::CodeAssemblerState* state) const {
-  typedef CodeStubAssembler::Label Label;
-  typedef compiler::Node Node;
-  CodeStubAssembler assembler(state);
-
-  assembler.Comment("StoreGlobalStub: cell_type=%d, constant_type=%d",
-                    cell_type(),
-                    PropertyCellType::kConstantType == cell_type()
-                        ? static_cast<int>(constant_type())
-                        : -1);
-
-  Node* receiver = assembler.Parameter(Descriptor::kReceiver);
-  Node* name = assembler.Parameter(Descriptor::kName);
-  Node* value = assembler.Parameter(Descriptor::kValue);
-  Node* slot = assembler.Parameter(Descriptor::kSlot);
-  Node* vector = assembler.Parameter(Descriptor::kVector);
-  Node* context = assembler.Parameter(Descriptor::kContext);
-
-  Label miss(&assembler);
-
-  Node* weak_cell = assembler.HeapConstant(isolate()->factory()->NewWeakCell(
-      StoreGlobalStub::property_cell_placeholder(isolate())));
-  Node* cell = assembler.LoadWeakCellValue(weak_cell);
-  assembler.GotoIf(assembler.TaggedIsSmi(cell), &miss);
-
-  // Load the payload of the global parameter cell. A hole indicates that the
-  // cell has been invalidated and that the store must be handled by the
-  // runtime.
-  Node* cell_contents =
-      assembler.LoadObjectField(cell, PropertyCell::kValueOffset);
-
-  PropertyCellType cell_type = this->cell_type();
-  if (cell_type == PropertyCellType::kConstant ||
-      cell_type == PropertyCellType::kUndefined) {
-    // This is always valid for all states a cell can be in.
-    assembler.GotoIf(assembler.WordNotEqual(cell_contents, value), &miss);
-  } else {
-    assembler.GotoIf(assembler.IsTheHole(cell_contents), &miss);
-
-    // When dealing with constant types, the type may be allowed to change, as
-    // long as optimized code remains valid.
-    bool value_is_smi = false;
-    if (cell_type == PropertyCellType::kConstantType) {
-      switch (constant_type()) {
-        case PropertyCellConstantType::kSmi:
-          assembler.GotoIfNot(assembler.TaggedIsSmi(value), &miss);
-          value_is_smi = true;
-          break;
-        case PropertyCellConstantType::kStableMap: {
-          // It is sufficient here to check that the value and cell contents
-          // have identical maps, no matter if they are stable or not or if they
-          // are the maps that were originally in the cell or not. If optimized
-          // code will deopt when a cell has a unstable map and if it has a
-          // dependency on a stable map, it will deopt if the map destabilizes.
-          assembler.GotoIf(assembler.TaggedIsSmi(value), &miss);
-          assembler.GotoIf(assembler.TaggedIsSmi(cell_contents), &miss);
-          Node* expected_map = assembler.LoadMap(cell_contents);
-          Node* map = assembler.LoadMap(value);
-          assembler.GotoIf(assembler.WordNotEqual(expected_map, map), &miss);
-          break;
-        }
-      }
-    }
-    if (value_is_smi) {
-      assembler.StoreObjectFieldNoWriteBarrier(cell, PropertyCell::kValueOffset,
-                                               value);
-    } else {
-      assembler.StoreObjectField(cell, PropertyCell::kValueOffset, value);
-    }
-  }
-
-  assembler.Return(value);
-
-  assembler.Bind(&miss);
-  {
-    assembler.Comment("Miss");
-    assembler.TailCallRuntime(Runtime::kStoreIC_Miss, context, value, slot,
-                              vector, receiver, name);
-  }
-}
-
 void KeyedLoadSloppyArgumentsStub::GenerateAssembly(
     compiler::CodeAssemblerState* state) const {
   typedef CodeStubAssembler::Label Label;

src/code-stubs.h

@@ -90,7 +90,6 @@ class Node;
   V(StringAdd)                                \
   V(GetProperty)                              \
   V(StoreFastElement)                         \
-  V(StoreGlobal)                              \
   V(StoreInterceptor)                         \
   V(LoadIndexedInterceptor)                   \
   V(GrowArrayElements)
@@ -895,49 +894,6 @@ class KeyedStoreSloppyArgumentsStub : public TurboFanCodeStub {
   DEFINE_TURBOFAN_CODE_STUB(KeyedStoreSloppyArguments, TurboFanCodeStub);
 };
 
-class StoreGlobalStub : public TurboFanCodeStub {
- public:
-  StoreGlobalStub(Isolate* isolate, PropertyCellType type,
-                  Maybe<PropertyCellConstantType> constant_type)
-      : TurboFanCodeStub(isolate) {
-    PropertyCellConstantType encoded_constant_type =
-        constant_type.FromMaybe(PropertyCellConstantType::kSmi);
-    minor_key_ = CellTypeBits::encode(type) |
-                 ConstantTypeBits::encode(encoded_constant_type);
-  }
-
-  Code::Kind GetCodeKind() const override { return Code::HANDLER; }
-  ExtraICState GetExtraICState() const override { return Code::STORE_IC; }
-
-  static Handle<HeapObject> property_cell_placeholder(Isolate* isolate) {
-    return isolate->factory()->uninitialized_value();
-  }
-
-  Handle<Code> GetCodeCopyFromTemplate(Handle<PropertyCell> cell) {
-    FindAndReplacePattern pattern;
-    pattern.Add(handle(property_cell_placeholder(isolate())->map()),
-                isolate()->factory()->NewWeakCell(cell));
-    return CodeStub::GetCodeCopy(pattern);
-  }
-
-  PropertyCellType cell_type() const {
-    return CellTypeBits::decode(minor_key_);
-  }
-
-  PropertyCellConstantType constant_type() const {
-    DCHECK(PropertyCellType::kConstantType == cell_type());
-    return ConstantTypeBits::decode(minor_key_);
-  }
-
- private:
-  class CellTypeBits : public BitField<PropertyCellType, 0, 2> {};
-  class ConstantTypeBits
-      : public BitField<PropertyCellConstantType, CellTypeBits::kNext, 2> {};
-
-  DEFINE_CALL_INTERFACE_DESCRIPTOR(StoreWithVector);
-  DEFINE_TURBOFAN_CODE_STUB(StoreGlobal, TurboFanCodeStub);
-};
-
 class CallApiCallbackStub : public PlatformCodeStub {
  public:
   static const int kArgBits = 3;

src/counters.h

@@ -815,8 +815,8 @@ class RuntimeCallTimer final {
   V(StoreIC_StoreField)                          \
   V(StoreIC_StoreFieldDH)                        \
   V(StoreIC_StoreFieldStub)                      \
-  V(StoreIC_StoreGlobal)                         \
-  V(StoreIC_StoreGlobalTransition)               \
+  V(StoreIC_StoreGlobalDH)                       \
+  V(StoreIC_StoreGlobalTransitionDH)             \
   V(StoreIC_StoreInterceptorStub)                \
   V(StoreIC_StoreNormalDH)                       \
   V(StoreIC_StoreScriptContextFieldStub)         \

src/ic/accessor-assembler.cc

@@ -580,7 +580,8 @@ void AccessorAssembler::HandleStoreICHandlerCase(
     const StoreICParameters* p, Node* handler, Label* miss,
     ElementSupport support_elements) {
   Label if_smi_handler(this), if_nonsmi_handler(this);
-  Label if_proto_handler(this), if_element_handler(this), call_handler(this);
+  Label if_proto_handler(this), if_element_handler(this), call_handler(this),
+      store_global(this);
 
   Branch(TaggedIsSmi(handler), &if_smi_handler, &if_nonsmi_handler);
 
@@ -628,6 +629,7 @@ void AccessorAssembler::HandleStoreICHandlerCase(
     if (support_elements == kSupportElements) {
       GotoIf(IsTuple2Map(handler_map), &if_element_handler);
     }
+    GotoIf(IsWeakCellMap(handler_map), &store_global);
     Branch(IsCodeMap(handler_map), &call_handler, &if_proto_handler);
   }
 
@@ -646,6 +648,66 @@ void AccessorAssembler::HandleStoreICHandlerCase(
     TailCallStub(descriptor, handler, p->context, p->receiver, p->name,
                  p->value, p->slot, p->vector);
   }
+
+  Bind(&store_global);
+  {
+    Node* cell = LoadWeakCellValue(handler, miss);
+    CSA_ASSERT(this, IsPropertyCell(cell));
+
+    // Load the payload of the global parameter cell. A hole indicates that
+    // the cell has been invalidated and that the store must be handled by the
+    // runtime.
+    Node* cell_contents = LoadObjectField(cell, PropertyCell::kValueOffset);
+    Node* details =
+        LoadAndUntagToWord32ObjectField(cell, PropertyCell::kDetailsOffset);
+    Node* type = DecodeWord32<PropertyDetails::PropertyCellTypeField>(details);
+
+    Label constant(this), store(this), not_smi(this);
+
+    GotoIf(
+        Word32Equal(
+            type, Int32Constant(static_cast<int>(PropertyCellType::kConstant))),
+        &constant);
+
+    GotoIf(IsTheHole(cell_contents), miss);
+
+    GotoIf(
+        Word32Equal(
+            type, Int32Constant(static_cast<int>(PropertyCellType::kMutable))),
+        &store);
+    CSA_ASSERT(this,
+               Word32Or(Word32Equal(type,
+                                    Int32Constant(static_cast<int>(
+                                        PropertyCellType::kConstantType))),
+                        Word32Equal(type,
+                                    Int32Constant(static_cast<int>(
+                                        PropertyCellType::kUndefined)))));
+
+    GotoIfNot(TaggedIsSmi(cell_contents), &not_smi);
+    GotoIfNot(TaggedIsSmi(p->value), miss);
+    Goto(&store);
+
+    Bind(&not_smi);
+    {
+      GotoIf(TaggedIsSmi(p->value), miss);
+      Node* expected_map = LoadMap(cell_contents);
+      Node* map = LoadMap(p->value);
+      GotoIfNot(WordEqual(expected_map, map), miss);
+      Goto(&store);
+    }
+
+    Bind(&store);
+    {
+      StoreObjectField(cell, PropertyCell::kValueOffset, p->value);
+      Return(p->value);
+    }
+
+    Bind(&constant);
+    {
+      GotoIfNot(WordEqual(cell_contents, p->value), miss);
+      Return(p->value);
+    }
+  }
 }
 
 void AccessorAssembler::HandleStoreICElementHandlerCase(

src/ic/handler-configuration-inl.h

@@ -92,15 +92,15 @@ Handle<Smi> LoadHandler::LoadElement(Isolate* isolate,
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Object> StoreHandler::StoreNormal(Isolate* isolate) {
+Handle<Smi> StoreHandler::StoreNormal(Isolate* isolate) {
   int config = KindBits::encode(kStoreNormal);
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Object> StoreHandler::StoreField(Isolate* isolate, Kind kind,
-                                        int descriptor, FieldIndex field_index,
-                                        Representation representation,
-                                        bool extend_storage) {
+Handle<Smi> StoreHandler::StoreField(Isolate* isolate, Kind kind,
+                                     int descriptor, FieldIndex field_index,
+                                     Representation representation,
+                                     bool extend_storage) {
   StoreHandler::FieldRepresentation field_rep;
   switch (representation.kind()) {
     case Representation::kSmi:
@@ -117,7 +117,7 @@ Handle<Object> StoreHandler::StoreField(Isolate* isolate, Kind kind,
       break;
     default:
       UNREACHABLE();
-      return Handle<Object>::null();
+      return Handle<Smi>::null();
   }
 
   DCHECK(kind == kStoreField || kind == kTransitionToField ||
@@ -134,26 +134,26 @@ Handle<Object> StoreHandler::StoreField(Isolate* isolate, Kind kind,
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Object> StoreHandler::StoreField(Isolate* isolate, int descriptor,
-                                        FieldIndex field_index,
-                                        PropertyConstness constness,
-                                        Representation representation) {
+Handle<Smi> StoreHandler::StoreField(Isolate* isolate, int descriptor,
+                                     FieldIndex field_index,
+                                     PropertyConstness constness,
+                                     Representation representation) {
   DCHECK_IMPLIES(!FLAG_track_constant_fields, constness == kMutable);
   Kind kind = constness == kMutable ? kStoreField : kStoreConstField;
   return StoreField(isolate, kind, descriptor, field_index, representation,
                     false);
 }
 
-Handle<Object> StoreHandler::TransitionToField(Isolate* isolate, int descriptor,
-                                               FieldIndex field_index,
-                                               Representation representation,
-                                               bool extend_storage) {
+Handle<Smi> StoreHandler::TransitionToField(Isolate* isolate, int descriptor,
+                                            FieldIndex field_index,
+                                            Representation representation,
+                                            bool extend_storage) {
   return StoreField(isolate, kTransitionToField, descriptor, field_index,
                     representation, extend_storage);
 }
 
-Handle<Object> StoreHandler::TransitionToConstant(Isolate* isolate,
-                                                  int descriptor) {
+Handle<Smi> StoreHandler::TransitionToConstant(Isolate* isolate,
+                                               int descriptor) {
   DCHECK(!FLAG_track_constant_fields);
   int config =
       StoreHandler::KindBits::encode(StoreHandler::kTransitionToConstant) |

src/ic/handler-configuration.h

@@ -191,32 +191,30 @@ class StoreHandler {
   static const int kFirstPrototypeIndex = 3;
 
   // Creates a Smi-handler for storing a field to fast object.
-  static inline Handle<Object> StoreField(Isolate* isolate, int descriptor,
-                                          FieldIndex field_index,
-                                          PropertyConstness constness,
-                                          Representation representation);
+  static inline Handle<Smi> StoreField(Isolate* isolate, int descriptor,
+                                       FieldIndex field_index,
+                                       PropertyConstness constness,
+                                       Representation representation);
 
   // Creates a Smi-handler for storing a property to a slow object.
-  static inline Handle<Object> StoreNormal(Isolate* isolate);
+  static inline Handle<Smi> StoreNormal(Isolate* isolate);
 
   // Creates a Smi-handler for transitioning store to a field.
-  static inline Handle<Object> TransitionToField(Isolate* isolate,
-                                                 int descriptor,
-                                                 FieldIndex field_index,
-                                                 Representation representation,
-                                                 bool extend_storage);
+  static inline Handle<Smi> TransitionToField(Isolate* isolate, int descriptor,
+                                              FieldIndex field_index,
+                                              Representation representation,
+                                              bool extend_storage);
 
   // Creates a Smi-handler for transitioning store to a constant field (in this
   // case the only thing that needs to be done is an update of a map).
-  static inline Handle<Object> TransitionToConstant(Isolate* isolate,
-                                                    int descriptor);
+  static inline Handle<Smi> TransitionToConstant(Isolate* isolate,
+                                                 int descriptor);
 
  private:
-  static inline Handle<Object> StoreField(Isolate* isolate, Kind kind,
-                                          int descriptor,
-                                          FieldIndex field_index,
-                                          Representation representation,
-                                          bool extend_storage);
+  static inline Handle<Smi> StoreField(Isolate* isolate, Kind kind,
+                                       int descriptor, FieldIndex field_index,
+                                       Representation representation,
+                                       bool extend_storage);
 };
 
 }  // namespace internal

src/ic/ic-inl.h

@@ -86,7 +86,7 @@ Code* IC::target() const {
 
 bool IC::IsHandler(Object* object) {
   return (object->IsSmi() && (object != nullptr)) || object->IsTuple2() ||
-         object->IsTuple3() || object->IsFixedArray() ||
+         object->IsTuple3() || object->IsFixedArray() || object->IsWeakCell() ||
          (object->IsCode() && Code::cast(object)->is_handler());
 }
 

src/ic/ic.cc

@@ -1705,7 +1705,7 @@ Handle<Object> StoreIC::StoreTransition(Handle<Map> receiver_map,
       Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate());
   if (validity_cell.is_null()) {
     DCHECK_EQ(0, checks_count);
-    validity_cell = handle(Smi::FromInt(0), isolate());
+    validity_cell = handle(Smi::kZero, isolate());
   }
 
   Handle<WeakCell> transition_cell = Map::WeakCellForMap(transition);
@@ -1724,22 +1724,14 @@ Handle<Object> StoreIC::StoreTransition(Handle<Map> receiver_map,
   return handler_array;
 }
 
-static Handle<Code> PropertyCellStoreHandler(Isolate* isolate,
-                                             Handle<JSObject> store_target,
-                                             Handle<Name> name,
-                                             Handle<PropertyCell> cell,
-                                             PropertyCellType type) {
-  auto constant_type = Nothing<PropertyCellConstantType>();
-  if (type == PropertyCellType::kConstantType) {
-    constant_type = Just(cell->GetConstantType());
-  }
-  StoreGlobalStub stub(isolate, type, constant_type);
-  auto code = stub.GetCodeCopyFromTemplate(cell);
-  // TODO(verwaest): Move caching of these NORMAL stubs outside as well.
-  HeapObject::UpdateMapCodeCache(store_target, name, code);
-  return code;
+namespace {
+
+Handle<Object> StoreGlobal(Isolate* isolate, Handle<PropertyCell> cell) {
+  return isolate->factory()->NewWeakCell(cell);
 }
 
+}  // namespace
+
 Handle<Object> StoreIC::GetMapIndependentHandler(LookupIterator* lookup) {
   DCHECK_NE(LookupIterator::JSPROXY, lookup->state());
 
@@ -1752,7 +1744,8 @@ Handle<Object> StoreIC::GetMapIndependentHandler(LookupIterator* lookup) {
     case LookupIterator::TRANSITION: {
       auto store_target = lookup->GetStoreTarget();
       if (store_target->IsJSGlobalObject()) {
-        break;  // Custom-compiled handler.
+        TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobalTransitionDH);
+        return StoreGlobal(isolate(), lookup->transition_cell());
       }
       // Currently not handled by CompileStoreTransition.
       if (!holder->HasFastProperties()) {
@@ -1829,7 +1822,8 @@ Handle<Object> StoreIC::GetMapIndependentHandler(LookupIterator* lookup) {
       DCHECK_EQ(kData, lookup->property_details().kind());
       if (lookup->is_dictionary_holder()) {
         if (holder->IsJSGlobalObject()) {
-          break;  // Custom-compiled handler.
+          TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobalDH);
+          return StoreGlobal(isolate(), lookup->GetPropertyCell());
         }
         TRACE_HANDLER_STATS(isolate(), StoreIC_StoreNormalDH);
         DCHECK(holder.is_identical_to(receiver));
@@ -1877,24 +1871,6 @@ Handle<Object> StoreIC::CompileHandler(LookupIterator* lookup,
   DCHECK(!receiver->IsAccessCheckNeeded() || lookup->name()->IsPrivate());
 
   switch (lookup->state()) {
-    case LookupIterator::TRANSITION: {
-      auto store_target = lookup->GetStoreTarget();
-      if (store_target->IsJSGlobalObject()) {
-        TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobalTransition);
-        Handle<PropertyCell> cell = lookup->transition_cell();
-        cell->set_value(*value);
-        Handle<Code> code =
-            PropertyCellStoreHandler(isolate(), receiver, lookup->name(), cell,
-                                     PropertyCellType::kConstant);
-        cell->set_value(isolate()->heap()->the_hole_value());
-        return code;
-      }
-      UNREACHABLE();
-    }
-
-    case LookupIterator::INTERCEPTOR:
-      UNREACHABLE();
-
     case LookupIterator::ACCESSOR: {
       DCHECK(holder->HasFastProperties());
       Handle<Object> accessors = lookup->GetAccessors();
@@ -1940,20 +1916,9 @@ Handle<Object> StoreIC::CompileHandler(LookupIterator* lookup,
       }
     }
 
-    case LookupIterator::DATA: {
-      DCHECK(lookup->is_dictionary_holder());
-      DCHECK(holder->IsJSGlobalObject());
-      TRACE_HANDLER_STATS(isolate(), StoreIC_StoreGlobal);
-      DCHECK(holder.is_identical_to(receiver) ||
-             receiver->map()->prototype() == *holder);
-      auto cell = lookup->GetPropertyCell();
-      auto updated_type =
-          PropertyCell::UpdatedType(cell, value, lookup->property_details());
-      auto code = PropertyCellStoreHandler(isolate(), receiver,
-                                           lookup->name(), cell, updated_type);
-      return code;
-    }
-
+    case LookupIterator::DATA:
+    case LookupIterator::TRANSITION:
+    case LookupIterator::INTERCEPTOR:
     case LookupIterator::INTEGER_INDEXED_EXOTIC:
     case LookupIterator::ACCESS_CHECK:
     case LookupIterator::JSPROXY: