From b2bf43d5297c679325ed913531dbcb9e7e1164e7 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Fri, 21 Jul 2017 15:55:49 -0700 Subject: [PATCH] [runtime] Load only 10 bits as PropertyArray length Bug: v8:6404 Change-Id: I187f20006c14aab4a36e2bfef31ca68ebb249e43 Reviewed-on: https://chromium-review.googlesource.com/576516 Commit-Queue: Sathya Gunasekaran Reviewed-by: Jakob Kummerow Reviewed-by: Ulan Degenbaev Reviewed-by: Michael Starzinger Reviewed-by: Camillo Bruni Cr-Commit-Position: refs/heads/master@{#46822} --- src/code-stub-assembler.cc | 27 +++++++++++++++++-- src/code-stub-assembler.h | 6 +++++ src/compiler/access-builder.cc | 13 +++++++++ src/compiler/access-builder.h | 3 +++ .../js-native-context-specialization.cc | 3 ++- src/compiler/type-cache.h | 5 ++++ src/heap/heap.cc | 19 ++++++++++--- src/ic/accessor-assembler.cc | 5 ++-- src/objects-inl.h | 19 +++++++++++-- src/objects.h | 12 ++++++--- src/objects/object-macros-undef.h | 2 ++ src/objects/object-macros.h | 4 +++ 12 files changed, 104 insertions(+), 14 deletions(-) diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index a214886c11..a41038a9c5 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -2349,6 +2349,30 @@ Node* CodeStubAssembler::AllocateFixedArray(ElementsKind kind, return array; } +void CodeStubAssembler::InitializePropertyArrayLength(Node* property_array, + Node* length, + ParameterMode mode) { + CSA_SLOW_ASSERT(this, IsPropertyArray(property_array)); + CSA_ASSERT( + this, IntPtrOrSmiGreaterThan(length, IntPtrOrSmiConstant(0, mode), mode)); + CSA_ASSERT( + this, + IntPtrOrSmiLessThanOrEqual( + length, IntPtrOrSmiConstant(PropertyArray::kMaxLength, mode), mode)); + StoreObjectFieldNoWriteBarrier(property_array, PropertyArray::kLengthOffset, + ParameterToTagged(length, mode), + MachineRepresentation::kTaggedSigned); +} + +Node* CodeStubAssembler::LoadPropertyArrayLength(Node* property_array) { + CSA_SLOW_ASSERT(this, IsPropertyArray(property_array)); + Node* const value = + LoadObjectField(property_array, PropertyArray::kLengthOffset, + MachineType::TaggedSigned()); + Node* const length = SmiAnd(value, SmiConstant(PropertyArray::kLengthMask)); + return length; +} + Node* CodeStubAssembler::AllocatePropertyArray(Node* capacity_node, ParameterMode mode, AllocationFlags flags) { @@ -2361,8 +2385,7 @@ Node* CodeStubAssembler::AllocatePropertyArray(Node* capacity_node, Heap::RootListIndex map_index = Heap::kPropertyArrayMapRootIndex; DCHECK(Heap::RootIsImmortalImmovable(map_index)); StoreMapNoWriteBarrier(array, map_index); - StoreObjectFieldNoWriteBarrier(array, FixedArray::kLengthOffset, - ParameterToTagged(capacity_node, mode)); + InitializePropertyArrayLength(array, capacity_node, mode); return array; } diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index 4d329f893a..444b71bcaa 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -419,6 +419,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler { // Map::GetConstructor()). Node* LoadMapConstructor(Node* map); + // This is only used on a newly allocated PropertyArray which + // doesn't have an existing hash. + void InitializePropertyArrayLength(Node* property_array, Node* length, + ParameterMode mode); + Node* LoadPropertyArrayLength(Node* property_array); + // Check if the map is set for slow properties. Node* IsDictionaryMap(Node* map); diff --git a/src/compiler/access-builder.cc b/src/compiler/access-builder.cc index 9602e30b2a..abde4bff6e 100644 --- a/src/compiler/access-builder.cc +++ b/src/compiler/access-builder.cc @@ -476,6 +476,19 @@ FieldAccess AccessBuilder::ForFixedArrayLength() { return access; } +// static +FieldAccess AccessBuilder::ForPropertyArrayLength() { + // TODO(gsathya): Update the value range once we add the hash code. + FieldAccess access = {kTaggedBase, + PropertyArray::kLengthOffset, + MaybeHandle(), + MaybeHandle(), + TypeCache::Get().kPropertyArrayLengthType, + MachineType::TaggedSigned(), + kNoWriteBarrier}; + return access; +} + // static FieldAccess AccessBuilder::ForFixedTypedArrayBaseBasePointer() { FieldAccess access = { diff --git a/src/compiler/access-builder.h b/src/compiler/access-builder.h index 6c08fe7270..6d59b5cc37 100644 --- a/src/compiler/access-builder.h +++ b/src/compiler/access-builder.h @@ -160,6 +160,9 @@ class V8_EXPORT_PRIVATE AccessBuilder final // Provides access to FixedArray::length() field. static FieldAccess ForFixedArrayLength(); + // Provides access to PropertyArray::length() field. + static FieldAccess ForPropertyArrayLength(); + // Provides access to FixedTypedArrayBase::base_pointer() field. static FieldAccess ForFixedTypedArrayBaseBasePointer(); diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index 677eb9bbb0..3aff330657 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc @@ -2248,13 +2248,14 @@ Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore( simplified()->StoreField(AccessBuilder::ForMap()), new_properties, jsgraph()->PropertyArrayMapConstant(), effect, control); effect = graph()->NewNode( - simplified()->StoreField(AccessBuilder::ForFixedArrayLength()), + simplified()->StoreField(AccessBuilder::ForPropertyArrayLength()), new_properties, jsgraph()->Constant(new_length), effect, control); for (int i = 0; i < new_length; ++i) { effect = graph()->NewNode( simplified()->StoreField(AccessBuilder::ForFixedArraySlot(i)), new_properties, values[i], effect, control); } + // TODO(gsathya): Update hash code here. return graph()->NewNode(common()->FinishRegion(), new_properties, effect); } diff --git a/src/compiler/type-cache.h b/src/compiler/type-cache.h index 5ac9072174..2241fd4661 100644 --- a/src/compiler/type-cache.h +++ b/src/compiler/type-cache.h @@ -83,6 +83,11 @@ class TypeCache final { // [0, FixedArray::kMaxLength]. Type* const kFixedArrayLengthType = CreateRange(0.0, FixedArray::kMaxLength); + // The PropertyArray::length property always containts a smi in the range + // [0, PropertyArray::kMaxLength]. + Type* const kPropertyArrayLengthType = + CreateRange(0.0, PropertyArray::kMaxLength); + // The FixedDoubleArray::length property always containts a smi in the range // [0, FixedDoubleArray::kMaxLength]. Type* const kFixedDoubleArrayLengthType = diff --git a/src/heap/heap.cc b/src/heap/heap.cc index c620d899b8..f4f4daa8dc 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -4084,6 +4084,19 @@ AllocationResult Heap::AllocateEmptyFixedTypedArray( return AllocateFixedTypedArray(0, array_type, false, TENURED); } +namespace { +template +void initialize_length(T* array, int length) { + array->set_length(length); +} + +template <> +void initialize_length(PropertyArray* array, int length) { + array->initialize_length(length); +} + +} // namespace + template AllocationResult Heap::CopyArrayAndGrow(T* src, int grow_by, PretenureFlag pretenure) { @@ -4098,7 +4111,7 @@ AllocationResult Heap::CopyArrayAndGrow(T* src, int grow_by, obj->set_map_after_allocation(src->map(), SKIP_WRITE_BARRIER); T* result = T::cast(obj); - result->set_length(new_len); + initialize_length(result, new_len); // Copy the content. DisallowHeapAllocation no_gc; @@ -4159,7 +4172,7 @@ AllocationResult Heap::CopyArrayWithMap(T* src, Map* map) { } // Slow case: Just copy the content one-by-one. - result->set_length(len); + initialize_length(result, len); for (int i = 0; i < len; i++) result->set(i, src->get(i), mode); return result; } @@ -4243,7 +4256,7 @@ AllocationResult Heap::AllocatePropertyArray(int length, result->set_map_after_allocation(property_array_map(), SKIP_WRITE_BARRIER); PropertyArray* array = PropertyArray::cast(result); - array->set_length(length); + array->initialize_length(length); MemsetPointer(array->data_start(), undefined_value(), length); return result; } diff --git a/src/ic/accessor-assembler.cc b/src/ic/accessor-assembler.cc index da5f3646d2..08056609ba 100644 --- a/src/ic/accessor-assembler.cc +++ b/src/ic/accessor-assembler.cc @@ -1050,9 +1050,7 @@ void AccessorAssembler::ExtendPropertiesBackingStore(Node* object, ParameterMode mode = OptimalParameterMode(); Node* properties = LoadProperties(object); - Node* length = (mode == INTPTR_PARAMETERS) - ? LoadAndUntagFixedArrayBaseLength(properties) - : LoadFixedArrayBaseLength(properties); + Node* length = TaggedToParameter(LoadPropertyArrayLength(properties), mode); // Previous property deletion could have left behind unused backing store // capacity even for a map that think it doesn't have any unused fields. @@ -1086,6 +1084,7 @@ void AccessorAssembler::ExtendPropertiesBackingStore(Node* object, CopyPropertyArrayValues(properties, new_properties, length, SKIP_WRITE_BARRIER, mode); + // TODO(gsathya): Update hash code here. StoreObjectField(object, JSObject::kPropertiesOrHashOffset, new_properties); Comment("] Extend storage"); Goto(&done); diff --git a/src/objects-inl.h b/src/objects-inl.h index b581d41074..4400f5dc4e 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -2701,8 +2701,23 @@ const HashTable* HashTable::cast( SMI_ACCESSORS(FixedArrayBase, length, kLengthOffset) SYNCHRONIZED_SMI_ACCESSORS(FixedArrayBase, length, kLengthOffset) -SMI_ACCESSORS(PropertyArray, length, kLengthOffset) -SYNCHRONIZED_SMI_ACCESSORS(PropertyArray, length, kLengthOffset) +int PropertyArray::length() const { + Object* value = READ_FIELD(this, kLengthOffset); + int len = Smi::ToInt(value); + return len & kLengthMask; +} + +void PropertyArray::initialize_length(int len) { + SLOW_DCHECK(len >= 0); + SLOW_DCHECK(len < kMaxLength); + WRITE_FIELD(this, kLengthOffset, Smi::FromInt(len)); +} + +int PropertyArray::synchronized_length() const { + Object* value = ACQUIRE_READ_FIELD(this, kLengthOffset); + int len = Smi::ToInt(value); + return len & kLengthMask; +} SMI_ACCESSORS(FreeSpace, size, kSizeOffset) RELAXED_SMI_ACCESSORS(FreeSpace, size, kSizeOffset) diff --git a/src/objects.h b/src/objects.h index fb7e5b21dd..2b627f8fd0 100644 --- a/src/objects.h +++ b/src/objects.h @@ -3041,11 +3041,13 @@ class PropertyArray : public HeapObject { public: // [length]: length of the array. inline int length() const; - inline void set_length(int length); - // Get and set the length using acquire loads and release stores. + // Get the length using acquire loads. inline int synchronized_length() const; - inline void synchronized_set_length(int value); + + // This is only used on a newly allocated PropertyArray which + // doesn't have an existing hash. + inline void initialize_length(int length); inline Object* get(int index) const; @@ -3075,6 +3077,10 @@ class PropertyArray : public HeapObject { // No weak fields. typedef BodyDescriptor BodyDescriptorWeak; + static const int kLengthMask = 1023; + static const int kMaxLength = kLengthMask; + STATIC_ASSERT(kMaxLength > kMaxNumberOfDescriptors); + private: DISALLOW_IMPLICIT_CONSTRUCTORS(PropertyArray); }; diff --git a/src/objects/object-macros-undef.h b/src/objects/object-macros-undef.h index 6d5f10c07d..4ad2cd2f71 100644 --- a/src/objects/object-macros-undef.h +++ b/src/objects/object-macros-undef.h @@ -34,6 +34,8 @@ #undef WRITE_INT_FIELD #undef READ_INTPTR_FIELD #undef WRITE_INTPTR_FIELD +#undef RELAXED_READ_INTPTR_FIELD +#undef RELAXED_WRITE_INTPTR_FIELD #undef READ_UINT8_FIELD #undef WRITE_UINT8_FIELD #undef READ_INT8_FIELD diff --git a/src/objects/object-macros.h b/src/objects/object-macros.h index e672af14c9..4aa492a4e4 100644 --- a/src/objects/object-macros.h +++ b/src/objects/object-macros.h @@ -178,6 +178,10 @@ #define WRITE_INT_FIELD(p, offset, value) \ (*reinterpret_cast(FIELD_ADDR(p, offset)) = value) +#define RELAXED_READ_INTPTR_FIELD(p, offset) \ + static_cast(base::Relaxed_Load( \ + reinterpret_cast(FIELD_ADDR_CONST(p, offset)))) + #define READ_INTPTR_FIELD(p, offset) \ (*reinterpret_cast(FIELD_ADDR_CONST(p, offset)))