Skip to content

Commit 42409a2

Browse files
santiaboyCommit Bot
santiaboy
authored and
Commit Bot
committed
[objects] Delete double field unboxing
Reasons: * We disabled it more than a year ago for all configs * Not easy to re-enable * Not compatible with pointer compression as-is * Not compatible with concurrent TP/TF as-is * No concrete plans to re-enable it Also remove Map's layout_descriptor since it was only used for double field unboxing. Bug: v8:11422 Change-Id: I9260906eac199213b3210712e9903f1ecf1d7979 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2676637 Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Cr-Commit-Position: refs/heads/master@{#72671}
1 parent 60ba220 commit 42409a2

56 files changed

Lines changed: 316 additions & 3356 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎BUILD.gn‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3175,9 +3175,6 @@ v8_source_set("v8_base_without_compiler") {
31753175
"src/objects/js-weak-refs.h",
31763176
"src/objects/keys.cc",
31773177
"src/objects/keys.h",
3178-
"src/objects/layout-descriptor-inl.h",
3179-
"src/objects/layout-descriptor.cc",
3180-
"src/objects/layout-descriptor.h",
31813178
"src/objects/literal-objects-inl.h",
31823179
"src/objects/literal-objects.cc",
31833180
"src/objects/literal-objects.h",

‎src/builtins/builtins-constructor-gen.cc‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -584,39 +584,26 @@ TNode<HeapObject> ConstructorBuiltinsAssembler::CreateShallowObjectLiteral(
584584
// Copy over in-object properties.
585585
Label continue_with_write_barrier(this), done_init(this);
586586
TVARIABLE(IntPtrT, offset, IntPtrConstant(JSObject::kHeaderSize));
587-
// Heap numbers are only mutable on 32-bit platforms.
588-
bool may_use_mutable_heap_numbers = !FLAG_unbox_double_fields;
589587
{
590588
Comment("Copy in-object properties fast");
591589
Label continue_fast(this, &offset);
592590
Branch(IntPtrEqual(offset.value(), instance_size), &done_init,
593591
&continue_fast);
594592
BIND(&continue_fast);
595-
if (may_use_mutable_heap_numbers) {
596-
TNode<Object> field = LoadObjectField(boilerplate, offset.value());
597-
Label store_field(this);
598-
GotoIf(TaggedIsSmi(field), &store_field);
599-
// TODO(leszeks): Read the field descriptor to decide if this heap
600-
// number is mutable or not.
601-
GotoIf(IsHeapNumber(CAST(field)), &continue_with_write_barrier);
602-
Goto(&store_field);
603-
BIND(&store_field);
604-
StoreObjectFieldNoWriteBarrier(copy, offset.value(), field);
605-
} else {
606-
// Copy fields as raw data.
607-
TNode<TaggedT> field =
608-
LoadObjectField<TaggedT>(boilerplate, offset.value());
609-
StoreObjectFieldNoWriteBarrier(copy, offset.value(), field);
610-
}
593+
TNode<Object> field = LoadObjectField(boilerplate, offset.value());
594+
Label store_field(this);
595+
GotoIf(TaggedIsSmi(field), &store_field);
596+
// TODO(leszeks): Read the field descriptor to decide if this heap
597+
// number is mutable or not.
598+
GotoIf(IsHeapNumber(CAST(field)), &continue_with_write_barrier);
599+
Goto(&store_field);
600+
BIND(&store_field);
601+
StoreObjectFieldNoWriteBarrier(copy, offset.value(), field);
611602
offset = IntPtrAdd(offset.value(), IntPtrConstant(kTaggedSize));
612603
Branch(WordNotEqual(offset.value(), instance_size), &continue_fast,
613604
&done_init);
614605
}
615606

616-
if (!may_use_mutable_heap_numbers) {
617-
BIND(&done_init);
618-
return copy;
619-
}
620607
// Continue initializing the literal after seeing the first sub-object
621608
// potentially causing allocation. In this case we prepare the new literal
622609
// by copying all pending fields over from the boilerplate and emit full
@@ -660,7 +647,6 @@ void ConstructorBuiltinsAssembler::CopyMutableHeapNumbersInObject(
660647
TNode<IntPtrT> end_offset) {
661648
// Iterate over all object properties of a freshly copied object and
662649
// duplicate mutable heap numbers.
663-
if (FLAG_unbox_double_fields) return;
664650
Comment("Copy mutable HeapNumber values");
665651
BuildFastLoop<IntPtrT>(
666652
start_offset, end_offset,

‎src/codegen/code-stub-assembler.cc‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8815,13 +8815,9 @@ void CodeStubAssembler::LoadPropertyFromFastObject(
88158815
}
88168816
BIND(&if_double);
88178817
{
8818-
if (FLAG_unbox_double_fields) {
8819-
var_double_value = LoadObjectField<Float64T>(object, field_offset);
8820-
} else {
8821-
TNode<HeapNumber> heap_number =
8822-
CAST(LoadObjectField(object, field_offset));
8823-
var_double_value = LoadHeapNumberValue(heap_number);
8824-
}
8818+
TNode<HeapNumber> heap_number =
8819+
CAST(LoadObjectField(object, field_offset));
8820+
var_double_value = LoadHeapNumberValue(heap_number);
88258821
Goto(&rebox_double);
88268822
}
88278823
}

‎src/common/globals.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,6 @@ STATIC_ASSERT(V8_DEFAULT_STACK_SIZE_KB* KB +
9999
kStackLimitSlackForDeoptimizationInBytes <=
100100
MB);
101101

102-
// Determine whether double field unboxing feature is enabled.
103-
#if V8_TARGET_ARCH_64_BIT && !defined(V8_COMPRESS_POINTERS)
104-
#define V8_DOUBLE_FIELDS_UNBOXING false
105-
#else
106-
#define V8_DOUBLE_FIELDS_UNBOXING false
107-
#endif
108-
109102
// Determine whether dict mode prototypes feature is enabled.
110103
#ifdef V8_DICT_MODE_PROTOTYPES
111104
#define V8_DICT_MODE_PROTOTYPES_BOOL true

‎src/compiler/access-info.cc‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -394,11 +394,9 @@ PropertyAccessInfo AccessInfoFactory::ComputeDataFieldAccessInfo(
394394
descriptor));
395395
} else if (details_representation.IsDouble()) {
396396
field_type = type_cache_->kFloat64;
397-
if (!FLAG_unbox_double_fields) {
398-
unrecorded_dependencies.push_back(
399-
dependencies()->FieldRepresentationDependencyOffTheRecord(
400-
map_ref, descriptor));
401-
}
397+
unrecorded_dependencies.push_back(
398+
dependencies()->FieldRepresentationDependencyOffTheRecord(map_ref,
399+
descriptor));
402400
} else if (details_representation.IsHeapObject()) {
403401
// Extract the field type from the property details (make sure its
404402
// representation is TaggedPointer to reflect the heap object case).
@@ -862,12 +860,10 @@ PropertyAccessInfo AccessInfoFactory::LookupTransition(
862860
transition_map_ref, number));
863861
} else if (details_representation.IsDouble()) {
864862
field_type = type_cache_->kFloat64;
865-
if (!FLAG_unbox_double_fields) {
866-
transition_map_ref.SerializeOwnDescriptor(number);
867-
unrecorded_dependencies.push_back(
868-
dependencies()->FieldRepresentationDependencyOffTheRecord(
869-
transition_map_ref, number));
870-
}
863+
transition_map_ref.SerializeOwnDescriptor(number);
864+
unrecorded_dependencies.push_back(
865+
dependencies()->FieldRepresentationDependencyOffTheRecord(
866+
transition_map_ref, number));
871867
} else if (details_representation.IsHeapObject()) {
872868
// Extract the field type from the property details (make sure its
873869
// representation is TaggedPointer to reflect the heap object case).

‎src/compiler/effect-control-linearizer.cc‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5220,13 +5220,8 @@ Node* EffectControlLinearizer::LowerLoadFieldByIndex(Node* node) {
52205220
Node* offset =
52215221
__ IntAdd(__ WordShl(index, __ IntPtrConstant(kTaggedSizeLog2)),
52225222
__ IntPtrConstant(JSObject::kHeaderSize - kHeapObjectTag));
5223-
if (FLAG_unbox_double_fields) {
5224-
Node* result = __ Load(MachineType::Float64(), object, offset);
5225-
__ Goto(&done_double, result);
5226-
} else {
5227-
Node* field = __ Load(MachineType::AnyTagged(), object, offset);
5228-
__ Goto(&loaded_field, field);
5229-
}
5223+
Node* field = __ Load(MachineType::AnyTagged(), object, offset);
5224+
__ Goto(&loaded_field, field);
52305225
}
52315226

52325227
__ Bind(&if_outofobject);

‎src/compiler/heap-refs.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,6 @@ class JSObjectRef : public JSReceiverRef {
323323

324324
Handle<JSObject> object() const;
325325

326-
uint64_t RawFastDoublePropertyAsBitsAt(FieldIndex index) const;
327-
double RawFastDoublePropertyAt(FieldIndex index) const;
328326
ObjectRef RawFastPropertyAt(FieldIndex index) const;
329327

330328
// Return the value of the property identified by the field {index}
@@ -699,7 +697,6 @@ class V8_EXPORT_PRIVATE MapRef : public HeapObjectRef {
699697
NameRef GetPropertyKey(InternalIndex descriptor_index) const;
700698
FieldIndex GetFieldIndexFor(InternalIndex descriptor_index) const;
701699
ObjectRef GetFieldType(InternalIndex descriptor_index) const;
702-
bool IsUnboxedDoubleField(InternalIndex descriptor_index) const;
703700
base::Optional<ObjectRef> GetStrongValue(
704701
InternalIndex descriptor_number) const;
705702

‎src/compiler/js-create-lowering.cc‎

Lines changed: 27 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,52 +1647,35 @@ Node* JSCreateLowering::AllocateFastLiteral(Node* effect, Node* control,
16471647
kFullWriteBarrier,
16481648
LoadSensitivity::kUnsafe,
16491649
const_field_info};
1650+
ObjectRef boilerplate_value = boilerplate.RawFastPropertyAt(index);
1651+
bool is_uninitialized =
1652+
boilerplate_value.IsHeapObject() &&
1653+
boilerplate_value.AsHeapObject().map().oddball_type() ==
1654+
OddballType::kUninitialized;
1655+
if (is_uninitialized) {
1656+
access.const_field_info = ConstFieldInfo::None();
1657+
}
16501658
Node* value;
1651-
if (boilerplate_map.IsUnboxedDoubleField(i)) {
1652-
access.machine_type = MachineType::Float64();
1653-
access.type = Type::Number();
1654-
uint64_t value_bits = boilerplate.RawFastDoublePropertyAsBitsAt(index);
1655-
if (value_bits == kHoleNanInt64) {
1656-
// This special case is analogous to is_uninitialized being true in the
1657-
// non-unboxed-double case below. The store of the hole NaN value here
1658-
// will always be followed by another store that actually initializes
1659-
// the field. The hole NaN should therefore be unobservable.
1660-
// Load elimination expects there to be at most one const store to any
1661-
// given field, so we always mark the unobservable ones as mutable.
1662-
access.const_field_info = ConstFieldInfo::None();
1663-
}
1664-
value = jsgraph()->Constant(bit_cast<double>(value_bits));
1659+
if (boilerplate_value.IsJSObject()) {
1660+
JSObjectRef boilerplate_object = boilerplate_value.AsJSObject();
1661+
value = effect =
1662+
AllocateFastLiteral(effect, control, boilerplate_object, allocation);
1663+
} else if (property_details.representation().IsDouble()) {
1664+
double number = boilerplate_value.AsHeapNumber().value();
1665+
// Allocate a mutable HeapNumber box and store the value into it.
1666+
AllocationBuilder builder(jsgraph(), effect, control);
1667+
builder.Allocate(HeapNumber::kSize, allocation);
1668+
builder.Store(AccessBuilder::ForMap(),
1669+
MapRef(broker(), factory()->heap_number_map()));
1670+
builder.Store(AccessBuilder::ForHeapNumberValue(),
1671+
jsgraph()->Constant(number));
1672+
value = effect = builder.Finish();
1673+
} else if (property_details.representation().IsSmi()) {
1674+
// Ensure that value is stored as smi.
1675+
value = is_uninitialized ? jsgraph()->ZeroConstant()
1676+
: jsgraph()->Constant(boilerplate_value.AsSmi());
16651677
} else {
1666-
ObjectRef boilerplate_value = boilerplate.RawFastPropertyAt(index);
1667-
bool is_uninitialized =
1668-
boilerplate_value.IsHeapObject() &&
1669-
boilerplate_value.AsHeapObject().map().oddball_type() ==
1670-
OddballType::kUninitialized;
1671-
if (is_uninitialized) {
1672-
access.const_field_info = ConstFieldInfo::None();
1673-
}
1674-
if (boilerplate_value.IsJSObject()) {
1675-
JSObjectRef boilerplate_object = boilerplate_value.AsJSObject();
1676-
value = effect = AllocateFastLiteral(effect, control,
1677-
boilerplate_object, allocation);
1678-
} else if (property_details.representation().IsDouble()) {
1679-
double number = boilerplate_value.AsHeapNumber().value();
1680-
// Allocate a mutable HeapNumber box and store the value into it.
1681-
AllocationBuilder builder(jsgraph(), effect, control);
1682-
builder.Allocate(HeapNumber::kSize, allocation);
1683-
builder.Store(AccessBuilder::ForMap(),
1684-
MapRef(broker(), factory()->heap_number_map()));
1685-
builder.Store(AccessBuilder::ForHeapNumberValue(),
1686-
jsgraph()->Constant(number));
1687-
value = effect = builder.Finish();
1688-
} else if (property_details.representation().IsSmi()) {
1689-
// Ensure that value is stored as smi.
1690-
value = is_uninitialized
1691-
? jsgraph()->ZeroConstant()
1692-
: jsgraph()->Constant(boilerplate_value.AsSmi());
1693-
} else {
1694-
value = jsgraph()->Constant(boilerplate_value);
1695-
}
1678+
value = jsgraph()->Constant(boilerplate_value);
16961679
}
16971680
inobject_fields.push_back(std::make_pair(access, value));
16981681
}

‎src/compiler/js-heap-broker.cc‎

Lines changed: 17 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,6 @@ bool IsFastLiteralHelper(Handle<JSObject> boilerplate, int max_depth,
950950
DCHECK_EQ(kData, details.kind());
951951
if ((*max_properties)-- == 0) return false;
952952
FieldIndex field_index = FieldIndex::ForDescriptor(boilerplate->map(), i);
953-
if (boilerplate->IsUnboxedDoubleField(field_index)) continue;
954953
Handle<Object> value(boilerplate->RawFastPropertyAt(field_index), isolate);
955954
if (value->IsJSObject()) {
956955
Handle<JSObject> value_object = Handle<JSObject>::cast(value);
@@ -1042,7 +1041,6 @@ struct PropertyDescriptor {
10421041
FieldIndex field_index;
10431042
ObjectData* field_owner = nullptr;
10441043
ObjectData* field_type = nullptr;
1045-
bool is_unboxed_double_field = false;
10461044
};
10471045

10481046
class MapData : public HeapObjectData {
@@ -1397,10 +1395,6 @@ class DescriptorArrayData : public HeapObjectData {
13971395
return contents_.at(descriptor_index.as_int()).field_type;
13981396
}
13991397

1400-
bool IsUnboxedDoubleField(InternalIndex descriptor_index) const {
1401-
return contents_.at(descriptor_index.as_int()).is_unboxed_double_field;
1402-
}
1403-
14041398
ObjectData* GetStrongValue(InternalIndex descriptor_index) const {
14051399
return contents_.at(descriptor_index.as_int()).value;
14061400
}
@@ -1440,7 +1434,6 @@ void DescriptorArrayData::SerializeDescriptor(JSHeapBroker* broker,
14401434
broker->GetOrCreateData(map->FindFieldOwner(isolate, descriptor_index));
14411435
d.field_type =
14421436
broker->GetOrCreateData(descriptors->GetFieldType(descriptor_index));
1443-
d.is_unboxed_double_field = map->IsUnboxedDoubleField(d.field_index);
14441437
}
14451438
contents_[descriptor_index.as_int()] = d;
14461439

@@ -2349,33 +2342,24 @@ void JSObjectData::SerializeRecursiveAsBoilerplate(JSHeapBroker* broker,
23492342
// this field.
23502343
DCHECK_EQ(field_index.property_index(),
23512344
static_cast<int>(inobject_fields_.size()));
2352-
if (boilerplate->IsUnboxedDoubleField(field_index)) {
2353-
uint64_t value_bits =
2354-
boilerplate->RawFastDoublePropertyAsBitsAt(field_index);
2355-
inobject_fields_.push_back(JSObjectField{value_bits});
2356-
} else {
2357-
Handle<Object> value(boilerplate->RawFastPropertyAt(field_index),
2358-
isolate);
2359-
// In case of double fields we use a sentinel NaN value to mark
2360-
// uninitialized fields. A boilerplate value with such a field may migrate
2361-
// from its double to a tagged representation. If the double is unboxed,
2362-
// the raw double is converted to a heap number, otherwise the (boxed)
2363-
// double ceases to be mutable, and becomes a normal heap number. The
2364-
// sentinel value carries no special meaning when it occurs in a heap
2365-
// number, so we would like to recover the uninitialized value. We check
2366-
// for the sentinel here, specifically, since migrations might have been
2367-
// triggered as part of boilerplate serialization.
2368-
if (!details.representation().IsDouble() && value->IsHeapNumber() &&
2369-
HeapNumber::cast(*value).value_as_bits() == kHoleNanInt64) {
2370-
value = isolate->factory()->uninitialized_value();
2371-
}
2372-
ObjectData* value_data = broker->GetOrCreateData(value);
2373-
if (value_data->IsJSObject() && !value_data->should_access_heap()) {
2374-
value_data->AsJSObject()->SerializeRecursiveAsBoilerplate(broker,
2375-
depth - 1);
2376-
}
2377-
inobject_fields_.push_back(JSObjectField{value_data});
2345+
Handle<Object> value(boilerplate->RawFastPropertyAt(field_index), isolate);
2346+
// In case of double fields we use a sentinel NaN value to mark
2347+
// uninitialized fields. A boilerplate value with such a field may migrate
2348+
// from its double to a tagged representation. The sentinel value carries
2349+
// no special meaning when it occurs in a heap number, so we would like to
2350+
// recover the uninitialized value. We check for the sentinel here,
2351+
// specifically, since migrations might have been triggered as part of
2352+
// boilerplate serialization.
2353+
if (!details.representation().IsDouble() && value->IsHeapNumber() &&
2354+
HeapNumber::cast(*value).value_as_bits() == kHoleNanInt64) {
2355+
value = isolate->factory()->uninitialized_value();
2356+
}
2357+
ObjectData* value_data = broker->GetOrCreateData(value);
2358+
if (value_data->IsJSObject() && !value_data->should_access_heap()) {
2359+
value_data->AsJSObject()->SerializeRecursiveAsBoilerplate(broker,
2360+
depth - 1);
23782361
}
2362+
inobject_fields_.push_back(JSObjectField{value_data});
23792363
}
23802364
TRACE(broker, "Copied " << inobject_fields_.size() << " in-object fields");
23812365

@@ -3079,24 +3063,6 @@ FeedbackCellRef FeedbackVectorRef::GetClosureFeedbackCell(int index) const {
30793063
data()->AsFeedbackVector()->GetClosureFeedbackCell(broker(), index));
30803064
}
30813065

3082-
double JSObjectRef::RawFastDoublePropertyAt(FieldIndex index) const {
3083-
if (data_->should_access_heap()) {
3084-
return object()->RawFastDoublePropertyAt(index);
3085-
}
3086-
JSObjectData* object_data = data()->AsJSObject();
3087-
CHECK(index.is_inobject());
3088-
return object_data->GetInobjectField(index.property_index()).AsDouble();
3089-
}
3090-
3091-
uint64_t JSObjectRef::RawFastDoublePropertyAsBitsAt(FieldIndex index) const {
3092-
if (data_->should_access_heap()) {
3093-
return object()->RawFastDoublePropertyAsBitsAt(index);
3094-
}
3095-
JSObjectData* object_data = data()->AsJSObject();
3096-
CHECK(index.is_inobject());
3097-
return object_data->GetInobjectField(index.property_index()).AsBitsOfDouble();
3098-
}
3099-
31003066
ObjectRef JSObjectRef::RawFastPropertyAt(FieldIndex index) const {
31013067
if (data_->should_access_heap()) {
31023068
return ObjectRef(broker(), broker()->CanonicalPersistentHandle(
@@ -3213,17 +3179,6 @@ ObjectRef MapRef::GetFieldType(InternalIndex descriptor_index) const {
32133179
return ObjectRef(broker(), descriptors->GetFieldType(descriptor_index));
32143180
}
32153181

3216-
bool MapRef::IsUnboxedDoubleField(InternalIndex descriptor_index) const {
3217-
CHECK_LT(descriptor_index.as_int(), NumberOfOwnDescriptors());
3218-
if (data_->should_access_heap()) {
3219-
return object()->IsUnboxedDoubleField(
3220-
FieldIndex::ForDescriptor(*object(), descriptor_index));
3221-
}
3222-
DescriptorArrayData* descriptors =
3223-
data()->AsMap()->instance_descriptors()->AsDescriptorArray();
3224-
return descriptors->IsUnboxedDoubleField(descriptor_index);
3225-
}
3226-
32273182
base::Optional<int> StringRef::length() const {
32283183
if (data_->should_access_heap()) {
32293184
if (data_->kind() == kNeverSerializedHeapObject &&

0 commit comments

Comments
 (0)