diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 3ca301ca6c..4bec33f914 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()); - Handle placeholder_map = isolate()->factory()->meta_map(); - HValue* cell = Add(Map::WeakCellForMap(placeholder_map)); - HValue* expected_map = - Add(cell, nullptr, HObjectAccess::ForWeakCellValue()); + HValue* map_cell = Add(isolate()->factory()->NewWeakCell( + StoreGlobalStub::global_map_placeholder(isolate()))); + HValue* expected_map = Add( + map_cell, nullptr, HObjectAccess::ForWeakCellValue()); HValue* map = Add(global, nullptr, HObjectAccess::ForMap()); IfBuilder map_check(this); @@ -1342,9 +1342,15 @@ 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); - if (stub->is_constant()) { + 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. IfBuilder builder(this); builder.If(cell_contents, value); builder.Then(); @@ -1352,15 +1358,40 @@ 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 424026f40b..a363f8607b 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1110,9 +1110,14 @@ class StoreTransitionStub : public HandlerStub { class StoreGlobalStub : public HandlerStub { public: - StoreGlobalStub(Isolate* isolate, bool is_constant, bool check_global) + StoreGlobalStub(Isolate* isolate, PropertyCellType type, + Maybe constant_type, + bool check_global) : HandlerStub(isolate) { - set_sub_minor_key(IsConstantBits::encode(is_constant) | + PropertyCellConstantType encoded_constant_type = + constant_type.FromMaybe(PropertyCellConstantType::kSmi); + set_sub_minor_key(CellTypeBits::encode(type) | + ConstantTypeBits::encode(encoded_constant_type) | CheckGlobalBits::encode(check_global)); } @@ -1120,33 +1125,35 @@ 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()) { - Code::FindAndReplacePattern pattern; - pattern.Add(isolate()->factory()->meta_map(), + pattern.Add(handle(global_map_placeholder(isolate())->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; } - bool is_constant() const { return IsConstantBits::decode(sub_minor_key()); } + 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 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())); @@ -1157,9 +1164,10 @@ class StoreGlobalStub : public HandlerStub { } private: - class IsConstantBits: public BitField {}; - class RepresentationBits: public BitField {}; - class CheckGlobalBits: public BitField {}; + class CellTypeBits : public BitField {}; + class ConstantTypeBits : 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 97d604a7c2..7b91d98461 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5396,8 +5396,10 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) { if (type == kUseCell) { Handle cell = it.GetPropertyCell(); - top_info()->dependencies()->AssumePropertyCell(cell); - if (it.property_details().cell_type() == PropertyCellType::kConstant) { + auto cell_type = it.property_details().cell_type(); + if (cell_type == PropertyCellType::kConstant || + cell_type == PropertyCellType::kUndefined) { + top_info()->dependencies()->AssumePropertyCell(cell); Handle constant_object(cell->value(), isolate()); if (constant_object->IsConsString()) { constant_object = @@ -5406,9 +5408,40 @@ 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 = New( - cell_constant, nullptr, HObjectAccess::ForPropertyCellValue()); + HLoadNamedField* instr; + if (field_maps == nullptr) { + instr = New(cell_constant, nullptr, access); + } else { + instr = New(cell_constant, nullptr, access, + field_maps, HType::HeapObject()); + } instr->ClearDependsOnFlag(kInobjectFields); instr->SetDependsOnFlag(kGlobalVars); return ast_context()->ReturnInstruction(instr, expr->id()); @@ -6563,7 +6596,9 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment( if (type == kUseCell) { Handle cell = it.GetPropertyCell(); top_info()->dependencies()->AssumePropertyCell(cell); - if (it.property_details().cell_type() == PropertyCellType::kConstant) { + auto cell_type = it.property_details().cell_type(); + if (cell_type == PropertyCellType::kConstant || + cell_type == PropertyCellType::kUndefined) { Handle constant(cell->value(), isolate()); if (value->IsConstant()) { HConstant* c_value = HConstant::cast(value); @@ -6587,8 +6622,30 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment( } } HConstant* cell_constant = Add(cell); - HInstruction* instr = Add( - cell_constant, HObjectAccess::ForPropertyCellValue(), value); + 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); instr->ClearChangesFlag(kInobjectFields); instr->SetChangesFlag(kGlobalVars); if (instr->HasObservableSideEffects()) { diff --git a/src/ic/ic.cc b/src/ic/ic.cc index a6ae92a892..ce3a862da1 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -1714,7 +1714,11 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle value, static Handle PropertyCellStoreHandler( Isolate* isolate, Handle receiver, Handle holder, Handle name, Handle cell, PropertyCellType type) { - StoreGlobalStub stub(isolate, type == PropertyCellType::kConstant, + auto constant_type = Nothing(); + if (type == PropertyCellType::kConstantType) { + constant_type = Just(cell->GetConstantType()); + } + StoreGlobalStub stub(isolate, type, constant_type, receiver->IsJSGlobalProxy()); auto code = stub.GetCodeCopyFromTemplate(holder, cell); // TODO(verwaest): Move caching of these NORMAL stubs outside as well. @@ -1815,11 +1819,12 @@ Handle StoreIC::CompileHandler(LookupIterator* lookup, DCHECK(holder.is_identical_to(receiver) || receiver->map()->prototype() == *holder); auto cell = lookup->GetPropertyCell(); - auto union_type = PropertyCell::UpdatedType( + auto updated_type = PropertyCell::UpdatedType( cell, value, lookup->property_details()); - return PropertyCellStoreHandler(isolate(), receiver, - Handle::cast(holder), - lookup->name(), cell, union_type); + auto code = PropertyCellStoreHandler( + isolate(), receiver, Handle::cast(holder), + lookup->name(), cell, updated_type); + return code; } DCHECK(holder.is_identical_to(receiver)); return isolate()->builtins()->StoreIC_Normal(); diff --git a/src/lookup.h b/src/lookup.h index 9556ec92be..ba75d0d203 100644 --- a/src/lookup.h +++ b/src/lookup.h @@ -145,6 +145,12 @@ 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, @@ -176,11 +182,6 @@ 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 f57c569d09..0cd8b71a77 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::kInvalid); + PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell); 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::kInvalid); + PropertyCellType::kNoCell); 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::kInvalid); + PropertyCellType::kNoCell); 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::kInvalid); + PropertyCellType::kNoCell); 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::kInvalid); + PropertyCellType::kNoCell); 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::kDeleted)); + PropertyCellType::kInvalidated)); return; } @@ -6425,7 +6425,7 @@ static bool UpdateGetterSetterInDictionary( if (details.attributes() != attributes) { dictionary->DetailsAtPut( entry, PropertyDetails(attributes, ACCESSOR_CONSTANT, index, - PropertyCellType::kInvalid)); + PropertyCellType::kNoCell)); } 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::kInvalid); + PropertyCellType::kNoCell); // 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::kInvalid); + PropertyCellType::kNoCell); dictionary->DetailsAtPut(entry, details); } @@ -12934,7 +12934,7 @@ MaybeHandle JSObject::SetDictionaryElement( } } - PropertyDetails details(attributes, DATA, 0, PropertyCellType::kInvalid); + PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell); 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::kDeleted); + PropertyCellType::kInvalidated); 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::kDeleted + details = details.set_cell_type(is_the_hole ? PropertyCellType::kInvalidated : PropertyCellType::kMutable); dictionary->DetailsAtPut(entry, details); // Old cell is ready for invalidation. @@ -17016,25 +17016,55 @@ 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()); - DCHECK_IMPLIES(cell->value()->IsTheHole(), - type == PropertyCellType::kUninitialized || - type == PropertyCellType::kDeleted); + 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; + } + } 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; } @@ -17087,8 +17117,7 @@ Handle PropertyCell::UpdateCell(Handle dictionary, cell->set_value(*value); // Deopt when transitioning from a constant type. - if (!invalidate && old_type == PropertyCellType::kConstant && - new_type != PropertyCellType::kConstant) { + if (!invalidate && (old_type != new_type)) { auto isolate = dictionary->GetIsolate(); cell->dependent_code()->DeoptimizeDependentCodeGroup( isolate, DependentCode::kPropertyCellChangedGroup); diff --git a/src/objects.h b/src/objects.h index fd9e8f4933..9076e3ffd4 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9822,6 +9822,8 @@ 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 517e7a9641..791eb524c7 100644 --- a/src/property-details.h +++ b/src/property-details.h @@ -186,12 +186,24 @@ static const int kInvalidEnumCacheSentinel = enum class PropertyCellType { - 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. + // 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, }; @@ -229,7 +241,7 @@ class PropertyDetails BASE_EMBEDDED { } static PropertyDetails Empty() { - return PropertyDetails(NONE, DATA, 0, PropertyCellType::kInvalid); + return PropertyDetails(NONE, DATA, 0, PropertyCellType::kNoCell); } int pointer() const { return DescriptorPointer::decode(value_); }