[ic] Create a new Smi handler for Stores involving interceptors.

based on dicussion at
docs.google.com/document/d/1UzCOai9H07fYcSaSqvF_H7BS2-sF5q91A4r9O1mRnHc/

Bug: v8:9305
Change-Id: I7464d4267b6465cc02bc27dffb602c8871d846f9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1696285
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63640}

src/builtins/builtins-definitions.h

@@ -230,7 +230,6 @@ namespace internal {
   TFH(StoreInArrayLiteralIC_Slow, StoreWithVector)                             \
   TFH(KeyedLoadIC_SloppyArguments, LoadWithVector)                             \
   TFH(LoadIndexedInterceptorIC, LoadWithVector)                                \
-  TFH(StoreInterceptorIC, StoreWithVector)                                     \
   TFH(KeyedStoreIC_SloppyArguments_Standard, StoreWithVector)                  \
   TFH(KeyedStoreIC_SloppyArguments_GrowNoTransitionHandleCOW, StoreWithVector) \
   TFH(KeyedStoreIC_SloppyArguments_NoTransitionIgnoreOOB, StoreWithVector)     \

src/builtins/builtins-handler-gen.cc

@@ -491,17 +491,6 @@ TF_BUILTIN(KeyedStoreIC_SloppyArguments_NoTransitionHandleCOW,
   Generate_KeyedStoreIC_SloppyArguments();
 }
 
-TF_BUILTIN(StoreInterceptorIC, CodeStubAssembler) {
-  Node* receiver = Parameter(Descriptor::kReceiver);
-  Node* name = Parameter(Descriptor::kName);
-  Node* value = Parameter(Descriptor::kValue);
-  Node* slot = Parameter(Descriptor::kSlot);
-  Node* vector = Parameter(Descriptor::kVector);
-  Node* context = Parameter(Descriptor::kContext);
-  TailCallRuntime(Runtime::kStorePropertyWithInterceptor, context, value, slot,
-                  vector, receiver, name);
-}
-
 TF_BUILTIN(LoadIndexedInterceptorIC, CodeStubAssembler) {
   Node* receiver = Parameter(Descriptor::kReceiver);
   Node* key = Parameter(Descriptor::kName);

src/ic/accessor-assembler.cc

@@ -951,10 +951,13 @@ void AccessorAssembler::HandleStoreICHandlerCase(
     Node* holder = p->receiver();
     TNode<IntPtrT> handler_word = SmiUntag(CAST(handler));
 
-    Label if_fast_smi(this), if_proxy(this);
+    Label if_fast_smi(this), if_proxy(this), if_interceptor(this),
+        if_slow(this);
 
     STATIC_ASSERT(StoreHandler::kGlobalProxy + 1 == StoreHandler::kNormal);
-    STATIC_ASSERT(StoreHandler::kNormal + 1 == StoreHandler::kProxy);
+    STATIC_ASSERT(StoreHandler::kNormal + 1 == StoreHandler::kInterceptor);
+    STATIC_ASSERT(StoreHandler::kInterceptor + 1 == StoreHandler::kSlow);
+    STATIC_ASSERT(StoreHandler::kSlow + 1 == StoreHandler::kProxy);
     STATIC_ASSERT(StoreHandler::kProxy + 1 == StoreHandler::kKindsNumber);
 
     TNode<UintPtrT> handler_kind =
@@ -964,6 +967,10 @@ void AccessorAssembler::HandleStoreICHandlerCase(
            &if_fast_smi);
     GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kProxy)),
            &if_proxy);
+    GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kInterceptor)),
+           &if_interceptor);
+    GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kSlow)),
+           &if_slow);
     CSA_ASSERT(this,
                WordEqual(handler_kind, IntPtrConstant(StoreHandler::kNormal)));
     TNode<NameDictionary> properties = CAST(LoadSlowProperties(holder));
@@ -1012,6 +1019,24 @@ void AccessorAssembler::HandleStoreICHandlerCase(
 
     BIND(&if_proxy);
     HandleStoreToProxy(p, holder, miss, support_elements);
+
+    BIND(&if_interceptor);
+    {
+      Comment("store_interceptor");
+      TailCallRuntime(Runtime::kStorePropertyWithInterceptor, p->context(),
+                      p->value(), p->slot(), p->vector(), p->receiver(),
+                      p->name());
+    }
+
+    BIND(&if_slow);
+    {
+      Comment("store_slow");
+      // The slow case calls into the runtime to complete the store without
+      // causing an IC miss that would otherwise cause a transition to the
+      // generic stub.
+      TailCallRuntime(Runtime::kKeyedStoreIC_Slow, p->context(), p->value(),
+                      p->receiver(), p->name());
+    }
   }
 
   BIND(&if_nonsmi_handler);
