[stubs] KeyedStoreGeneric: inline dictionary property stores

src/ic/keyed-store-generic.cc

Index: src/ic/keyed-store-generic.cc
diff --git a/src/ic/keyed-store-generic.cc b/src/ic/keyed-store-generic.cc
index e7d0fe2231879fd4a8d622763391ee1ded52d9a6..a6bb956d53bfc56f48f221c23f5f2e7b809dbcc2 100644
--- a/src/ic/keyed-store-generic.cc
+++ b/src/ic/keyed-store-generic.cc
@@ -4,6 +4,7 @@
 
 #include "src/ic/keyed-store-generic.h"
 
+#include "src/code-factory.h"
 #include "src/code-stub-assembler.h"
 #include "src/contexts.h"
 #include "src/ic/accessor-assembler-impl.h"
@@ -34,7 +35,8 @@ class KeyedStoreGenericAssembler : public AccessorAssemblerImpl {
                                Node* value, Node* context, Label* slow);
 
   void EmitGenericPropertyStore(Node* receiver, Node* receiver_map,
-                                const StoreICParameters* p, Label* slow);
+                                const StoreICParameters* p, Label* slow,
+                                LanguageMode language_mode);
 
   void BranchIfPrototypesHaveNonFastElements(Node* receiver_map,
                                              Label* non_fast_elements,
@@ -63,6 +65,13 @@ class KeyedStoreGenericAssembler : public AccessorAssemblerImpl {
                                 Node* current_elements_kind, Node* context,
                                 ElementsKind packed_kind,
                                 ElementsKind packed_kind_2, Label* bailout);
+
+  void JumpIfDataProperty(Node* details, Label* writable, Label* readonly);
+  void LookupPropertyOnPrototypeChain(Node* receiver_map, Node* name,
+                                      Label* accessor,
+                                      Variable* var_accessor_pair,
+                                      Variable* var_accessor_holder,
+                                      Label* readonly, Label* bailout);
 };
 
 void KeyedStoreGenericGenerator::Generate(compiler::CodeAssemblerState* state,
@@ -478,28 +487,254 @@ void KeyedStoreGenericAssembler::EmitGenericElementStore(
   }
 }
 
