[builtins] DeleteProperty: Handle last-added fast properties

src/ic/accessor-assembler.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/accessor-assembler.h"
 
 #include "src/code-factory.h"
 #include "src/code-stubs.h"
 #include "src/counters.h"
 #include "src/ic/handler-configuration.h"
@@ skipping 966 matching lines @@
                     transition_to_field, miss);
     if (transition_to_field) {
       StoreMap(holder, transition);
     }
     Return(value);
   }
 
   BIND(&if_out_of_object);
   {
     if (transition_to_field) {
-      Label storage_extended(this);
-      GotoIfNot(IsSetWord<StoreHandler::ExtendStorageBits>(handler_word),
-                &storage_extended);
-      Comment("[ Extend storage");
-      ExtendPropertiesBackingStore(holder);
-      Comment("] Extend storage");
-      Goto(&storage_extended);
-
-      BIND(&storage_extended);
+      ExtendPropertiesBackingStore(holder, handler_word);
     }
 
     StoreNamedField(handler_word, holder, false, representation, prepared_value,
                     transition_to_field, miss);
     if (transition_to_field) {
       StoreMap(holder, transition);
     }
     Return(value);
   }
 }
@@ skipping 40 matching lines @@
 
   } else if (representation.IsSmi()) {
     GotoIfNot(TaggedIsSmi(value), bailout);
 
   } else {
     DCHECK(representation.IsTagged());
   }
   return value;
 }
 
-void AccessorAssembler::ExtendPropertiesBackingStore(Node* object) {
+void AccessorAssembler::ExtendPropertiesBackingStore(Node* object,
+                                                     Node* handler_word) {
+  Label done(this);
+  GotoIfNot(IsSetWord<StoreHandler::ExtendStorageBits>(handler_word), &done);
+  Comment("[ Extend storage");
+
   ParameterMode mode = OptimalParameterMode();
 
   Node* properties = LoadProperties(object);
   Node* length = (mode == INTPTR_PARAMETERS)
                      ? LoadAndUntagFixedArrayBaseLength(properties)
                      : LoadFixedArrayBaseLength(properties);
 
+  // Previous property deletion could have left behind unused backing store
+  // capacity even for a map that think it doesn't have any unused fields.
+  // Perform a bounds check to see if we actually have to grow the array.
+  Node* new_offset =
+      IntPtrSub(DecodeWord<StoreHandler::FieldOffsetBits>(handler_word),
+                IntPtrConstant(FixedArray::OffsetOfElementAt(0)));
+  constexpr int kSmiShift = kSmiTagSize + kSmiShiftSize;
+  int shift = kPointerSizeLog2 - (mode == SMI_PARAMETERS ? kSmiShift : 0);
+  Node* new_index = WordShr(new_offset, shift);
+  GotoIf(IntPtrOrSmiLessThan(new_index, length, mode), &done);

[Igor Sheludko, 2017/04/21 14:17:50]

Probably the following code would be a bit more readable:

Node* offset = DecodeWord<StoreHandler::FieldOffsetBits>(handler_word);
Node* size = ElementOffsetFromIndex(length, FAST_ELEMENTS, mode,
FixedArray::kHeaderSize);
GotoIf(UintPtrLessThan(offset, size, mode), &done);

[Jakob Kummerow, 2017/04/21 17:49:31]

Thanks, that's much better. Done.

+
   Node* delta = IntPtrOrSmiConstant(JSObject::kFieldsAdded, mode);
   Node* new_capacity = IntPtrOrSmiAdd(length, delta, mode);
 
   // Grow properties array.
   ElementsKind kind = FAST_ELEMENTS;
   DCHECK(kMaxNumberOfDescriptors + JSObject::kFieldsAdded <
          FixedArrayBase::GetMaxLengthForNewSpaceAllocation(kind));
   // The size of a new properties backing store is guaranteed to be small
   // enough that the new backing store will be allocated in new space.
   CSA_ASSERT(this,
              UintPtrOrSmiLessThan(
                  new_capacity,
                  IntPtrOrSmiConstant(
                      kMaxNumberOfDescriptors + JSObject::kFieldsAdded, mode),
                  mode));
 
   Node* new_properties = AllocateFixedArray(kind, new_capacity, mode);
 
   FillFixedArrayWithValue(kind, new_properties, length, new_capacity,
                           Heap::kUndefinedValueRootIndex, mode);
 
   // |new_properties| is guaranteed to be in new space, so we can skip
   // the write barrier.
   CopyFixedArrayElements(kind, properties, new_properties, length,
                          SKIP_WRITE_BARRIER, mode);
 
   StoreObjectField(object, JSObject::kPropertiesOffset, new_properties);
+  Comment("] Extend storage");
+  Goto(&done);
+
+  BIND(&done);
 }
 
 void AccessorAssembler::StoreNamedField(Node* handler_word, Node* object,
                                         bool is_inobject,
                                         Representation representation,
                                         Node* value, bool transition_to_field,
                                         Label* bailout) {
   bool store_value_as_double = representation.IsDouble();
   Node* property_storage = object;
   if (!is_inobject) {
@@ skipping 1388 matching lines @@
   Node* context = Parameter(Descriptor::kContext);
   Node* vector = LoadFeedbackVectorForStub();
 
   Callable callable =
       CodeFactory::KeyedStoreICInOptimizedCode(isolate(), language_mode);
   TailCallStub(callable, context, receiver, name, value, slot, vector);
 }
 
 }  // namespace internal
 }  // namespace v8