@@ -1455,7 +1480,8 @@ void AccessorAssembler::HandleStoreICProtoHandler(
 
   {
     Label if_add_normal(this), if_store_global_proxy(this), if_api_setter(this),
-        if_accessor(this), if_native_data_property(this);
+        if_accessor(this), if_native_data_property(this), if_slow(this),
+        if_interceptor(this);
 
     CSA_ASSERT(this, TaggedIsSmi(smi_handler));
     TNode<IntPtrT> handler_word = SmiUntag(smi_handler);
@@ -1482,6 +1508,12 @@ void AccessorAssembler::HandleStoreICProtoHandler(
     GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kApiSetter)),
            &if_api_setter);
 
+    GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kSlow)),
+           &if_slow);
+
+    GotoIf(WordEqual(handler_kind, IntPtrConstant(StoreHandler::kInterceptor)),
+           &if_interceptor);
+
     GotoIf(WordEqual(handler_kind,
                      IntPtrConstant(StoreHandler::kApiSetterHolderIsPrototype)),
            &if_api_setter);
@@ -1490,6 +1522,24 @@ void AccessorAssembler::HandleStoreICProtoHandler(
                WordEqual(handler_kind, IntPtrConstant(StoreHandler::kProxy)));
     HandleStoreToProxy(p, holder, miss, support_elements);
 
