diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 4bec33f914..3ca301ca6c 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -1324,10 +1324,10 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { Add(proxy, nullptr, HObjectAccess::ForMap()); HValue* global = Add(proxy_map, nullptr, HObjectAccess::ForPrototype()); - HValue* map_cell = Add(isolate()->factory()->NewWeakCell( - StoreGlobalStub::global_map_placeholder(isolate()))); - HValue* expected_map = Add( - map_cell, nullptr, HObjectAccess::ForWeakCellValue()); + Handle placeholder_map = isolate()->factory()->meta_map(); + HValue* cell = Add(Map::WeakCellForMap(placeholder_map)); + HValue* expected_map = + Add(cell, nullptr, HObjectAccess::ForWeakCellValue()); HValue* map = Add(global, nullptr, HObjectAccess::ForMap()); IfBuilder map_check(this); @@ -1342,15 +1342,9 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { HObjectAccess::ForWeakCellValue()); Add(cell); HObjectAccess access = HObjectAccess::ForPropertyCellValue(); - // Load the payload of the global parameter cell. A hole indicates that the - // cell has been invalidated and that the store must be handled by the - // runtime. HValue* cell_contents = Add(cell, nullptr, access); - auto cell_type = stub->cell_type(); - if (cell_type == PropertyCellType::kConstant || - cell_type == PropertyCellType::kUndefined) { - // This is always valid for all states a cell can be in. + if (stub->is_constant()) { IfBuilder builder(this); builder.If(cell_contents, value); builder.Then(); @@ -1358,40 +1352,15 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { Deoptimizer::kUnexpectedCellContentsInConstantGlobalStore); builder.End(); } else { + // Load the payload of the global parameter cell. A hole indicates that the + // property has been deleted and that the store must be handled by the + // runtime. IfBuilder builder(this); HValue* hole_value = graph()->GetConstantHole(); builder.If(cell_contents, hole_value); builder.Then(); builder.Deopt(Deoptimizer::kUnexpectedCellContentsInGlobalStore); builder.Else(); - // When dealing with constant types, the type may be allowed to change, as - // long as optimized code remains valid. - if (cell_type == PropertyCellType::kConstantType) { - switch (stub->constant_type()) { - case PropertyCellConstantType::kSmi: - access = access.WithRepresentation(Representation::Smi()); - break; - case PropertyCellConstantType::kStableMap: { - // It is sufficient here to check that the value and cell contents - // have identical maps, no matter if they are stable or not or if they - // are the maps that were originally in the cell or not. If optimized - // code will deopt when a cell has a unstable map and if it has a - // dependency on a stable map, it will deopt if the map destabilizes. - Add(value); - Add(cell_contents); - HValue* expected_map = Add(cell_contents, nullptr, - HObjectAccess::ForMap()); - HValue* map = - Add(value, nullptr, HObjectAccess::ForMap()); - IfBuilder map_check(this); - map_check.IfNot(expected_map, map); - map_check.ThenDeopt(Deoptimizer::kUnknownMap); - map_check.End(); - access = access.WithRepresentation(Representation::HeapObject()); - break; - } - } - } Add(cell, access, value); builder.End(); } diff --git a/src/code-stubs.h b/src/code-stubs.h index a363f8607b..424026f40b 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1110,14 +1110,9 @@ class StoreTransitionStub : public HandlerStub { class StoreGlobalStub : public HandlerStub { public: - StoreGlobalStub(Isolate* isolate, PropertyCellType type, - Maybe constant_type, - bool check_global) + StoreGlobalStub(Isolate* isolate, bool is_constant, bool check_global) : HandlerStub(isolate) { - PropertyCellConstantType encoded_constant_type = - constant_type.FromMaybe(PropertyCellConstantType::kSmi); - set_sub_minor_key(CellTypeBits::encode(type) | - ConstantTypeBits::encode(encoded_constant_type) | + set_sub_minor_key(IsConstantBits::encode(is_constant) | CheckGlobalBits::encode(check_global)); } @@ -1125,35 +1120,33 @@ class StoreGlobalStub : public HandlerStub { return isolate->factory()->uninitialized_value(); } - static Handle global_map_placeholder(Isolate* isolate) { - return isolate->factory()->termination_exception(); - } - Handle GetCodeCopyFromTemplate(Handle global, Handle cell) { - Code::FindAndReplacePattern pattern; if (check_global()) { - pattern.Add(handle(global_map_placeholder(isolate())->map()), + Code::FindAndReplacePattern pattern; + pattern.Add(isolate()->factory()->meta_map(), Map::WeakCellForMap(Handle(global->map()))); + pattern.Add(Handle(property_cell_placeholder(isolate())->map()), + isolate()->factory()->NewWeakCell(cell)); + return CodeStub::GetCodeCopy(pattern); + } else { + Code::FindAndReplacePattern pattern; + pattern.Add(Handle(property_cell_placeholder(isolate())->map()), + isolate()->factory()->NewWeakCell(cell)); + return CodeStub::GetCodeCopy(pattern); } - pattern.Add(handle(property_cell_placeholder(isolate())->map()), - isolate()->factory()->NewWeakCell(cell)); - return CodeStub::GetCodeCopy(pattern); } Code::Kind kind() const override { return Code::STORE_IC; } - PropertyCellType cell_type() const { - return CellTypeBits::decode(sub_minor_key()); - } - - PropertyCellConstantType constant_type() const { - DCHECK(PropertyCellType::kConstantType == cell_type()); - return ConstantTypeBits::decode(sub_minor_key()); - } + bool is_constant() const { return IsConstantBits::decode(sub_minor_key()); } bool check_global() const { return CheckGlobalBits::decode(sub_minor_key()); } + void set_is_constant(bool value) { + set_sub_minor_key(IsConstantBits::update(sub_minor_key(), value)); + } + Representation representation() { return Representation::FromKind( RepresentationBits::decode(sub_minor_key())); @@ -1164,10 +1157,9 @@ class StoreGlobalStub : public HandlerStub { } private: - class CellTypeBits : public BitField {}; - class ConstantTypeBits : public BitField {}; - class RepresentationBits : public BitField {}; - class CheckGlobalBits : public BitField {}; + class IsConstantBits: public BitField {}; + class RepresentationBits: public BitField {}; + class CheckGlobalBits: public BitField {}; DEFINE_HANDLER_CODE_STUB(StoreGlobal, HandlerStub); }; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index ef9901593d..e7621d4d0f 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5396,10 +5396,8 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) { if (type == kUseCell) { Handle cell = it.GetPropertyCell(); - auto cell_type = it.property_details().cell_type(); - if (cell_type == PropertyCellType::kConstant || - cell_type == PropertyCellType::kUndefined) { - top_info()->dependencies()->AssumePropertyCell(cell); + top_info()->dependencies()->AssumePropertyCell(cell); + if (it.property_details().cell_type() == PropertyCellType::kConstant) { Handle constant_object(cell->value(), isolate()); if (constant_object->IsConsString()) { constant_object = @@ -5408,40 +5406,9 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) { HConstant* constant = New(constant_object); return ast_context()->ReturnInstruction(constant, expr->id()); } else { - auto access = HObjectAccess::ForPropertyCellValue(); - UniqueSet* field_maps = nullptr; - if (cell_type == PropertyCellType::kConstantType) { - switch (cell->GetConstantType()) { - case PropertyCellConstantType::kSmi: - access = access.WithRepresentation(Representation::Smi()); - break; - case PropertyCellConstantType::kStableMap: { - // Check that the map really is stable. The heap object could - // have mutated without the cell updating state. - Handle map(HeapObject::cast(cell->value())->map()); - if (map->is_stable()) { - access = - access.WithRepresentation(Representation::HeapObject()); - field_maps = new (zone()) - UniqueSet(Unique::CreateImmovable(map), zone()); - } else { - auto dictionary = handle(global->property_dictionary()); - cell = PropertyCell::InvalidateEntry(dictionary, - it.dictionary_entry()); - } - break; - } - } - } - top_info()->dependencies()->AssumePropertyCell(cell); HConstant* cell_constant = Add(cell); - HLoadNamedField* instr; - if (field_maps == nullptr) { - instr = New(cell_constant, nullptr, access); - } else { - instr = New(cell_constant, nullptr, access, - field_maps, HType::HeapObject()); - } + HLoadNamedField* instr = New( + cell_constant, nullptr, HObjectAccess::ForPropertyCellValue()); instr->ClearDependsOnFlag(kInobjectFields); instr->SetDependsOnFlag(kGlobalVars); return ast_context()->ReturnInstruction(instr, expr->id()); @@ -6596,9 +6563,7 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment( if (type == kUseCell) { Handle cell = it.GetPropertyCell(); top_info()->dependencies()->AssumePropertyCell(cell); - auto cell_type = it.property_details().cell_type(); - if (cell_type == PropertyCellType::kConstant || - cell_type == PropertyCellType::kUndefined) { + if (it.property_details().cell_type() == PropertyCellType::kConstant) { Handle constant(cell->value(), isolate()); if (value->IsConstant()) { HConstant* c_value = HConstant::cast(value); @@ -6622,30 +6587,8 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment( } } HConstant* cell_constant = Add(cell); - auto access = HObjectAccess::ForPropertyCellValue(); - if (cell_type == PropertyCellType::kConstantType) { - switch (cell->GetConstantType()) { - case PropertyCellConstantType::kSmi: - access = access.WithRepresentation(Representation::Smi()); - break; - case PropertyCellConstantType::kStableMap: { - // Check that the map really is stable. The heap object could have - // mutated without the cell updating state. - Handle map(HeapObject::cast(cell->value())->map()); - if (map->is_stable()) { - Add(value); - value = Add(value, map); - access = access.WithRepresentation(Representation::HeapObject()); - } else { - auto dictionary = handle(global->property_dictionary()); - cell = PropertyCell::InvalidateEntry(dictionary, - it.dictionary_entry()); - } - break; - } - } - } - HInstruction* instr = Add(cell_constant, access, value); + HInstruction* instr = Add( + cell_constant, HObjectAccess::ForPropertyCellValue(), value); instr->ClearChangesFlag(kInobjectFields); instr->SetChangesFlag(kGlobalVars); if (instr->HasObservableSideEffects()) { diff --git a/src/ic/ic.cc b/src/ic/ic.cc index ce3a862da1..a6ae92a892 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -1714,11 +1714,7 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle value, static Handle PropertyCellStoreHandler( Isolate* isolate, Handle receiver, Handle holder, Handle name, Handle cell, PropertyCellType type) { - auto constant_type = Nothing(); - if (type == PropertyCellType::kConstantType) { - constant_type = Just(cell->GetConstantType()); - } - StoreGlobalStub stub(isolate, type, constant_type, + StoreGlobalStub stub(isolate, type == PropertyCellType::kConstant, receiver->IsJSGlobalProxy()); auto code = stub.GetCodeCopyFromTemplate(holder, cell); // TODO(verwaest): Move caching of these NORMAL stubs outside as well. @@ -1819,12 +1815,11 @@ Handle StoreIC::CompileHandler(LookupIterator* lookup, DCHECK(holder.is_identical_to(receiver) || receiver->map()->prototype() == *holder); auto cell = lookup->GetPropertyCell(); - auto updated_type = PropertyCell::UpdatedType( + auto union_type = PropertyCell::UpdatedType( cell, value, lookup->property_details()); - auto code = PropertyCellStoreHandler( - isolate(), receiver, Handle::cast(holder), - lookup->name(), cell, updated_type); - return code; + return PropertyCellStoreHandler(isolate(), receiver, + Handle::cast(holder), + lookup->name(), cell, union_type); } DCHECK(holder.is_identical_to(receiver)); return isolate()->builtins()->StoreIC_Normal(); diff --git a/src/lookup.h b/src/lookup.h index ba75d0d203..9556ec92be 100644 --- a/src/lookup.h +++ b/src/lookup.h @@ -145,12 +145,6 @@ class LookupIterator final BASE_EMBEDDED { Handle WriteDataValue(Handle value); void InternalizeName(); - int dictionary_entry() const { - DCHECK(has_property_); - DCHECK(holder_map_->is_dictionary_map()); - return number_; - } - private: enum class InterceptorState { kUninitialized, @@ -182,6 +176,11 @@ class LookupIterator final BASE_EMBEDDED { DCHECK(!holder_map_->is_dictionary_map()); return number_; } + int dictionary_entry() const { + DCHECK(has_property_); + DCHECK(holder_map_->is_dictionary_map()); + return number_; + } static Configuration ComputeConfiguration( Configuration configuration, Handle name) { diff --git a/src/objects.cc b/src/objects.cc index 0cd8b71a77..f57c569d09 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1786,7 +1786,7 @@ void JSObject::AddSlowProperty(Handle object, DCHECK(!object->HasFastProperties()); Isolate* isolate = object->GetIsolate(); Handle dict(object->property_dictionary()); - PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell); + PropertyDetails details(attributes, DATA, 0, PropertyCellType::kInvalid); if (object->IsGlobalObject()) { int entry = dict->FindEntry(name); // If there's a cell there, just invalidate and set the property. @@ -4592,7 +4592,7 @@ void JSObject::MigrateFastToSlow(Handle object, case DATA_CONSTANT: { Handle value(descs->GetConstant(i), isolate); PropertyDetails d(details.attributes(), DATA, i + 1, - PropertyCellType::kNoCell); + PropertyCellType::kInvalid); dictionary = NameDictionary::Add(dictionary, key, value, d); break; } @@ -4611,7 +4611,7 @@ void JSObject::MigrateFastToSlow(Handle object, } } PropertyDetails d(details.attributes(), DATA, i + 1, - PropertyCellType::kNoCell); + PropertyCellType::kInvalid); dictionary = NameDictionary::Add(dictionary, key, value, d); break; } @@ -4619,14 +4619,14 @@ void JSObject::MigrateFastToSlow(Handle object, FieldIndex index = FieldIndex::ForDescriptor(*map, i); Handle value(object->RawFastPropertyAt(index), isolate); PropertyDetails d(details.attributes(), ACCESSOR_CONSTANT, i + 1, - PropertyCellType::kNoCell); + PropertyCellType::kInvalid); dictionary = NameDictionary::Add(dictionary, key, value, d); break; } case ACCESSOR_CONSTANT: { Handle value(descs->GetCallbacksObject(i), isolate); PropertyDetails d(details.attributes(), ACCESSOR_CONSTANT, i + 1, - PropertyCellType::kNoCell); + PropertyCellType::kInvalid); dictionary = NameDictionary::Add(dictionary, key, value, d); break; } @@ -5351,7 +5351,7 @@ void JSObject::DeleteNormalizedProperty(Handle object, cell->set_value(isolate->heap()->the_hole_value()); // TODO(dcarney): InvalidateForDelete dictionary->DetailsAtPut(entry, dictionary->DetailsAt(entry).set_cell_type( - PropertyCellType::kInvalidated)); + PropertyCellType::kDeleted)); return; } @@ -6425,7 +6425,7 @@ static bool UpdateGetterSetterInDictionary( if (details.attributes() != attributes) { dictionary->DetailsAtPut( entry, PropertyDetails(attributes, ACCESSOR_CONSTANT, index, - PropertyCellType::kNoCell)); + PropertyCellType::kInvalid)); } AccessorPair::cast(result)->SetComponents(getter, setter); return true; @@ -6528,7 +6528,7 @@ void JSObject::SetElementCallback(Handle object, PropertyAttributes attributes) { Heap* heap = object->GetHeap(); PropertyDetails details = PropertyDetails(attributes, ACCESSOR_CONSTANT, 0, - PropertyCellType::kNoCell); + PropertyCellType::kInvalid); // Normalize elements to make this operation simple. bool had_dictionary_elements = object->HasDictionaryElements(); @@ -12891,7 +12891,7 @@ MaybeHandle JSObject::SetDictionaryElement( dictionary->UpdateMaxNumberKey(index); if (set_mode == DEFINE_PROPERTY) { details = PropertyDetails(attributes, DATA, details.dictionary_index(), - PropertyCellType::kNoCell); + PropertyCellType::kInvalid); dictionary->DetailsAtPut(entry, details); } @@ -12934,7 +12934,7 @@ MaybeHandle JSObject::SetDictionaryElement( } } - PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell); + PropertyDetails details(attributes, DATA, 0, PropertyCellType::kInvalid); Handle new_dictionary = SeededNumberDictionary::AddNumberEntry(dictionary, index, value, details); @@ -15366,7 +15366,7 @@ Handle GlobalObject::EnsurePropertyCell( DCHECK(dictionary->DetailsAt(entry).cell_type() == PropertyCellType::kUninitialized || dictionary->DetailsAt(entry).cell_type() == - PropertyCellType::kInvalidated); + PropertyCellType::kDeleted); DCHECK(dictionary->ValueAt(entry)->IsPropertyCell()); cell = handle(PropertyCell::cast(dictionary->ValueAt(entry))); DCHECK(cell->value()->IsTheHole()); @@ -17001,7 +17001,7 @@ Handle PropertyCell::InvalidateEntry( bool is_the_hole = cell->value()->IsTheHole(); // Cell is officially mutable henceforth. auto details = dictionary->DetailsAt(entry); - details = details.set_cell_type(is_the_hole ? PropertyCellType::kInvalidated + details = details.set_cell_type(is_the_hole ? PropertyCellType::kDeleted : PropertyCellType::kMutable); dictionary->DetailsAtPut(entry, details); // Old cell is ready for invalidation. @@ -17016,55 +17016,25 @@ Handle PropertyCell::InvalidateEntry( } -PropertyCellConstantType PropertyCell::GetConstantType() { - if (value()->IsSmi()) return PropertyCellConstantType::kSmi; - return PropertyCellConstantType::kStableMap; -} - - -static bool RemainsConstantType(Handle cell, - Handle value) { - // TODO(dcarney): double->smi and smi->double transition from kConstant - if (cell->value()->IsSmi() && value->IsSmi()) { - return true; - } else if (cell->value()->IsHeapObject() && value->IsHeapObject()) { - return HeapObject::cast(cell->value())->map() == - HeapObject::cast(*value)->map() && - HeapObject::cast(*value)->map()->is_stable(); - } - return false; -} - - PropertyCellType PropertyCell::UpdatedType(Handle cell, Handle value, PropertyDetails details) { PropertyCellType type = details.cell_type(); DCHECK(!value->IsTheHole()); - if (cell->value()->IsTheHole()) { - switch (type) { - // Only allow a cell to transition once into constant state. - case PropertyCellType::kUninitialized: - if (value->IsUndefined()) return PropertyCellType::kUndefined; - return PropertyCellType::kConstant; - case PropertyCellType::kInvalidated: - return PropertyCellType::kMutable; - default: - UNREACHABLE(); - return PropertyCellType::kMutable; - } - } + DCHECK_IMPLIES(cell->value()->IsTheHole(), + type == PropertyCellType::kUninitialized || + type == PropertyCellType::kDeleted); switch (type) { + // Only allow a cell to transition once into constant state. + case PropertyCellType::kUninitialized: + if (value->IsUndefined()) return PropertyCellType::kUndefined; + return PropertyCellType::kConstant; case PropertyCellType::kUndefined: return PropertyCellType::kConstant; case PropertyCellType::kConstant: + // No transition. if (*value == cell->value()) return PropertyCellType::kConstant; // Fall through. - case PropertyCellType::kConstantType: - if (RemainsConstantType(cell, value)) { - return PropertyCellType::kConstantType; - } - // Fall through. case PropertyCellType::kMutable: return PropertyCellType::kMutable; } @@ -17117,7 +17087,8 @@ Handle PropertyCell::UpdateCell(Handle dictionary, cell->set_value(*value); // Deopt when transitioning from a constant type. - if (!invalidate && (old_type != new_type)) { + if (!invalidate && old_type == PropertyCellType::kConstant && + new_type != PropertyCellType::kConstant) { auto isolate = dictionary->GetIsolate(); cell->dependent_code()->DeoptimizeDependentCodeGroup( isolate, DependentCode::kPropertyCellChangedGroup); diff --git a/src/objects.h b/src/objects.h index 9076e3ffd4..fd9e8f4933 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9822,8 +9822,6 @@ class PropertyCell : public HeapObject { // property. DECL_ACCESSORS(dependent_code, DependentCode) - PropertyCellConstantType GetConstantType(); - // Computes the new type of the cell's contents for the given value, but // without actually modifying the details. static PropertyCellType UpdatedType(Handle cell, diff --git a/src/property-details.h b/src/property-details.h index 791eb524c7..517e7a9641 100644 --- a/src/property-details.h +++ b/src/property-details.h @@ -186,24 +186,12 @@ static const int kInvalidEnumCacheSentinel = enum class PropertyCellType { - // Meaningful when a property cell does not contain the hole. - kUndefined, // The PREMONOMORPHIC of property cells. - kConstant, // Cell has been assigned only once. - kConstantType, // Cell has been assigned only one type. - kMutable, // Cell will no longer be tracked as constant. - - // Meaningful when a property cell contains the hole. - kUninitialized = kUndefined, // Cell has never been initialized. - kInvalidated = kConstant, // Cell has been deleted or invalidated. - - // For dictionaries not holding cells. - kNoCell = kMutable, -}; - - -enum class PropertyCellConstantType { - kSmi, - kStableMap, + kUninitialized, // Cell is deleted or not yet defined. + kUndefined, // The PREMONOMORPHIC of property cells. + kConstant, // Cell has been assigned only once. + kMutable, // Cell will no longer be tracked as constant. + kDeleted = kConstant, // like kUninitialized, but for cells already deleted. + kInvalid = kMutable, // For dictionaries not holding cells. }; @@ -241,7 +229,7 @@ class PropertyDetails BASE_EMBEDDED { } static PropertyDetails Empty() { - return PropertyDetails(NONE, DATA, 0, PropertyCellType::kNoCell); + return PropertyDetails(NONE, DATA, 0, PropertyCellType::kInvalid); } int pointer() const { return DescriptorPointer::decode(value_); }