Skip to content

Commit

Permalink
Revert of track global accesses to constant types (patchset #15 id:28…
Browse files Browse the repository at this point in the history 
…0001 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}
  • Loading branch information
@mi-ac
mi-ac authored and Junliang Yan committed Aug 18, 2015
1 parent 50aec0d commit f3aa3ce
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 219 deletions.
47 changes: 8 additions & 39 deletions src/code-stubs-hydrogen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -1342,56 +1342,25 @@ 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();
builder.ElseDeopt(
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();
}
Expand Down
48 changes: 20 additions & 28 deletions src/code-stubs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,50 +1124,43 @@ 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));
}

static Handle<HeapObject> property_cell_placeholder(Isolate* isolate) {
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()));
Expand All @@ -1178,10 +1171,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);
};
Expand Down
71 changes: 7 additions & 64 deletions src/hydrogen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5398,10 +5398,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 =
Expand All @@ -5410,40 +5408,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());
Expand Down Expand Up @@ -6598,9 +6565,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);
Expand All @@ -6624,30 +6589,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()) {
Expand Down
15 changes: 5 additions & 10 deletions src/ic/ic.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down
11 changes: 5 additions & 6 deletions src/lookup.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit f3aa3ce

Please sign in to comment.