[ic] Unify handling of Load/StoreHandler objects in load/store IC dispatchers.

This CL also adds support for "lookup on dictionary receivers" to store ICs.

Bug: v8:7206, v8:5561
Change-Id: Icebbc2d52c71f5d25b43f2f2a8adf674e4ec2cbc
Reviewed-on: https://chromium-review.googlesource.com/819232
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50113}

src/ic/accessor-assembler.h

@@ -82,6 +82,9 @@ class AccessorAssembler : public CodeStubAssembler {
   // construction on common paths.
   void LoadIC_BytecodeHandler(const LoadICParameters* p, ExitPoint* exit_point);
 
+  // Loads dataX field from the DataHandler object.
+  Node* LoadHandlerDataField(Node* handler, int data_index);
+
  protected:
   struct StoreICParameters : public LoadICParameters {
     StoreICParameters(Node* context, Node* receiver, Node* name, Node* value,
@@ -91,9 +94,10 @@ class AccessorAssembler : public CodeStubAssembler {
     Node* value;
   };
 
+  enum class ICMode { kNonGlobalIC, kGlobalIC };
   enum ElementSupport { kOnlyProperties, kSupportElements };
   void HandleStoreICHandlerCase(
-      const StoreICParameters* p, Node* handler, Label* miss,
+      const StoreICParameters* p, Node* handler, Label* miss, ICMode ic_mode,
       ElementSupport support_elements = kOnlyProperties);
   void JumpIfDataProperty(Node* details, Label* writable, Label* readonly);
 
@@ -136,7 +140,6 @@ class AccessorAssembler : public CodeStubAssembler {
 
   // LoadIC implementation.
 
-  enum class ICMode { kNonGlobalIC, kGlobalIC };
   void HandleLoadICHandlerCase(
       const LoadICParameters* p, Node* handler, Label* miss,
       ExitPoint* exit_point, ICMode ic_mode = ICMode::kNonGlobalIC,
@@ -148,13 +151,10 @@ class AccessorAssembler : public CodeStubAssembler {
                                   bool throw_reference_error_if_nonexistent,
                                   ElementSupport support_elements);
 
-  void HandleLoadICProtoHandlerCase(const LoadICParameters* p, Node* handler,
-                                    Variable* var_holder,
-                                    Variable* var_smi_handler,
-                                    Label* if_smi_handler, Label* miss,
-                                    ExitPoint* exit_point,
-                                    bool throw_reference_error_if_nonexistent,
-                                    ICMode ic_mode);
+  void HandleLoadICProtoHandler(const LoadICParameters* p, Node* handler,
+                                Variable* var_holder, Variable* var_smi_handler,
+                                Label* if_smi_handler, Label* miss,
+                                ExitPoint* exit_point, ICMode ic_mode);
 
   void HandleLoadField(Node* holder, Node* handler_word,
                        Variable* var_double_value, Label* rebox_double,
@@ -175,7 +175,8 @@ class AccessorAssembler : public CodeStubAssembler {
                                        Node* handler, Label* miss);
 
   void HandleStoreICProtoHandler(const StoreICParameters* p, Node* handler,
-                                 Label* miss, ElementSupport support_elements);
+                                 Label* miss, ICMode ic_mode,
+                                 ElementSupport support_elements);
   // If |transition| is nullptr then the normal field store is generated or
   // transitioning store otherwise.
   void HandleStoreICSmiHandlerCase(Node* handler_word, Node* holder,
@@ -208,6 +209,16 @@ class AccessorAssembler : public CodeStubAssembler {
 
   // Low-level helpers.
 
+  typedef std::function<void(Node* code_handler)> OnCodeHandler;
+  typedef std::function<void(Node* properties, Node* name_index)>
+      OnFoundOnReceiver;
+
+  template <typename ICHandler, typename ICParameters>
+  Node* HandleProtoHandler(const ICParameters* p, Node* handler,
+                           const OnCodeHandler& on_code_handler,
+                           const OnFoundOnReceiver& on_found_on_receiver,
+                           Label* miss, ICMode ic_mode);
+
   Node* GetLanguageMode(Node* vector, Node* slot);
 
   Node* PrepareValueForStore(Node* handler_word, Node* holder,

src/ic/handler-configuration-inl.h

@@ -83,28 +83,6 @@ Handle<Smi> LoadHandler::LoadModuleExport(Isolate* isolate, int index) {
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Smi> LoadHandler::EnableAccessCheckOnReceiver(Isolate* isolate,
-                                                     Handle<Smi> smi_handler) {
-  int config = smi_handler->value();
-#ifdef DEBUG
-  Kind kind = KindBits::decode(config);
-  DCHECK_NE(kElement, kind);
-#endif
-  config = DoAccessCheckOnReceiverBits::update(config, true);
-  return handle(Smi::FromInt(config), isolate);
-}
-
-Handle<Smi> LoadHandler::EnableLookupOnReceiver(Isolate* isolate,
-                                                Handle<Smi> smi_handler) {
-  int config = smi_handler->value();
-#ifdef DEBUG
-  Kind kind = KindBits::decode(config);
-  DCHECK_NE(kElement, kind);
-#endif
-  config = LookupOnReceiverBits::update(config, true);
-  return handle(Smi::FromInt(config), isolate);
-}
-
 Handle<Smi> LoadHandler::LoadNonExistent(Isolate* isolate) {
   int config = KindBits::encode(kNonExistent);
   return handle(Smi::FromInt(config), isolate);
@@ -150,13 +128,6 @@ Handle<Smi> StoreHandler::StoreProxy(Isolate* isolate) {
   return handle(Smi::FromInt(config), isolate);
 }
 
-Handle<Smi> StoreHandler::EnableAccessCheckOnReceiver(Isolate* isolate,
-                                                      Handle<Smi> smi_handler) {
-  int config = smi_handler->value();
-  config = DoAccessCheckOnReceiverBits::update(config, true);
-  return handle(Smi::FromInt(config), isolate);
-}
-
 Handle<Smi> StoreHandler::StoreField(Isolate* isolate, Kind kind,
                                      int descriptor, FieldIndex field_index,
                                      Representation representation,
@@ -246,11 +217,6 @@ WeakCell* StoreHandler::GetTransitionCell(Object* handler) {
   return cell;
 }
 
-// static
-bool StoreHandler::IsHandler(Object* maybe_handler) {
-  return maybe_handler->IsStoreHandler();
-}
-
 }  // namespace internal
 }  // namespace v8
 

src/ic/handler-configuration.cc

@@ -13,16 +13,27 @@ namespace internal {
 
 namespace {
 
-template <bool fill_array = true>
+template <typename BitField>
+Handle<Smi> SetBitFieldValue(Isolate* isolate, Handle<Smi> smi_handler,
+                             typename BitField::FieldType value) {
+  int config = smi_handler->value();
+  config = BitField::update(config, true);
+  return handle(Smi::FromInt(config), isolate);
+}
+
+// TODO(ishell): Remove templatezation once we move common bits from
+// Load/StoreHandler to the base class.
+template <typename ICHandler, bool fill_array = true>
 int InitPrototypeChecks(Isolate* isolate, Handle<Map> receiver_map,
                         Handle<JSReceiver> holder, Handle<Name> name,
-                        Handle<DataHandler> handler) {
+                        Handle<ICHandler> handler,
+                        Handle<Smi>* smi_handler = nullptr) {
   if (!holder.is_null() && holder->map() == *receiver_map) return 0;
 
-  HandleScope scope(isolate);
   int checks_count = 0;
-
-  if (receiver_map->IsPrimitiveMap() || receiver_map->IsJSGlobalProxyMap()) {
+  if (receiver_map->IsPrimitiveMap() ||
+      receiver_map->is_access_check_needed()) {
+    DCHECK(!receiver_map->IsJSGlobalObjectMap());
     // The validity cell check for primitive and global proxy receivers does
     // not guarantee that certain native context ever had access to other
     // native context. However, a handler created for one native context could
@@ -32,8 +43,19 @@ int InitPrototypeChecks(Isolate* isolate, Handle<Map> receiver_map,
     if (fill_array) {
       Handle<Context> native_context = isolate->native_context();
       handler->set_data2(native_context->self_weak_cell());
+    } else {
+      // Enable access checks on receiver.
+      typedef typename ICHandler::DoAccessCheckOnReceiverBits Bit;
+      *smi_handler = SetBitFieldValue<Bit>(isolate, *smi_handler, true);
     }
     checks_count++;
+  } else if (receiver_map->is_dictionary_map() &&
+             !receiver_map->IsJSGlobalObjectMap()) {
+    if (!fill_array) {
+      // Enable lookup on receiver.
+      typedef typename ICHandler::LookupOnReceiverBits Bit;
+      *smi_handler = SetBitFieldValue<Bit>(isolate, *smi_handler, true);
+    }
   }
   return checks_count;
 }
@@ -44,10 +66,13 @@ int InitPrototypeChecks(Isolate* isolate, Handle<Map> receiver_map,
 // checked.
 // Returns -1 if the handler has to be compiled or the number of prototype
 // checks otherwise.
+template <typename ICHandler>
 int GetPrototypeCheckCount(Isolate* isolate, Handle<Map> receiver_map,
-                           Handle<JSReceiver> holder, Handle<Name> name) {
-  return InitPrototypeChecks<false>(isolate, receiver_map, holder, name,
-                                    Handle<DataHandler>());
+                           Handle<JSReceiver> holder, Handle<Name> name,
+                           Handle<Smi>* smi_handler) {
+  DCHECK_NOT_NULL(smi_handler);
+  return InitPrototypeChecks<ICHandler, false>(
+      isolate, receiver_map, holder, name, Handle<ICHandler>(), smi_handler);
 }
 
 }  // namespace
@@ -59,20 +84,8 @@ Handle<Object> LoadHandler::LoadFromPrototype(Isolate* isolate,
                                               Handle<Name> name,
                                               Handle<Smi> smi_handler,
                                               MaybeHandle<Object> maybe_data) {
-  int checks_count =
-      GetPrototypeCheckCount(isolate, receiver_map, holder, name);
-  DCHECK_LE(0, checks_count);
-  DCHECK_LE(checks_count, 1);
-
-  if (receiver_map->IsPrimitiveMap() ||
-      receiver_map->is_access_check_needed()) {
-    DCHECK(!receiver_map->is_dictionary_map());
-    DCHECK_EQ(1, checks_count);  // For native context.
-    smi_handler = EnableAccessCheckOnReceiver(isolate, smi_handler);
-  } else if (receiver_map->is_dictionary_map() &&
-             !receiver_map->IsJSGlobalObjectMap()) {
-    smi_handler = EnableLookupOnReceiver(isolate, smi_handler);
-  }
+  int checks_count = GetPrototypeCheckCount<LoadHandler>(
+      isolate, receiver_map, holder, name, &smi_handler);
 
   Handle<Cell> validity_cell =
       Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate);
@@ -100,19 +113,8 @@ Handle<Object> LoadHandler::LoadFullChain(Isolate* isolate,
                                           Handle<Name> name,
                                           Handle<Smi> smi_handler) {
   Handle<JSReceiver> end;  // null handle
-  int checks_count = GetPrototypeCheckCount(isolate, receiver_map, end, name);
-  DCHECK_LE(0, checks_count);
-  DCHECK_LE(checks_count, 1);
-
-  if (receiver_map->IsPrimitiveMap() ||
-      receiver_map->is_access_check_needed()) {
-    DCHECK(!receiver_map->is_dictionary_map());
-    DCHECK_EQ(1, checks_count);  // For native context.
-    smi_handler = EnableAccessCheckOnReceiver(isolate, smi_handler);
-  } else if (receiver_map->is_dictionary_map() &&
-             !receiver_map->IsJSGlobalObjectMap()) {
-    smi_handler = EnableLookupOnReceiver(isolate, smi_handler);
-  }
+  int checks_count = GetPrototypeCheckCount<LoadHandler>(
+      isolate, receiver_map, end, name, &smi_handler);
 
   Handle<Object> validity_cell =
       Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate);
@@ -199,16 +201,8 @@ Handle<Object> StoreHandler::StoreThroughPrototype(
     Isolate* isolate, Handle<Map> receiver_map, Handle<JSReceiver> holder,
     Handle<Name> name, Handle<Smi> smi_handler,
     MaybeHandle<Object> maybe_data) {
-  int checks_count =
-      GetPrototypeCheckCount(isolate, receiver_map, holder, name);
-
-  DCHECK_LE(0, checks_count);
-
-  if (receiver_map->is_access_check_needed()) {
-    DCHECK(!receiver_map->is_dictionary_map());
-    DCHECK_LE(1, checks_count);  // For native context.
-    smi_handler = EnableAccessCheckOnReceiver(isolate, smi_handler);
-  }
+  int checks_count = GetPrototypeCheckCount<StoreHandler>(
+      isolate, receiver_map, holder, name, &smi_handler);
 
   Handle<Object> validity_cell =
       Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate);

src/ic/handler-configuration.h

@@ -83,7 +83,8 @@ class LoadHandler final : public DataHandler {
   //
   // Encoding when KindBits contains kElement or kIndexedString.
   //
-  class AllowOutOfBoundsBits : public BitField<bool, KindBits::kNext, 1> {};
+  class AllowOutOfBoundsBits
+      : public BitField<bool, LookupOnReceiverBits::kNext, 1> {};
 
   //
   // Encoding when KindBits contains kElement.
@@ -99,8 +100,9 @@ class LoadHandler final : public DataHandler {
   //
   // Encoding when KindBits contains kModuleExport.
   //
-  class ExportsIndexBits : public BitField<unsigned, KindBits::kNext,
-                                           kSmiValueSize - KindBits::kNext> {};
+  class ExportsIndexBits
+      : public BitField<unsigned, LookupOnReceiverBits::kNext,
+                        kSmiValueSize - LookupOnReceiverBits::kNext> {};
 
   // Decodes kind from Smi-handler.
   static inline Kind GetHandlerKind(Smi* smi_handler);
@@ -173,17 +175,6 @@ class LoadHandler final : public DataHandler {
 
   // Decodes the KeyedAccessLoadMode from a {handler}.
   static KeyedAccessLoadMode GetKeyedAccessLoadMode(Object* handler);
-
- private:
-  // Sets DoAccessCheckOnReceiverBits in given Smi-handler. The receiver
-  // check is a part of a prototype chain check.
-  static inline Handle<Smi> EnableAccessCheckOnReceiver(
-      Isolate* isolate, Handle<Smi> smi_handler);
-
-  // Sets LookupOnReceiverBits in given Smi-handler. The receiver
-  // check is a part of a prototype chain check.
-  static inline Handle<Smi> EnableLookupOnReceiver(Isolate* isolate,
-                                                   Handle<Smi> smi_handler);
 };
 
 // A set of bit fields representing Smi handlers for stores and a HeapObject
@@ -216,21 +207,24 @@ class StoreHandler final : public DataHandler {
 
   enum FieldRepresentation { kSmi, kDouble, kHeapObject, kTagged };
 
-  static inline bool IsHandler(Object* maybe_handler);
-
   // Applicable to kGlobalProxy, kProxy kinds.
 
   // Defines whether access rights check should be done on receiver object.
   class DoAccessCheckOnReceiverBits
       : public BitField<bool, KindBits::kNext, 1> {};
 
+  // Defines whether a lookup should be done on receiver object before
+  // proceeding to the prototype chain. Applicable to named property kinds only
+  // when storing through prototype chain. Ignored when storing to holder.
+  class LookupOnReceiverBits
+      : public BitField<bool, DoAccessCheckOnReceiverBits::kNext, 1> {};
+
   // Applicable to kField, kTransitionToField and kTransitionToConstant
   // kinds.
 
   // Index of a value entry in the descriptor array.
-  class DescriptorBits
-      : public BitField<unsigned, DoAccessCheckOnReceiverBits::kNext,
-                        kDescriptorIndexBitCount> {};
+  class DescriptorBits : public BitField<unsigned, LookupOnReceiverBits::kNext,
+                                         kDescriptorIndexBitCount> {};
   //
   // Encoding when KindBits contains kTransitionToConstant.
   //
@@ -306,11 +300,6 @@ class StoreHandler final : public DataHandler {
   static inline Handle<Smi> StoreProxy(Isolate* isolate);
 
  private:
-  // Sets DoAccessCheckOnReceiverBits in given Smi-handler. The receiver
-  // check is a part of a prototype chain check.
-  static inline Handle<Smi> EnableAccessCheckOnReceiver(
-      Isolate* isolate, Handle<Smi> smi_handler);
-
   static inline Handle<Smi> StoreField(Isolate* isolate, Kind kind,
                                        int descriptor, FieldIndex field_index,
                                        Representation representation,

src/ic/keyed-store-generic.cc

@@ -824,7 +824,8 @@ void KeyedStoreGenericAssembler::EmitGenericPropertyStore(
       BIND(&found_handler);
       {
         Comment("KeyedStoreGeneric found transition handler");
-        HandleStoreICHandlerCase(p, var_handler.value(), notfound);
+        HandleStoreICHandlerCase(p, var_handler.value(), notfound,
+                                 ICMode::kNonGlobalIC);
       }
     }
   }
@@ -942,7 +943,8 @@ void KeyedStoreGenericAssembler::EmitGenericPropertyStore(
     BIND(&found_handler);
     {
       Comment("KeyedStoreGeneric found handler");
-      HandleStoreICHandlerCase(p, var_handler.value(), &stub_cache_miss);
+      HandleStoreICHandlerCase(p, var_handler.value(), &stub_cache_miss,
+                               ICMode::kNonGlobalIC);
     }
     BIND(&stub_cache_miss);
     {

src/transitions.cc

@@ -17,7 +17,7 @@ void TransitionsAccessor::Initialize() {
     encoding_ = kUninitialized;
   } else if (HeapObject::cast(raw_transitions_)->IsWeakCell()) {
     encoding_ = kWeakCell;
-  } else if (StoreHandler::IsHandler(raw_transitions_)) {
+  } else if (HeapObject::cast(raw_transitions_)->IsStoreHandler()) {
     encoding_ = kHandler;
   } else if (HeapObject::cast(raw_transitions_)->IsTransitionArray()) {
     encoding_ = kFullTransitionArray;
@@ -250,7 +250,7 @@ Object* TransitionsAccessor::SearchHandler(Name* name,
       int transition = transitions()->Search(kData, name, NONE);
       if (transition == kNotFound) return nullptr;
       Object* raw_handler = transitions()->GetRawTarget(transition);
-      if (StoreHandler::IsHandler(raw_handler)) {
+      if (raw_handler->IsStoreHandler()) {
         return StoreHandler::ValidHandlerOrNull(raw_handler, name,
                                                 out_transition);
       }

test/mjsunit/ic-lookup-on-receiver.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+(function TestLookupOnReceiver() {
+  let log = [];
+
+  function f(o, v) {
+    o.x = v;
+    return o.x;
+  }
+
+  let p = {};
+  Object.defineProperty(
+      p, "x",
+      {
+        get: function() { return 153; },
+        set: function(v) { log.push("set"); },
+        configurable: true
+      });
+
+  let o = Object.create(p);
+  // Turn o to dictionary mode.
+  for (let i = 0; i < 2048; i++) {
+    o["p"+i] = 0;
+  }
+  assertFalse(%HasFastProperties(o));
+
+  for (let i = 0; i < 5; i++) {
+    log.push(f(o, i));
+  }
+
+  Object.defineProperty(o, "x", { value: 0, configurable: true, writable: true});
+
+  for (let i = 0; i < 5; i++) {
+    log.push(f(o, 42 + i));
+  }
+
+  assertEquals(log,
+               ["set", 153, "set", 153, "set", 153, "set", 153, "set", 153,
+                42, 43, 44, 45, 46]);
+})();