Revert of track global accesses to constant types (patchset #15 id:280001 of https://codereview.chromium.org/1062163005/)

Reason for revert:
[Sheriff] Speculative revert for maybe breaking layout tests, e.g.
http://build.chromium.org/p/client.v8/builders/V8-Blink%20Linux%2032/builds/2589

Will reland if it doesn't help.

Original issue's description:
> track global accesses to constant types
>
> BUG=

TBR=verwaest@chromium.org,dcarney@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/1094313003

Cr-Commit-Position: refs/heads/master@{#27982}
This commit is contained in:
machenbach 2015-04-21 11:03:11 -07:00 committed by Commit bot
parent 3a67685803
commit 9974348674
8 changed files with 74 additions and 219 deletions

View File

@ -1324,10 +1324,10 @@ HValue* CodeStubGraphBuilder<StoreGlobalStub>::BuildCodeInitializedStub() {
Add<HLoadNamedField>(proxy, nullptr, HObjectAccess::ForMap());
HValue* global =
Add<HLoadNamedField>(proxy_map, nullptr, HObjectAccess::ForPrototype());
HValue* map_cell = Add<HConstant>(isolate()->factory()->NewWeakCell(
StoreGlobalStub::global_map_placeholder(isolate())));
HValue* expected_map = Add<HLoadNamedField>(
map_cell, nullptr, HObjectAccess::ForWeakCellValue());
Handle<Map> placeholder_map = isolate()->factory()->meta_map();
HValue* cell = Add<HConstant>(Map::WeakCellForMap(placeholder_map));
HValue* expected_map =
Add<HLoadNamedField>(cell, nullptr, HObjectAccess::ForWeakCellValue());
HValue* map =
Add<HLoadNamedField>(global, nullptr, HObjectAccess::ForMap());
IfBuilder map_check(this);
@ -1342,15 +1342,9 @@ HValue* CodeStubGraphBuilder<StoreGlobalStub>::BuildCodeInitializedStub() {
HObjectAccess::ForWeakCellValue());
Add<HCheckHeapObject>(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<HLoadNamedField>(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<HCompareObjectEqAndBranch>(cell_contents, value);
builder.Then();
@ -1358,40 +1352,15 @@ HValue* CodeStubGraphBuilder<StoreGlobalStub>::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<HCompareObjectEqAndBranch>(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<HCheckHeapObject>(value);
Add<HCheckHeapObject>(cell_contents);
HValue* expected_map = Add<HLoadNamedField>(cell_contents, nullptr,
HObjectAccess::ForMap());
HValue* map =
Add<HLoadNamedField>(value, nullptr, HObjectAccess::ForMap());
IfBuilder map_check(this);
map_check.IfNot<HCompareObjectEqAndBranch>(expected_map, map);
map_check.ThenDeopt(Deoptimizer::kUnknownMap);
map_check.End();
access = access.WithRepresentation(Representation::HeapObject());
break;
}
}
}
Add<HStoreNamedField>(cell, access, value);
builder.End();
}

View File

@ -1110,14 +1110,9 @@ class StoreTransitionStub : public HandlerStub {
class StoreGlobalStub : public HandlerStub {
public:
StoreGlobalStub(Isolate* isolate, PropertyCellType type,
Maybe<PropertyCellConstantType> 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<HeapObject> global_map_placeholder(Isolate* isolate) {
return isolate->factory()->termination_exception();
}
Handle<Code> GetCodeCopyFromTemplate(Handle<GlobalObject> global,
Handle<PropertyCell> 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<Map>(global->map())));
pattern.Add(Handle<Map>(property_cell_placeholder(isolate())->map()),
isolate()->factory()->NewWeakCell(cell));
return CodeStub::GetCodeCopy(pattern);
} else {
Code::FindAndReplacePattern pattern;
pattern.Add(Handle<Map>(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<PropertyCellType, 0, 2> {};
class ConstantTypeBits : public BitField<PropertyCellConstantType, 2, 2> {};
class RepresentationBits : public BitField<Representation::Kind, 4, 8> {};
class CheckGlobalBits : public BitField<bool, 12, 1> {};
class IsConstantBits: public BitField<bool, 0, 1> {};
class RepresentationBits: public BitField<Representation::Kind, 1, 8> {};
class CheckGlobalBits: public BitField<bool, 9, 1> {};
DEFINE_HANDLER_CODE_STUB(StoreGlobal, HandlerStub);
};

View File

@ -5396,10 +5396,8 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
if (type == kUseCell) {
Handle<PropertyCell> 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<Object> constant_object(cell->value(), isolate());
if (constant_object->IsConsString()) {
constant_object =
@ -5408,40 +5406,9 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
HConstant* constant = New<HConstant>(constant_object);
return ast_context()->ReturnInstruction(constant, expr->id());
} else {
auto access = HObjectAccess::ForPropertyCellValue();
UniqueSet<Map>* 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> map(HeapObject::cast(cell->value())->map());
if (map->is_stable()) {
access =
access.WithRepresentation(Representation::HeapObject());
field_maps = new (zone())
UniqueSet<Map>(Unique<Map>::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<HConstant>(cell);
HLoadNamedField* instr;
if (field_maps == nullptr) {
instr = New<HLoadNamedField>(cell_constant, nullptr, access);
} else {
instr = New<HLoadNamedField>(cell_constant, nullptr, access,
field_maps, HType::HeapObject());
}
HLoadNamedField* instr = New<HLoadNamedField>(
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<PropertyCell> 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<Object> constant(cell->value(), isolate());
if (value->IsConstant()) {
HConstant* c_value = HConstant::cast(value);
@ -6622,30 +6587,8 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
}
}
HConstant* cell_constant = Add<HConstant>(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> map(HeapObject::cast(cell->value())->map());
if (map->is_stable()) {
Add<HCheckHeapObject>(value);
value = Add<HCheckMaps>(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<HStoreNamedField>(cell_constant, access, value);
HInstruction* instr = Add<HStoreNamedField>(
cell_constant, HObjectAccess::ForPropertyCellValue(), value);
instr->ClearChangesFlag(kInobjectFields);
instr->SetChangesFlag(kGlobalVars);
if (instr->HasObservableSideEffects()) {

View File

@ -1714,11 +1714,7 @@ void StoreIC::UpdateCaches(LookupIterator* lookup, Handle<Object> value,
static Handle<Code> PropertyCellStoreHandler(
Isolate* isolate, Handle<JSObject> receiver, Handle<GlobalObject> holder,
Handle<Name> name, Handle<PropertyCell> cell, PropertyCellType type) {
auto constant_type = Nothing<PropertyCellConstantType>();
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<Code> 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<GlobalObject>::cast(holder),
lookup->name(), cell, updated_type);
return code;
return PropertyCellStoreHandler(isolate(), receiver,
Handle<GlobalObject>::cast(holder),
lookup->name(), cell, union_type);
}
DCHECK(holder.is_identical_to(receiver));
return isolate()->builtins()->StoreIC_Normal();

View File

@ -145,12 +145,6 @@ class LookupIterator final BASE_EMBEDDED {
Handle<Object> WriteDataValue(Handle<Object> 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> name) {

View File

@ -1786,7 +1786,7 @@ void JSObject::AddSlowProperty(Handle<JSObject> object,
DCHECK(!object->HasFastProperties());
Isolate* isolate = object->GetIsolate();
Handle<NameDictionary> 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<JSObject> object,
case DATA_CONSTANT: {
Handle<Object> 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<JSObject> 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<JSObject> object,
FieldIndex index = FieldIndex::ForDescriptor(*map, i);
Handle<Object> 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<Object> 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<JSObject> 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<JSObject> 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<Object> 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<Object> JSObject::SetDictionaryElement(
}
}
PropertyDetails details(attributes, DATA, 0, PropertyCellType::kNoCell);
PropertyDetails details(attributes, DATA, 0, PropertyCellType::kInvalid);
Handle<SeededNumberDictionary> new_dictionary =
SeededNumberDictionary::AddNumberEntry(dictionary, index, value,
details);
@ -15366,7 +15366,7 @@ Handle<PropertyCell> 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> 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> PropertyCell::InvalidateEntry(
}
PropertyCellConstantType PropertyCell::GetConstantType() {
if (value()->IsSmi()) return PropertyCellConstantType::kSmi;
return PropertyCellConstantType::kStableMap;
}
static bool RemainsConstantType(Handle<PropertyCell> cell,
Handle<Object> 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<PropertyCell> cell,
Handle<Object> 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<Object> PropertyCell::UpdateCell(Handle<NameDictionary> 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);

View File

@ -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<PropertyCell> cell,

View File

@ -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_); }