+    BIND(&if_slow);
+    {
+      Comment("store_slow");
+      // The slow case calls into the runtime to complete the store without
+      // causing an IC miss that would otherwise cause a transition to the
+      // generic stub.
+      TailCallRuntime(Runtime::kKeyedStoreIC_Slow, p->context(), p->value(),
+                      p->receiver(), p->name());
+    }
+
+    BIND(&if_interceptor);
+    {
+      Comment("store_interceptor");
+      TailCallRuntime(Runtime::kStorePropertyWithInterceptor, p->context(),
+                      p->value(), p->slot(), p->vector(), p->receiver(),
+                      p->name());
+    }
+
     BIND(&if_add_normal);
     {
       // This is a case of "transitioning store" to a dictionary mode object

src/ic/handler-configuration-inl.h

@@ -127,6 +127,16 @@ Handle<Smi> StoreHandler::StoreNormal(Isolate* isolate) {
   return handle(Smi::FromInt(config), isolate);
 }
 
+Handle<Smi> StoreHandler::StoreInterceptor(Isolate* isolate) {
+  int config = KindBits::encode(kInterceptor);
+  return handle(Smi::FromInt(config), isolate);
+}
+
+Handle<Smi> StoreHandler::StoreSlow(Isolate* isolate) {
+  int config = KindBits::encode(kSlow);
+  return handle(Smi::FromInt(config), isolate);
+}
+
 Handle<Smi> StoreHandler::StoreProxy(Isolate* isolate) {
   int config = KindBits::encode(kProxy);
   return handle(Smi::FromInt(config), isolate);

src/ic/handler-configuration.h

@@ -197,6 +197,8 @@ class StoreHandler final : public DataHandler {
     kApiSetterHolderIsPrototype,
     kGlobalProxy,
     kNormal,
+    kInterceptor,
+    kSlow,
     kProxy,
     kKindsNumber  // Keep last
   };
@@ -283,6 +285,12 @@ class StoreHandler final : public DataHandler {
   // Creates a Smi-handler for storing a property to a slow object.
   static inline Handle<Smi> StoreNormal(Isolate* isolate);
 
+  // Creates a Smi-handler for storing a property to an interceptor.
+  static inline Handle<Smi> StoreInterceptor(Isolate* isolate);
+
+  // Creates a Smi-handler for storing a property.
+  static inline Handle<Smi> StoreSlow(Isolate* isolate);
+
   // Creates a Smi-handler for storing a property on a proxy.
   static inline Handle<Smi> StoreProxy(Isolate* isolate);
 

src/ic/ic.cc

@@ -1310,12 +1310,10 @@ bool StoreIC::LookupForWrite(LookupIterator* it, Handle<Object> value,
         case LookupIterator::INTERCEPTOR: {
           Handle<JSObject> holder = it->GetHolder<JSObject>();
           InterceptorInfo info = holder->GetNamedInterceptor();
-          if (it->HolderIsReceiverOrHiddenPrototype()) {
-            return !info.non_masking() && receiver.is_identical_to(holder) &&
-                   !info.setter().IsUndefined(isolate());
-          } else if (!info.getter().IsUndefined(isolate()) ||
+          if (it->HolderIsReceiverOrHiddenPrototype() ||
+              !info.getter().IsUndefined(isolate()) ||
               !info.query().IsUndefined(isolate())) {
-            return false;
+            return true;
           }
           break;
         }
@@ -1474,8 +1472,6 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
         return;
       }
     }
-    handler = ComputeHandler(lookup);
-  } else {
     if (state() == UNINITIALIZED && IsStoreGlobalIC() &&
         lookup->state() == LookupIterator::INTERCEPTOR) {
       InterceptorInfo info =
@@ -1493,10 +1489,11 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
         return;
       }
     }
-
+    handler = ComputeHandler(lookup);
+  } else {
     set_slow_stub_reason("LookupForWrite said 'false'");
     // TODO(marja): change slow_stub to return MaybeObjectHandle.
-    handler = MaybeObjectHandle(slow_stub());
+    handler = MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
   }
 
   PatchCache(lookup->name(), handler);
@@ -1537,12 +1534,23 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
 
     case LookupIterator::INTERCEPTOR: {
       Handle<JSObject> holder = lookup->GetHolder<JSObject>();
-      USE(holder);
+      Handle<Smi> smi_handler = StoreHandler::StoreInterceptor(isolate());
+      InterceptorInfo info = holder->GetNamedInterceptor();
+
+      if (!info.getter().IsUndefined(isolate()) ||
+          !info.query().IsUndefined(isolate()) || info.non_masking()) {
+        smi_handler = StoreHandler::StoreSlow(isolate());
+      }
 
+      if (receiver_map().is_identical_to(holder) &&
+          !info.setter().IsUndefined(isolate()) && !info.non_masking()) {
         DCHECK(!holder->GetNamedInterceptor().setter().IsUndefined(isolate()));
-      // TODO(jgruber): Update counter name.
-      TRACE_HANDLER_STATS(isolate(), StoreIC_StoreInterceptorStub);
-      return MaybeObjectHandle(BUILTIN_CODE(isolate(), StoreInterceptorIC));
+        return MaybeObjectHandle(smi_handler);
+      }
+
+      Handle<Object> handler = StoreHandler::StoreThroughPrototype(
+          isolate(), receiver_map(), holder, smi_handler);
+      return MaybeObjectHandle(handler);
     }
 
     case LookupIterator::ACCESSOR: {
@@ -1562,18 +1570,18 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
         if (v8::ToCData<Address>(info->setter()) == kNullAddress) {
           set_slow_stub_reason("setter == kNullAddress");
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         }
         if (AccessorInfo::cast(*accessors).is_special_data_property() &&
             !lookup->HolderIsReceiverOrHiddenPrototype()) {
           set_slow_stub_reason("special data property in prototype chain");
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         }
         if (!AccessorInfo::IsCompatibleReceiverMap(info, receiver_map())) {
           set_slow_stub_reason("incompatible receiver type");
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         }
 
         Handle<Smi> smi_handler = StoreHandler::StoreNativeDataProperty(
@@ -1593,7 +1601,7 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
         if (!setter->IsJSFunction() && !setter->IsFunctionTemplateInfo()) {
           set_slow_stub_reason("setter not a function");
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         }
 
         if ((setter->IsFunctionTemplateInfo() &&
@@ -1602,7 +1610,7 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
              JSFunction::cast(*setter).shared().BreakAtEntry())) {
           // Do not install an IC if the api function has a breakpoint.
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         }
 
         CallOptimization call_optimization(isolate(), setter);
@@ -1626,11 +1634,11 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
           }
           set_slow_stub_reason("incompatible receiver");
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         } else if (setter->IsFunctionTemplateInfo()) {
           set_slow_stub_reason("setter non-simple template");
           TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-          return MaybeObjectHandle(slow_stub());
+          return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
         }
 
         Handle<Smi> smi_handler =
@@ -1646,7 +1654,7 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
             isolate(), receiver_map(), holder, smi_handler));
       }
       TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-      return MaybeObjectHandle(slow_stub());
+      return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
     }
 
     case LookupIterator::DATA: {
@@ -1689,7 +1697,7 @@ MaybeObjectHandle StoreIC::ComputeHandler(LookupIterator* lookup) {
       DCHECK_EQ(kDescriptor, lookup->property_details().location());
       set_slow_stub_reason("constant property");
       TRACE_HANDLER_STATS(isolate(), StoreIC_SlowStub);
-      return MaybeObjectHandle(slow_stub());
+      return MaybeObjectHandle(StoreHandler::StoreSlow(isolate()));
     }
     case LookupIterator::JSPROXY: {
       Handle<JSReceiver> receiver =