From 1d2be2af40d5741741c1614a6ae680dd2c604a6c Mon Sep 17 00:00:00 2001 From: dcarney Date: Mon, 27 Apr 2015 07:01:15 -0700 Subject: [PATCH] Reland: track global accesses to constant types R=verwaest@chromium.org BUG=468620 LOG=N Review URL: https://codereview.chromium.org/1102543002 Cr-Commit-Position: refs/heads/master@{#28081} --- src/code-stubs-hydrogen.cc | 47 ++++++++++++++++++++++++----- src/code-stubs.h | 48 +++++++++++++++++------------- src/hydrogen.cc | 60 +++++++++++++++++++++++++++++++++---- src/ic/ic.cc | 15 ++++++---- src/objects.cc | 73 ++++++++++++++++++++++++++++++++-------------- src/objects.h | 2 ++ src/property-details.h | 26 ++++++++++++----- 7 files changed, 203 insertions(+), 68 deletions(-) diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 65bcd45..5a3c0e0 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -1340,10 +1340,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); @@ -1358,9 +1358,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(); @@ -1368,15 +1374,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 c40bc85..6bfe8b5 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1137,9 +1137,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)); } @@ -1147,33 +1152,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()); } - - bool check_global() const { return CheckGlobalBits::decode(sub_minor_key()); } + PropertyCellType cell_type() const { + return CellTypeBits::decode(sub_minor_key()); + } - void set_is_constant(bool value) { - set_sub_minor_key(IsConstantBits::update(sub_minor_key(), value)); + 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()); } + Representation representation() { return Representation::FromKind( RepresentationBits::decode(sub_minor_key())); @@ -1184,9 +1191,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 756c6ee..57e6396 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5464,7 +5464,9 @@ 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) { Handle constant_object(cell->value(), isolate()); if (constant_object->IsConsString()) { constant_object = @@ -5473,9 +5475,37 @@ 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. In that case, + // make no promises about the loaded value except that it's a + // heap object. + access = + access.WithRepresentation(Representation::HeapObject()); + Handle map(HeapObject::cast(cell->value())->map()); + if (map->is_stable()) { + field_maps = new (zone()) + UniqueSet(Unique::CreateImmovable(map), zone()); + } + break; + } + } + } 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()); @@ -6649,7 +6679,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); @@ -6673,8 +6705,24 @@ 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: { + // The map may no longer be stable, deopt if it's ever different from + // what is currently there, which will allow for restablization. + Handle map(HeapObject::cast(cell->value())->map()); + Add(value); + value = Add(value, map); + access = access.WithRepresentation(Representation::HeapObject()); + 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 6ecc077..2100567 100644 --- a/src/ic/ic.cc +++ b/src/ic/ic.cc @@ -1720,7 +1720,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. @@ -1821,11 +1825,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/objects.cc b/src/objects.cc index 99bf653..8d94786 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. @@ -4618,7 +4618,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; } @@ -4637,7 +4637,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; } @@ -4645,14 +4645,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; } @@ -5382,7 +5382,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; } @@ -6457,7 +6457,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; @@ -6560,7 +6560,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(); @@ -12974,7 +12974,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); } @@ -13017,7 +13017,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); @@ -15464,7 +15464,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()); @@ -17072,7 +17072,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. @@ -17087,25 +17087,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; } @@ -17158,8 +17188,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 8b0477a..c5b235d 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9837,6 +9837,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 517e7a9..791eb52 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_); } -- 2.7.4