[stubs] KeyedStoreGeneric: overwrite existing fast properties directly

src/ic/keyed-store-generic.cc

 // Copyright 2016 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.
 
 #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.h"
@@ skipping 54 matching lines @@
                                 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 CheckFieldType(Node* descriptors, Node* name_index, Node* representation,
+                      Node* value, Label* bailout);
+  void OverwriteExistingFastProperty(Node* object, Node* object_map,
+                                     Node* properties, Node* descriptors,
+                                     Node* descriptor_name_index, Node* details,
+                                     Node* value, Label* slow);
 };
 
 void KeyedStoreGenericGenerator::Generate(compiler::CodeAssemblerState* state,
                                           LanguageMode language_mode) {
   KeyedStoreGenericAssembler assembler(state);
   assembler.KeyedStoreGeneric(language_mode);
 }
 
 void KeyedStoreGenericAssembler::BranchIfPrototypesHaveNonFastElements(
     Node* receiver_map, Label* non_fast_elements, Label* only_fast_elements) {
@@ skipping 452 matching lines @@
                         &var_entry, &next_proto, bailout);
       Bind(&found_fast);
       {
         Node* descriptors = var_meta_storage.value();
         Node* name_index = var_entry.value();
         Node* details =
             LoadDetailsByKeyIndex<DescriptorArray>(descriptors, name_index);
         JumpIfDataProperty(details, &ok_to_write, readonly);
 
         // Accessor case.
+        // TODO(jkummerow): Implement a trimmed-down LoadAccessorFromFastObject.
         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();
@@ skipping 35 matching lines @@
            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::CheckFieldType(Node* descriptors,
+                                                Node* name_index,
+                                                Node* representation,
+                                                Node* value, Label* bailout) {
+  Label r_smi(this), r_double(this), r_heapobject(this), all_fine(this);
+  if (FLAG_track_fields) {
+    GotoIf(Word32Equal(representation, Int32Constant(Representation::kSmi)),
+           &r_smi);
+  }
+  if (FLAG_track_double_fields) {
+    GotoIf(Word32Equal(representation, Int32Constant(Representation::kDouble)),
+           &r_double);
+  }
+  if (FLAG_track_heap_object_fields) {
+    GotoIf(
+        Word32Equal(representation, Int32Constant(Representation::kHeapObject)),
+        &r_heapobject);
+  }
+  if (FLAG_track_fields) {
+    GotoIf(Word32Equal(representation, Int32Constant(Representation::kNone)),
+           bailout);
+  }

[Igor Sheludko, 2017/02/16 16:04:37]

CSA_ASSERT(representation == kTagged)?

[Jakob Kummerow, 2017/02/16 18:24:27]

Done.

+  Goto(&all_fine);
+
+  Bind(&r_smi);

[Igor Sheludko, 2017/02/16 16:04:37]

1) "if (FLAG_track_fields) {" or we will have an unreachable block.
2) I think FLAG_track_fields must be made readonly or we could end up using stub
which is not in sync with the runtime code.

[Jakob Kummerow, 2017/02/16 18:24:27]

I agree, but as discussed, that unfortunately doesn't work, because the snapshot
intentionally turns off FLAG_track_double_fields. To solve that issue, let's
just always emit all code here, regardless of flags.

+  { Branch(TaggedIsSmi(value), &all_fine, bailout); }
+
+  Bind(&r_double);

[Igor Sheludko, 2017/02/16 16:04:37]

Same here.

+  {
+    GotoIf(TaggedIsSmi(value), &all_fine);
+    Node* value_map = LoadMap(value);
+    // While supporting mutable HeapNumbers would be straightforward, such
+    // objects should not end up here anyway.
+    CSA_ASSERT(this,
+               WordNotEqual(value_map,
+                            LoadRoot(Heap::kMutableHeapNumberMapRootIndex)));
+    Branch(IsHeapNumberMap(value_map), &all_fine, bailout);
+  }
+
+  Bind(&r_heapobject);

[Igor Sheludko, 2017/02/16 16:04:37]

Same here.

