Revert of [ic] Support data handlers that represent simple field stores....

Revert of [ic] Support data handlers that represent simple field stores. (patchset #2 id:40001 of https://codereview.chromium.org/2438553003/ )

Reason for revert:
http://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/9306 "V8 Mac GC Stress"

Original issue's description:
> [ic] Support data handlers that represent simple field stores.
>
> BUG=
>
> Review-Url: https://codereview.chromium.org/2438553003
> Cr-Commit-Position: refs/heads/master@{#40503}

TBR=jkummerow@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review-Url: https://chromiumcodereview.appspot.com/2442523003
Cr-Commit-Position: refs/heads/master@{#40512}

src/code-stub-assembler.cc

@@ -5707,125 +5707,6 @@ void CodeStubAssembler::KeyedLoadICGeneric(const LoadICParameters* p) {
   }
 }
 
-void CodeStubAssembler::HandleStoreFieldAndReturn(
-    Node* handler_word, Node* holder, Representation representation,
-    Node* value, bool transition_to_field, Label* miss) {
-  Node* prepared_value = PrepareValueForWrite(value, representation, miss);
-
-  Node* offset = DecodeWord<StoreHandler::FieldOffsetBits>(handler_word);
-  Label if_inobject(this), if_out_of_object(this);
-  Branch(IsSetWord<StoreHandler::IsInobjectBits>(handler_word), &if_inobject,
-         &if_out_of_object);
-
-  Bind(&if_inobject);
-  {
-    StoreNamedField(holder, offset, true, representation, prepared_value,
-                    transition_to_field);
-    Return(value);
-  }
-
-  Bind(&if_out_of_object);
-  {
-    StoreNamedField(holder, offset, false, representation, prepared_value,
-                    transition_to_field);
-    Return(value);
-  }
-}
-
-void CodeStubAssembler::HandleStoreICSmiHandlerCase(Node* handler_word,
-                                                    Node* holder, Node* value,
-                                                    bool transition_to_field,
-                                                    Label* miss) {
-  Comment(transition_to_field ? "transitioning field store" : "field store");
-
-  Node* field_representation =
-      DecodeWord<StoreHandler::FieldRepresentationBits>(handler_word);
-
-  Label if_smi_field(this), if_double_field(this), if_heap_object_field(this),
-      if_tagged_field(this);
-
-  GotoIf(WordEqual(field_representation, IntPtrConstant(StoreHandler::kTagged)),
-         &if_tagged_field);
-  GotoIf(WordEqual(field_representation,
-                   IntPtrConstant(StoreHandler::kHeapObject)),
-         &if_heap_object_field);
-  GotoIf(WordEqual(field_representation, IntPtrConstant(StoreHandler::kDouble)),
-         &if_double_field);
-  CSA_ASSERT(
-      WordEqual(field_representation, IntPtrConstant(StoreHandler::kSmi)));
-  Goto(&if_smi_field);
-
-  Bind(&if_tagged_field);
-  {
-    Comment("store tagged field");
-    HandleStoreFieldAndReturn(handler_word, holder, Representation::Tagged(),
-                              value, transition_to_field, miss);
-  }
-
-  Bind(&if_double_field);
-  {
-    Comment("store double field");
-    HandleStoreFieldAndReturn(handler_word, holder, Representation::Double(),
-                              value, transition_to_field, miss);
-  }
-
-  Bind(&if_heap_object_field);
-  {
-    Comment("store heap object field");
-    // Generate full field type check here and then store value as Tagged.
-    Node* prepared_value =
-        PrepareValueForWrite(value, Representation::HeapObject(), miss);
-    Node* value_index_in_descriptor =
-        DecodeWord<StoreHandler::DescriptorValueIndexBits>(handler_word);
-    Node* descriptors = LoadMapDescriptors(LoadMap(holder));
-    Node* maybe_field_type = LoadFixedArrayElement(
-        descriptors, value_index_in_descriptor, 0, INTPTR_PARAMETERS);
-    Label do_store(this);
-    GotoIf(TaggedIsSmi(maybe_field_type), &do_store);
-    // Check that value type matches the field type.
-    {
-      Node* field_type = LoadWeakCellValue(maybe_field_type, miss);
-      Branch(WordEqual(LoadMap(prepared_value), field_type), &do_store, miss);
-    }
-    Bind(&do_store);
-    HandleStoreFieldAndReturn(handler_word, holder, Representation::Tagged(),
-                              prepared_value, transition_to_field, miss);
-  }
-
-  Bind(&if_smi_field);
-  {
-    Comment("store smi field");
-    HandleStoreFieldAndReturn(handler_word, holder, Representation::Smi(),
-                              value, transition_to_field, miss);
-  }
-}
-
-void CodeStubAssembler::HandleStoreICHandlerCase(const StoreICParameters* p,
-                                                 Node* handler, Label* miss) {
-  Label if_smi_handler(this), call_handler(this);
-
-  Branch(TaggedIsSmi(handler), &if_smi_handler, &call_handler);
-
-  // |handler| is a Smi, encoding what to do. See SmiHandler methods
-  // for the encoding format.
-  Bind(&if_smi_handler);
-  {
-    Node* holder = p->receiver;
-    Node* handler_word = SmiUntag(handler);
-
-    // Handle non-transitioning stores.
-    HandleStoreICSmiHandlerCase(handler_word, holder, p->value, false, miss);
-  }
-
-  // |handler| is a heap object. Must be code, call it.
-  Bind(&call_handler);
-  {
-    StoreWithVectorDescriptor descriptor(isolate());
-    TailCallStub(descriptor, handler, p->context, p->receiver, p->name,
-                 p->value, p->slot, p->vector);
-  }
-}
-
 void CodeStubAssembler::StoreIC(const StoreICParameters* p) {
   Variable var_handler(this, MachineRepresentation::kTagged);
   // TODO(ishell): defer blocks when it works.
@@ -5842,7 +5723,9 @@ void CodeStubAssembler::StoreIC(const StoreICParameters* p) {
   Bind(&if_handler);
   {
     Comment("StoreIC_if_handler");
-    HandleStoreICHandlerCase(p, var_handler.value(), &miss);
+    StoreWithVectorDescriptor descriptor(isolate());
+    TailCallStub(descriptor, var_handler.value(), p->context, p->receiver,
+                 p->name, p->value, p->slot, p->vector);
   }
 
   Bind(&try_polymorphic);
@@ -5876,9 +5759,6 @@ void CodeStubAssembler::StoreIC(const StoreICParameters* p) {
 void CodeStubAssembler::KeyedStoreIC(const StoreICParameters* p,
                                      LanguageMode language_mode) {
   Variable var_handler(this, MachineRepresentation::kTagged);
-  // This is to make |miss| label see the var_handler bound on all paths.
-  var_handler.Bind(IntPtrConstant(0));
-
   // TODO(ishell): defer blocks when it works.
   Label if_handler(this, &var_handler), try_polymorphic(this),
       try_megamorphic(this /*, Label::kDeferred*/),
@@ -5894,7 +5774,9 @@ void CodeStubAssembler::KeyedStoreIC(const StoreICParameters* p,
   Bind(&if_handler);
   {
     Comment("KeyedStoreIC_if_handler");
-    HandleStoreICHandlerCase(p, var_handler.value(), &miss);
+    StoreWithVectorDescriptor descriptor(isolate());
+    TailCallStub(descriptor, var_handler.value(), p->context, p->receiver,
+                 p->name, p->value, p->slot, p->vector);
   }
 
   Bind(&try_polymorphic);

src/code-stub-assembler.h

@@ -1077,21 +1077,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   void HandleLoadICHandlerCase(
       const LoadICParameters* p, compiler::Node* handler, Label* miss,
       ElementSupport support_elements = kOnlyProperties);
-
-  void HandleStoreFieldAndReturn(compiler::Node* handler_word,
-                                 compiler::Node* holder,
-                                 Representation representation,
-                                 compiler::Node* value,
-                                 bool transition_to_field, Label* miss);
-
-  void HandleStoreICSmiHandlerCase(compiler::Node* handler_word,
-                                   compiler::Node* holder,
-                                   compiler::Node* value,
-                                   bool transition_to_field, Label* miss);
-
-  void HandleStoreICHandlerCase(const StoreICParameters* p,
-                                compiler::Node* handler, Label* miss);
-
   compiler::Node* TryToIntptr(compiler::Node* key, Label* miss);
   void EmitFastElementsBoundsCheck(compiler::Node* object,
                                    compiler::Node* elements,

src/counters.h

@@ -749,7 +749,6 @@ class RuntimeCallTimer {
   V(StoreIC_SlowStub)                           \
   V(StoreIC_StoreCallback)                      \
   V(StoreIC_StoreField)                         \
-  V(StoreIC_StoreFieldDH)                       \
   V(StoreIC_StoreFieldStub)                     \
   V(StoreIC_StoreGlobal)                        \
   V(StoreIC_StoreGlobalTransition)              \

src/flag-definitions.h

@@ -795,8 +795,6 @@ DEFINE_BOOL(use_ic, true, "use inline caching")
 DEFINE_BOOL(trace_ic, false, "trace inline cache state transitions")
 DEFINE_BOOL_READONLY(tf_load_ic_stub, true, "use TF LoadIC stub")
 DEFINE_BOOL(tf_store_ic_stub, true, "use TF StoreIC stub")
-DEFINE_BOOL(store_ic_smi_handlers, true, "use data based StoreIC handlers")
-DEFINE_IMPLICATION(store_ic_smi_handlers, tf_store_ic_stub)
 
 // macro-assembler-ia32.cc
 DEFINE_BOOL(native_code_counters, false,

src/ic/handler-configuration-inl.h

@@ -40,37 +40,6 @@ Handle<Object> LoadHandler::LoadElement(Isolate* isolate,
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Object> StoreHandler::StoreField(Isolate* isolate, int descriptor,
-                                        FieldIndex field_index,
-                                        Representation representation) {
-  StoreHandler::FieldRepresentation field_rep;
-  switch (representation.kind()) {
-    case Representation::kSmi:
-      field_rep = StoreHandler::kSmi;
-      break;
-    case Representation::kDouble:
-      field_rep = StoreHandler::kDouble;
-      break;
-    case Representation::kHeapObject:
-      field_rep = StoreHandler::kHeapObject;
-      break;
-    case Representation::kTagged:
-      field_rep = StoreHandler::kTagged;
-      break;
-    default:
-      UNREACHABLE();
-      return Handle<Object>::null();
-  }
-  int value_index = DescriptorArray::ToValueIndex(descriptor);
-
-  int config = StoreHandler::KindBits::encode(StoreHandler::kForFields) |
-               StoreHandler::IsInobjectBits::encode(field_index.is_inobject()) |
-               StoreHandler::FieldRepresentationBits::encode(field_rep) |
-               StoreHandler::DescriptorValueIndexBits::encode(value_index) |
-               StoreHandler::FieldOffsetBits::encode(field_index.offset());
-  return handle(Smi::FromInt(config), isolate);
-}
-
 }  // namespace internal
 }  // namespace v8
 

src/ic/handler-configuration.h

@@ -66,37 +66,6 @@ class LoadHandler {
                                            bool is_js_array);
 };
 
-// A set of bit fields representing Smi handlers for stores.
-class StoreHandler {
- public:
-  enum Kind { kForElements, kForFields };
-  class KindBits : public BitField<Kind, 0, 1> {};
-
-  enum FieldRepresentation { kSmi, kDouble, kHeapObject, kTagged };
-
-  //
-  // Encoding when KindBits contains kForFields.
-  //
-  class IsInobjectBits : public BitField<bool, KindBits::kNext, 1> {};
-  class FieldRepresentationBits
-      : public BitField<FieldRepresentation, IsInobjectBits::kNext, 2> {};
-  // +2 here is because each descriptor entry occupies 3 slots in array.
-  class DescriptorValueIndexBits
-      : public BitField<unsigned, FieldRepresentationBits::kNext,
-                        kDescriptorIndexBitCount + 2> {};
-  // +1 here is to cover all possible JSObject header sizes.
-  class FieldOffsetBits
-      : public BitField<unsigned, DescriptorValueIndexBits::kNext,
-                        kDescriptorIndexBitCount + 1 + kPointerSizeLog2> {};
-  // Make sure we don't overflow the smi.
-  STATIC_ASSERT(FieldOffsetBits::kNext <= kSmiValueSize);
-
-  // Creates a Smi-handler for storing a field to fast object.
-  static inline Handle<Object> StoreField(Isolate* isolate, int descriptor,
-                                          FieldIndex field_index,
-                                          Representation representation);
-};
-
 }  // namespace internal
 }  // namespace v8
 

src/ic/ic.cc

@@ -566,11 +566,11 @@ void IC::ConfigureVectorState(Handle<Name> name, Handle<Map> map,
     nexus->ConfigureMonomorphic(name, map, handler);
   } else if (kind() == Code::STORE_IC) {
     StoreICNexus* nexus = casted_nexus<StoreICNexus>();
-    nexus->ConfigureMonomorphic(map, handler);
+    nexus->ConfigureMonomorphic(map, Handle<Code>::cast(handler));
   } else {
     DCHECK(kind() == Code::KEYED_STORE_IC);
     KeyedStoreICNexus* nexus = casted_nexus<KeyedStoreICNexus>();
-    nexus->ConfigureMonomorphic(name, map, handler);
+    nexus->ConfigureMonomorphic(name, map, Handle<Code>::cast(handler));
   }
 
   vector_set_ = true;
@@ -789,10 +789,8 @@ bool IC::IsTransitionOfMonomorphicTarget(Map* source_map, Map* target_map) {
 void IC::PatchCache(Handle<Name> name, Handle<Object> handler) {
   DCHECK(IsHandler(*handler));
   // Currently only LoadIC and KeyedLoadIC support non-code handlers.
-  DCHECK_IMPLIES(!handler->IsCode(), kind() == Code::LOAD_IC ||
-                                         kind() == Code::KEYED_LOAD_IC ||
-                                         kind() == Code::STORE_IC ||
-                                         kind() == Code::KEYED_STORE_IC);
+  DCHECK_IMPLIES(!handler->IsCode(),
+                 kind() == Code::LOAD_IC || kind() == Code::KEYED_LOAD_IC);
   switch (state()) {
     case UNINITIALIZED:
     case PREMONOMORPHIC:
@@ -1011,37 +1009,6 @@ StubCache* IC::stub_cache() {
 }
 
 void IC::UpdateMegamorphicCache(Map* map, Name* name, Object* handler) {
-  if (FLAG_store_ic_smi_handlers && handler->IsSmi() &&
-      (kind() == Code::STORE_IC || kind() == Code::KEYED_STORE_IC)) {
-    // TODO(ishell, jkummerow): Implement data handlers support in
-    // KeyedStoreIC_Megamorphic.
-    Handle<Map> map_handle(map, isolate());
-    Handle<Name> name_handle(name, isolate());
-    int config = Smi::cast(handler)->value();
-    int value_index = StoreHandler::DescriptorValueIndexBits::decode(config);
-    int descriptor = (value_index - DescriptorArray::kDescriptorValue -
-                      DescriptorArray::kFirstIndex) /
-                     DescriptorArray::kDescriptorSize;
-    if (map->instance_descriptors()->length()) {
-      PropertyDetails details =
-          map->instance_descriptors()->GetDetails(descriptor);
-      DCHECK_EQ(DATA, details.type());
-      DCHECK_EQ(name, map->instance_descriptors()->GetKey(descriptor));
-      Representation representation = details.representation();
-      FieldIndex index = FieldIndex::ForDescriptor(map, descriptor);
-      TRACE_HANDLER_STATS(isolate(), StoreIC_StoreFieldStub);
-      StoreFieldStub stub(isolate(), index, representation);
-      handler = *stub.GetCode();
-    } else {
-      // It must be a prototype map that some prototype used to have. This map
-      // check will never succeed so write a dummy smi to the cache.
-      DCHECK(!map->is_dictionary_map());
-      DCHECK(map->is_prototype_map());
-      handler = Smi::FromInt(1);
-    }
-    stub_cache()->Set(*name_handle, *map_handle, handler);
-    return;
-  }
   stub_cache()->Set(name, map, handler);
 }
 
@@ -1673,10 +1640,10 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
   if (!use_ic) {
     TRACE_GENERIC_IC(isolate(), "StoreIC", "LookupForWrite said 'false'");
   }
-  Handle<Object> handler = use_ic ? ComputeHandler(lookup, value)
-                                  : Handle<Object>::cast(slow_stub());
+  Handle<Code> code =
+      use_ic ? Handle<Code>::cast(ComputeHandler(lookup, value)) : slow_stub();
 
-  PatchCache(lookup->name(), handler);
+  PatchCache(lookup->name(), code);
   TRACE_IC("StoreIC", lookup->name());
 }
 
@@ -1795,25 +1762,17 @@ Handle<Object> StoreIC::GetMapIndependentHandler(LookupIterator* lookup) {
 
       // -------------- Fields --------------
       if (lookup->property_details().type() == DATA) {
-        if (FLAG_store_ic_smi_handlers) {
-          TRACE_HANDLER_STATS(isolate(), StoreIC_StoreFieldDH);
-          int descriptor = lookup->GetFieldDescriptorIndex();
-          FieldIndex index = lookup->GetFieldIndex();
-          return StoreHandler::StoreField(isolate(), descriptor, index,
-                                          lookup->representation());
-        } else {
-          bool use_stub = true;
-          if (lookup->representation().IsHeapObject()) {
-            // Only use a generic stub if no types need to be tracked.
-            Handle<FieldType> field_type = lookup->GetFieldType();
-            use_stub = !field_type->IsClass();
-          }
-          if (use_stub) {
-            TRACE_HANDLER_STATS(isolate(), StoreIC_StoreFieldStub);
-            StoreFieldStub stub(isolate(), lookup->GetFieldIndex(),
-                                lookup->representation());
-            return stub.GetCode();
-          }
+        bool use_stub = true;
+        if (lookup->representation().IsHeapObject()) {
+          // Only use a generic stub if no types need to be tracked.
+          Handle<FieldType> field_type = lookup->GetFieldType();
+          use_stub = !field_type->IsClass();
+        }
+        if (use_stub) {
+          TRACE_HANDLER_STATS(isolate(), StoreIC_StoreFieldStub);
+          StoreFieldStub stub(isolate(), lookup->GetFieldIndex(),
+                              lookup->representation());
+          return stub.GetCode();
         }
         break;  // Custom-compiled handler.
       }
@@ -1931,7 +1890,6 @@ Handle<Code> StoreIC::CompileHandler(LookupIterator* lookup,
 #ifdef DEBUG
         bool use_stub = true;
         if (lookup->representation().IsHeapObject()) {
-          DCHECK(!FLAG_store_ic_smi_handlers);
           // Only use a generic stub if no types need to be tracked.
           Handle<FieldType> field_type = lookup->GetFieldType();
           use_stub = !field_type->IsClass();

src/lookup.cc

@@ -593,12 +593,6 @@ Handle<Object> LookupIterator::FetchValue() const {
   return handle(result, isolate_);
 }
 
-int LookupIterator::GetFieldDescriptorIndex() const {
-  DCHECK(has_property_);
-  DCHECK(holder_->HasFastProperties());
-  DCHECK_EQ(v8::internal::DATA, property_details_.type());
-  return descriptor_number();
-}
 
 int LookupIterator::GetAccessorIndex() const {
   DCHECK(has_property_);

src/lookup.h

@@ -238,7 +238,6 @@ class V8_EXPORT_PRIVATE LookupIterator final BASE_EMBEDDED {
   }
   FieldIndex GetFieldIndex() const;
   Handle<FieldType> GetFieldType() const;
-  int GetFieldDescriptorIndex() const;
   int GetAccessorIndex() const;
   int GetConstantIndex() const;
   Handle<PropertyCell> GetPropertyCell() const;

src/type-feedback-vector.cc

@@ -733,16 +733,18 @@ void KeyedLoadICNexus::ConfigureMonomorphic(Handle<Name> name,
   }
 }
 
+
 void StoreICNexus::ConfigureMonomorphic(Handle<Map> receiver_map,
-                                        Handle<Object> handler) {
+                                        Handle<Code> handler) {
   Handle<WeakCell> cell = Map::WeakCellForMap(receiver_map);
   SetFeedback(*cell);
   SetFeedbackExtra(*handler);
 }
 
+
 void KeyedStoreICNexus::ConfigureMonomorphic(Handle<Name> name,
                                              Handle<Map> receiver_map,
-                                             Handle<Object> handler) {
+                                             Handle<Code> handler) {
   Handle<WeakCell> cell = Map::WeakCellForMap(receiver_map);
   if (name.is_null()) {
     SetFeedback(*cell);

src/type-feedback-vector.h

@@ -609,7 +609,7 @@ class StoreICNexus : public FeedbackNexus {
 
   void Clear(Code* host);
 
-  void ConfigureMonomorphic(Handle<Map> receiver_map, Handle<Object> handler);
+  void ConfigureMonomorphic(Handle<Map> receiver_map, Handle<Code> handler);
 
   void ConfigurePolymorphic(MapHandleList* maps,
                             List<Handle<Object>>* handlers);
@@ -637,7 +637,7 @@ class KeyedStoreICNexus : public FeedbackNexus {
 
   // name can be a null handle for element loads.
   void ConfigureMonomorphic(Handle<Name> name, Handle<Map> receiver_map,
-                            Handle<Object> handler);
+                            Handle<Code> handler);
   // name can be null.
   void ConfigurePolymorphic(Handle<Name> name, MapHandleList* maps,
                             List<Handle<Object>>* handlers);