+void KeyedStoreGenericAssembler::JumpIfDataProperty(Node* details,
+                                                    Label* writable,
+                                                    Label* readonly) {
+  GotoIf(IsSetWord32(details, PropertyDetails::kAttributesReadOnlyMask),

[Igor Sheludko, 2016/11/22 12:05:08]

Please add a comment that accessor properties never have READ_ONLY attribute.

[Jakob Kummerow, 2016/11/22 13:47:57]

Done.

+         readonly);
+  Node* kind = DecodeWord32<PropertyDetails::KindField>(details);
+  GotoIf(Word32Equal(kind, Int32Constant(kData)), writable);
+  // Fall through if it's an accessor property.
+}
+
+void KeyedStoreGenericAssembler::LookupPropertyOnPrototypeChain(
+    Node* receiver_map, Node* name, Label* accessor,
+    Variable* var_accessor_pair, Variable* var_accessor_holder, Label* readonly,
+    Label* bailout) {
+  Label ok_to_write(this);
+  Variable var_holder(this, MachineRepresentation::kTagged);
+  var_holder.Bind(LoadMapPrototype(receiver_map));
+  Variable var_holder_map(this, MachineRepresentation::kTagged);
+  var_holder_map.Bind(LoadMap(var_holder.value()));
+
+  Variable* merged_variables[] = {&var_holder, &var_holder_map};
+  Label loop(this, arraysize(merged_variables), merged_variables);
+  Goto(&loop);
+  Bind(&loop);
+  {
+    Node* holder = var_holder.value();
+    Node* holder_map = var_holder_map.value();
+    Node* instance_type = LoadMapInstanceType(holder_map);
+    Label next_proto(this);
+    {
+      Label found(this), found_fast(this), found_dict(this), found_global(this);
+      Variable var_meta_storage(this, MachineRepresentation::kTagged);
+      Variable var_entry(this, MachineType::PointerRepresentation());
+      TryLookupProperty(holder, holder_map, instance_type, name, &found_fast,
+                        &found_dict, &found_global, &var_meta_storage,
+                        &var_entry, &next_proto, bailout);
+      Bind(&found_fast);
+      {
+        Node* descriptors = var_meta_storage.value();
+        Node* name_index = var_entry.value();
+        const int kNameToDetailsOffset = (DescriptorArray::kDescriptorDetails -
+                                          DescriptorArray::kDescriptorKey) *
+                                         kPointerSize;
+        Node* details = LoadAndUntagToWord32FixedArrayElement(
+            descriptors, name_index, kNameToDetailsOffset);
+        JumpIfDataProperty(details, &ok_to_write, readonly);
+
+        // Accessor case.
+        Variable var_details(this, MachineRepresentation::kWord32);
+        LoadPropertyFromFastObject(holder, holder_map, descriptors, name_index,
+                                   &var_details, var_accessor_pair);
+        var_accessor_holder->Bind(holder);
+        Goto(accessor);
+      }
+
+      Bind(&found_dict);
+      {
+        Node* dictionary = var_meta_storage.value();
+        Node* entry = var_entry.value();
+        const int kNameToDetailsOffset = (NameDictionary::kEntryDetailsIndex -
+                                          NameDictionary::kEntryKeyIndex) *
+                                         kPointerSize;
+        Node* details = LoadAndUntagToWord32FixedArrayElement(
+            dictionary, entry, kNameToDetailsOffset);
+        JumpIfDataProperty(details, &ok_to_write, readonly);
+
+        // Accessor case.
+        const int kNameToValueOffset = (NameDictionary::kEntryValueIndex -
+                                        NameDictionary::kEntryKeyIndex) *
+                                       kPointerSize;
+        var_accessor_pair->Bind(
+            LoadFixedArrayElement(dictionary, entry, kNameToValueOffset));
+        var_accessor_holder->Bind(holder);
+        Goto(accessor);
+      }
+
+      Bind(&found_global);
+      {
+        Node* dictionary = var_meta_storage.value();
+        Node* entry = var_entry.value();
+        const int kNameToValueOffset = (GlobalDictionary::kEntryValueIndex -

[Igor Sheludko, 2016/11/22 12:05:08]

Idea for next CL: add helper functions for accessing value and details by entry.

[Jakob Kummerow, 2016/11/22 13:47:57]

Acknowledged. Good idea. An alternative I've considered is to move the constants
into the respective Dictionary class, so here we could simply say
"GlobalDictionary::kNameToValueOffset".

Added a TODO for now.

+                                        GlobalDictionary::kEntryKeyIndex) *
+                                       kPointerSize;
+
+        Node* property_cell =
+            LoadFixedArrayElement(dictionary, entry, kNameToValueOffset);
+
+        Node* value =
+            LoadObjectField(property_cell, PropertyCell::kValueOffset);
+        GotoIf(WordEqual(value, TheHoleConstant()), &next_proto);
+        Node* details = LoadAndUntagToWord32ObjectField(
+            property_cell, PropertyCell::kDetailsOffset);
+        JumpIfDataProperty(details, &ok_to_write, readonly);
+
+        // Accessor case.
+        var_accessor_pair->Bind(value);
+        var_accessor_holder->Bind(holder);
+        Goto(accessor);
+      }
+    }
+
+    Bind(&next_proto);
+    // Bailout if it can be an integer indexed exotic case.
+    GotoIf(Word32Equal(instance_type, Int32Constant(JS_TYPED_ARRAY_TYPE)),
+           bailout);
+    Node* proto = LoadMapPrototype(holder_map);
+    GotoIf(WordEqual(proto, NullConstant()), &ok_to_write);
+    var_holder.Bind(proto);
+    var_holder_map.Bind(LoadMap(proto));
+    Goto(&loop);
+  }
+  Bind(&ok_to_write);
+}
+
 void KeyedStoreGenericAssembler::EmitGenericPropertyStore(
-    Node* receiver, Node* receiver_map, const StoreICParameters* p,
-    Label* slow) {
-  Comment("stub cache probe");
-  // TODO(jkummerow): Don't rely on the stub cache as much.
-  // - existing properties can be overwritten inline (unless readonly).
-  // - for dictionary mode receivers, we can even add properties inline
-  //   (unless the prototype chain prevents it).
-  Variable var_handler(this, MachineRepresentation::kTagged);
-  Label found_handler(this, &var_handler), stub_cache_miss(this);
-  TryProbeStubCache(isolate()->store_stub_cache(), receiver, p->name,
-                    &found_handler, &var_handler, &stub_cache_miss);
-  Bind(&found_handler);
+    Node* receiver, Node* receiver_map, const StoreICParameters* p, Label* slow,
+    LanguageMode language_mode) {
+  Variable var_accessor_pair(this, MachineRepresentation::kTagged);
+  Variable var_accessor_holder(this, MachineRepresentation::kTagged);
+  Label stub_cache(this), fast_properties(this), dictionary_properties(this),
+      accessor(this), readonly(this);
+  Node* properties = LoadProperties(receiver);
+  Node* properties_map = LoadMap(properties);
+  Branch(WordEqual(properties_map, LoadRoot(Heap::kHashTableMapRootIndex)),
+         &dictionary_properties, &fast_properties);
+
+  Bind(&fast_properties);
+  {
+    // TODO(jkummerow): Does it make sense to support some cases here inline?
+    // Maybe overwrite existing writable properties?
+    // Maybe support map transitions?
+    Goto(&stub_cache);
+  }
+
+  Bind(&dictionary_properties);
   {
-    Comment("KeyedStoreGeneric found handler");
-    HandleStoreICHandlerCase(p, var_handler.value(), slow);
+    Comment("dictionary property store");
+    // We checked for LAST_CUSTOM_ELEMENTS_RECEIVER before, which rules out
+    // seeing global objects here (which would need special handling).
+
+    Variable var_name_index(this, MachineType::PointerRepresentation());
+    Label dictionary_found(this, &var_name_index), not_found(this);
+    NameDictionaryLookup<NameDictionary>(properties, p->name, &dictionary_found,
+                                         &var_name_index, &not_found);
+    Bind(&dictionary_found);
+    {
+      Label overwrite(this);
+      const int kNameToDetailsOffset = (NameDictionary::kEntryDetailsIndex -
+                                        NameDictionary::kEntryKeyIndex) *
+                                       kPointerSize;
+      Node* details = LoadAndUntagToWord32FixedArrayElement(
+          properties, var_name_index.value(), kNameToDetailsOffset);
+      JumpIfDataProperty(details, &overwrite, &readonly);
+
+      // Accessor case.
+      const int kNameToValueOffset =
+          (NameDictionary::kEntryValueIndex - NameDictionary::kEntryKeyIndex) *
+          kPointerSize;
+      var_accessor_pair.Bind(LoadFixedArrayElement(
+          properties, var_name_index.value(), kNameToValueOffset));
+      var_accessor_holder.Bind(receiver);
+      Goto(&accessor);
+
+      Bind(&overwrite);
+      {
+        StoreFixedArrayElement(properties, var_name_index.value(), p->value,
+                               kNameToValueOffset, UPDATE_WRITE_BARRIER,
+                               INTPTR_PARAMETERS);
+        Return(p->value);
+      }
+    }
+
+    Bind(&not_found);
+    {
+      LookupPropertyOnPrototypeChain(receiver_map, p->name, &accessor,
+                                     &var_accessor_pair, &var_accessor_holder,
+                                     &readonly, slow);
+      Add<NameDictionary>(properties, p->name, p->value, slow);
+      Return(p->value);
+    }
+  }
+
+  Bind(&accessor);
+  {
+    Label not_callable(this);
+    Node* accessor_pair = var_accessor_pair.value();
+    GotoIf(Word32Equal(LoadInstanceType(accessor_pair),

[Igor Sheludko, 2016/11/22 12:05:08]

You can compare maps instead of instance types.

[Jakob Kummerow, 2016/11/22 13:47:57]

Done. (I wonder which is faster...)

[Igor Sheludko, 2016/11/22 14:07:31]

I heard rumors that CSA can replace LoadRoot() with a constant if it makes sense
for particular platform and root index. So I think it should be faster.

+                       Int32Constant(ACCESSOR_INFO_TYPE)),
+           slow);
+    CSA_ASSERT(this, HasInstanceType(accessor_pair, ACCESSOR_PAIR_TYPE));
+    Node* setter = LoadObjectField(accessor_pair, AccessorPair::kSetterOffset);
+    Node* setter_map = LoadMap(setter);
+    // FunctionTemplateInfo setters are not supported yet.
+    GotoIf(Word32Equal(LoadMapInstanceType(setter_map),

[Igor Sheludko, 2016/11/22 12:05:08]

Same here.

[Jakob Kummerow, 2016/11/22 13:47:57]

Done.

+                       Int32Constant(FUNCTION_TEMPLATE_INFO_TYPE)),
+           slow);
+    GotoUnless(IsCallableMap(setter_map), &not_callable);
+
+    Callable callable = CodeFactory::Call(isolate());
+    CallJS(callable, p->context, setter, receiver, p->value);
+    Return(p->value);
+
+    Bind(&not_callable);
+    {
+      if (language_mode == STRICT) {
+        Node* message =
+            SmiConstant(Smi::FromInt(MessageTemplate::kNoSetterInCallback));
+        TailCallRuntime(Runtime::kThrowTypeError, p->context, message, p->name,
+                        var_accessor_holder.value());
+      } else {
+        DCHECK(language_mode == SLOPPY);

[Igor Sheludko, 2016/11/22 12:05:08]

DCHECK_EQ

[Jakob Kummerow, 2016/11/22 13:47:57]

Done.

+        Return(p->value);
+      }
+    }
   }
-  Bind(&stub_cache_miss);
+
+  Bind(&readonly);
   {
-    Comment("KeyedStoreGeneric_miss");
-    TailCallRuntime(Runtime::kKeyedStoreIC_Miss, p->context, p->value, p->slot,
-                    p->vector, p->receiver, p->name);
+    if (language_mode == STRICT) {
+      Node* message =
+          SmiConstant(Smi::FromInt(MessageTemplate::kStrictReadOnlyProperty));
+      Node* type = Typeof(p->receiver, p->context);
+      TailCallRuntime(Runtime::kThrowTypeError, p->context, message, p->name,
+                      type, p->receiver);
+    } else {
+      DCHECK(language_mode == SLOPPY);

[Igor Sheludko, 2016/11/22 12:05:08]

DCHECK_EQ

[Jakob Kummerow, 2016/11/22 13:47:57]

Done.

+      Return(p->value);
+    }
+  }
+
+  Bind(&stub_cache);
+  {
+    Comment("stub cache probe");
+    Variable var_handler(this, MachineRepresentation::kTagged);
+    Label found_handler(this, &var_handler), stub_cache_miss(this);
+    TryProbeStubCache(isolate()->store_stub_cache(), receiver, p->name,
+                      &found_handler, &var_handler, &stub_cache_miss);
+    Bind(&found_handler);
+    {
+      Comment("KeyedStoreGeneric found handler");
+      HandleStoreICHandlerCase(p, var_handler.value(), slow);
+    }
+    Bind(&stub_cache_miss);
+    {
+      Comment("KeyedStoreGeneric_miss");
+      TailCallRuntime(Runtime::kKeyedStoreIC_Miss, p->context, p->value,
+                      p->slot, p->vector, p->receiver, p->name);
+    }
   }
 }
 
@@ -539,7 +774,7 @@ void KeyedStoreGenericAssembler::KeyedStoreGeneric(LanguageMode language_mode) {
     Comment("key is unique name");
     KeyedStoreGenericAssembler::StoreICParameters p(context, receiver, name,
                                                     value, slot, vector);
-    EmitGenericPropertyStore(receiver, receiver_map, &p, &slow);
+    EmitGenericPropertyStore(receiver, receiver_map, &p, &slow, language_mode);
   }
 
   Bind(&slow);