+  {
+    GotoIf(TaggedIsSmi(value), bailout);
+    Node* field_type =
+        LoadValueByKeyIndex<DescriptorArray>(descriptors, name_index);
+    intptr_t kNoneType = reinterpret_cast<intptr_t>(FieldType::None());
+    intptr_t kAnyType = reinterpret_cast<intptr_t>(FieldType::Any());
+    // FieldType::None can't hold any value.
+    GotoIf(WordEqual(field_type, IntPtrConstant(kNoneType)), bailout);
+    // FieldType::Any can hold any value.
+    GotoIf(WordEqual(field_type, IntPtrConstant(kAnyType)), &all_fine);
+    CSA_ASSERT(this, IsWeakCell(field_type));
+    // Cleared WeakCells count as FieldType::None, which can't hold any value.
+    field_type = LoadWeakCellValue(field_type, bailout);
+    // FieldType::Class(...) performs a map check.
+    CSA_ASSERT(this, IsMap(field_type));
+    Branch(WordEqual(LoadMap(value), field_type), &all_fine, bailout);
+  }
+
+  Bind(&all_fine);
+}
+
+void KeyedStoreGenericAssembler::OverwriteExistingFastProperty(
+    Node* object, Node* object_map, Node* properties, Node* descriptors,
+    Node* descriptor_name_index, Node* details, Node* value, Label* slow) {
+  // Properties in descriptors can't be overwritten without map transition.
+  GotoIf(Word32NotEqual(DecodeWord32<PropertyDetails::LocationField>(details),
+                        Int32Constant(kField)),
+         slow);
+
+  if (FLAG_track_constant_fields) {
+    // TODO(ishell): Taking the slow path is not necessary if new and old
+    // values are identical.
+    GotoIf(Word32Equal(DecodeWord32<PropertyDetails::ConstnessField>(details),
+                       Int32Constant(kConst)),
+           slow);
+  }
+
+  Label done(this);
+  Node* representation =
+      DecodeWord32<PropertyDetails::RepresentationField>(details);
+
+  CheckFieldType(descriptors, descriptor_name_index, representation, value,
+                 slow);
+  Node* field_index =
+      DecodeWordFromWord32<PropertyDetails::FieldIndexField>(details);
+  Node* inobject_properties = LoadMapInobjectProperties(object_map);
+
+  Label inobject(this), backing_store(this);
+  Branch(UintPtrLessThan(field_index, inobject_properties), &inobject,
+         &backing_store);
+
+  Bind(&inobject);
+  {
+    Node* field_offset =
+        IntPtrMul(IntPtrSub(LoadMapInstanceSize(object_map),
+                            IntPtrSub(inobject_properties, field_index)),
+                  IntPtrConstant(kPointerSize));
+    Label tagged_rep(this), double_rep(this);
+    Branch(Word32Equal(representation, Int32Constant(Representation::kDouble)),
+           &double_rep, &tagged_rep);
+    Bind(&double_rep);
+    {
+      Node* double_value = ChangeNumberToFloat64(value);
+      if (FLAG_unbox_double_fields) {
+        StoreObjectFieldNoWriteBarrier(object, field_offset, double_value,
+                                       MachineRepresentation::kFloat64);
+      } else {
+        Node* mutable_heap_number = LoadObjectField(object, field_offset);
+        StoreHeapNumberValue(mutable_heap_number, double_value);
+      }
+      Goto(&done);
+    }
+
+    Bind(&tagged_rep);
+    {
+      StoreObjectField(object, field_offset, value);
+      Goto(&done);
+    }
+  }
+
+  Bind(&backing_store);
+  {
+    Node* backing_store_index = IntPtrSub(field_index, inobject_properties);
+    Label tagged_rep(this), double_rep(this);
+    Branch(Word32Equal(representation, Int32Constant(Representation::kDouble)),
+           &double_rep, &tagged_rep);
+    Bind(&double_rep);
+    {
+      Node* double_value = ChangeNumberToFloat64(value);
+      Node* mutable_heap_number =
+          LoadFixedArrayElement(properties, backing_store_index);
+      StoreHeapNumberValue(mutable_heap_number, double_value);
+      Goto(&done);
+    }
+    Bind(&tagged_rep);
+    {
+      StoreFixedArrayElement(properties, backing_store_index, value);
+      Goto(&done);
+    }
+  }
+  Bind(&done);
+}
+
 void KeyedStoreGenericAssembler::EmitGenericPropertyStore(
     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);
+    Comment("fast property store");
+    Node* bitfield3 = LoadMapBitField3(receiver_map);
+    Node* descriptors = LoadMapDescriptors(receiver_map);
+    Label descriptor_found(this);
+    Variable var_name_index(this, MachineType::PointerRepresentation());
+    // TODO(jkummerow): Maybe look for existing map transitions?
+    Label* notfound = &stub_cache;
+    DescriptorLookup(p->name, descriptors, bitfield3, &descriptor_found,
+                     &var_name_index, notfound);
+
+    Bind(&descriptor_found);
+    {
+      Node* name_index = var_name_index.value();
+      Node* details =
+          LoadDetailsByKeyIndex<DescriptorArray>(descriptors, name_index);
+      Label data_property(this);
+      JumpIfDataProperty(details, &data_property, &readonly);
+
+      // Accessor case.
+      // TODO(jkummerow): Implement a trimmed-down LoadAccessorFromFastObject.
+      Variable var_details(this, MachineRepresentation::kWord32);
+      LoadPropertyFromFastObject(receiver, receiver_map, descriptors,
+                                 name_index, &var_details, &var_accessor_pair);
+      var_accessor_holder.Bind(receiver);
+      Goto(&accessor);
+
+      Bind(&data_property);
+      {
+        OverwriteExistingFastProperty(receiver, receiver_map, properties,
+                                      descriptors, name_index, details,
+                                      p->value, slow);
+        Return(p->value);
+      }
+    }
   }
 
   Bind(&dictionary_properties);
   {
     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);
@@ skipping 139 matching lines @@
   Bind(&slow);
   {
     Comment("KeyedStoreGeneric_slow");
     TailCallRuntime(Runtime::kSetProperty, context, receiver, name, value,
                     SmiConstant(language_mode));
   }
 }
 
 }  // namespace internal
 }  // namespace v8