Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(166)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/crankshaft/hydrogen.cc

Issue 2408133002: Revert of Speedup access to global_proxy.* attributes/accessors. (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/crankshaft/hydrogen.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/crankshaft/hydrogen.cc
diff --git a/src/crankshaft/hydrogen.cc b/src/crankshaft/hydrogen.cc
index 8576ef2d34f8b0519d51e5ced194c232b722475b..19e83ef36ebf8b82e9a11fc2c21e39b3dce8e0d8 100644
--- a/src/crankshaft/hydrogen.cc
+++ b/src/crankshaft/hydrogen.cc
@@ -5448,16 +5448,12 @@
}
}
-bool HOptimizedGraphBuilder::CanInlineGlobalPropertyAccess(
- Variable* var, LookupIterator* it, PropertyAccessType access_type) {
- if (var->is_this()) return false;
- return CanInlineGlobalPropertyAccess(it, access_type);
-}
-
-bool HOptimizedGraphBuilder::CanInlineGlobalPropertyAccess(
- LookupIterator* it, PropertyAccessType access_type) {
- if (!current_info()->has_global_object()) {
- return false;
+
+HOptimizedGraphBuilder::GlobalPropertyAccess
+HOptimizedGraphBuilder::LookupGlobalProperty(Variable* var, LookupIterator* it,
+ PropertyAccessType access_type) {
+ if (var->is_this() || !current_info()->has_global_object()) {
+ return kUseGeneric;
}
switch (it->state()) {
@@ -5466,17 +5462,17 @@
case LookupIterator::INTERCEPTOR:
case LookupIterator::INTEGER_INDEXED_EXOTIC:
case LookupIterator::NOT_FOUND:
- return false;
+ return kUseGeneric;
case LookupIterator::DATA:
- if (access_type == STORE && it->IsReadOnly()) return false;
- if (!it->GetHolder<JSObject>()->IsJSGlobalObject()) return false;
- return true;
+ if (access_type == STORE && it->IsReadOnly()) return kUseGeneric;
+ if (!it->GetHolder<JSObject>()->IsJSGlobalObject()) return kUseGeneric;
+ return kUseCell;
case LookupIterator::JSPROXY:
case LookupIterator::TRANSITION:
UNREACHABLE();
}
UNREACHABLE();
- return false;
+ return kUseGeneric;
}
@@ -5492,55 +5488,6 @@
return context;
}
-void HOptimizedGraphBuilder::InlineGlobalPropertyLoad(LookupIterator* it,
- BailoutId ast_id) {
- 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) {
- Handle<Object> constant_object(cell->value(), isolate());
- if (constant_object->IsConsString()) {
- constant_object = String::Flatten(Handle<String>::cast(constant_object));
- }
- HConstant* constant = New<HConstant>(constant_object);
- return ast_context()->ReturnInstruction(constant, ast_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. In that case,
- // make no promises about the loaded value except that it's a
- // heap object.
- access = access.WithRepresentation(Representation::HeapObject());
- Handle<Map> map(HeapObject::cast(cell->value())->map());
- if (map->is_stable()) {
- field_maps = new (zone())
- UniqueSet<Map>(Unique<Map>::CreateImmovable(map), zone());
- }
- break;
- }
- }
- }
- 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());
- }
- instr->ClearDependsOnFlag(kInobjectFields);
- instr->SetDependsOnFlag(kGlobalVars);
- return ast_context()->ReturnInstruction(instr, ast_id);
- }
-}
void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
DCHECK(!HasStackOverflow());
@@ -5589,9 +5536,57 @@
}
LookupIterator it(global, variable->name(), LookupIterator::OWN);
- if (CanInlineGlobalPropertyAccess(variable, &it, LOAD)) {
- InlineGlobalPropertyLoad(&it, expr->id());
- return;
+ GlobalPropertyAccess type = LookupGlobalProperty(variable, &it, LOAD);
+
+ 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) {
+ Handle<Object> constant_object(cell->value(), isolate());
+ if (constant_object->IsConsString()) {
+ constant_object =
+ String::Flatten(Handle<String>::cast(constant_object));
+ }
+ 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. In that case,
+ // make no promises about the loaded value except that it's a
+ // heap object.
+ access =
+ access.WithRepresentation(Representation::HeapObject());
+ Handle<Map> map(HeapObject::cast(cell->value())->map());
+ if (map->is_stable()) {
+ field_maps = new (zone())
+ UniqueSet<Map>(Unique<Map>::CreateImmovable(map), zone());
+ }
+ break;
+ }
+ }
+ }
+ 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());
+ }
+ instr->ClearDependsOnFlag(kInobjectFields);
+ instr->SetDependsOnFlag(kGlobalVars);
+ return ast_context()->ReturnInstruction(instr, expr->id());
+ }
} else {
Handle<TypeFeedbackVector> vector(current_feedback_vector(), isolate());
@@ -6694,61 +6689,6 @@
expr->AssignmentId(), expr->IsUninitialized());
}
-void HOptimizedGraphBuilder::InlineGlobalPropertyStore(LookupIterator* it,
- HValue* value,
- BailoutId ast_id) {
- 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) {
- Handle<Object> constant(cell->value(), isolate());
- if (value->IsConstant()) {
- HConstant* c_value = HConstant::cast(value);
- if (!constant.is_identical_to(c_value->handle(isolate()))) {
- Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
- Deoptimizer::EAGER);
- }
- } else {
- HValue* c_constant = Add<HConstant>(constant);
- IfBuilder builder(this);
- if (constant->IsNumber()) {
- builder.If<HCompareNumericAndBranch>(value, c_constant, Token::EQ);
- } else {
- builder.If<HCompareObjectEqAndBranch>(value, c_constant);
- }
- builder.Then();
- builder.Else();
- Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
- Deoptimizer::EAGER);
- builder.End();
- }
- }
- 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: {
- // 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> map(HeapObject::cast(cell->value())->map());
- Add<HCheckHeapObject>(value);
- value = Add<HCheckMaps>(value, map);
- access = access.WithRepresentation(Representation::HeapObject());
- break;
- }
- }
- }
- HInstruction* instr = Add<HStoreNamedField>(cell_constant, access, value);
- instr->ClearChangesFlag(kInobjectFields);
- instr->SetChangesFlag(kGlobalVars);
- if (instr->HasObservableSideEffects()) {
- Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
- }
-}
// Because not every expression has a position and there is not common
// superclass of Assignment and CountOperation, we cannot just pass the
@@ -6789,8 +6729,59 @@
}
LookupIterator it(global, var->name(), LookupIterator::OWN);
- if (CanInlineGlobalPropertyAccess(var, &it, STORE)) {
- InlineGlobalPropertyStore(&it, value, ast_id);
+ GlobalPropertyAccess type = LookupGlobalProperty(var, &it, STORE);
+ 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) {
+ Handle<Object> constant(cell->value(), isolate());
+ if (value->IsConstant()) {
+ HConstant* c_value = HConstant::cast(value);
+ if (!constant.is_identical_to(c_value->handle(isolate()))) {
+ Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
+ Deoptimizer::EAGER);
+ }
+ } else {
+ HValue* c_constant = Add<HConstant>(constant);
+ IfBuilder builder(this);
+ if (constant->IsNumber()) {
+ builder.If<HCompareNumericAndBranch>(value, c_constant, Token::EQ);
+ } else {
+ builder.If<HCompareObjectEqAndBranch>(value, c_constant);
+ }
+ builder.Then();
+ builder.Else();
+ Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
+ Deoptimizer::EAGER);
+ builder.End();
+ }
+ }
+ 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: {
+ // 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> map(HeapObject::cast(cell->value())->map());
+ Add<HCheckHeapObject>(value);
+ value = Add<HCheckMaps>(value, map);
+ access = access.WithRepresentation(Representation::HeapObject());
+ break;
+ }
+ }
+ }
+ HInstruction* instr = Add<HStoreNamedField>(cell_constant, access, value);
+ instr->ClearChangesFlag(kInobjectFields);
+ instr->SetChangesFlag(kGlobalVars);
+ if (instr->HasObservableSideEffects()) {
+ Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
+ }
} else {
HValue* global_object = Add<HLoadNamedField>(
BuildGetNativeContext(), nullptr,
@@ -7705,37 +7696,6 @@
DCHECK(maps != NULL);
if (maps->length() > 0) {
- Handle<JSGlobalObject> global_object(current_info()->global_object());
- Handle<Context> current_context(current_info()->context());
- Handle<JSObject> global_proxy(current_context->global_proxy());
-
- // Check for special case: Access via a single map to the global proxy
- // can also be handled monomorphically.
- Handle<Object> map_constructor =
- handle(maps->first()->GetConstructor(), isolate());
- if (map_constructor->IsJSFunction()) {
- Handle<Context> map_context =
- handle(Handle<JSFunction>::cast(map_constructor)->context());
- bool is_global_proxy_access =
- maps->length() == 1 && // More than one map, fallback to polymorphic?
- maps->first()->IsJSGlobalProxyMap() &&
- isolate()->MayAccess(map_context, global_proxy);
-
- if (is_global_proxy_access) {
- LookupIterator it(global_object, name, LookupIterator::OWN);
- if (CanInlineGlobalPropertyAccess(&it, access)) {
- BuildCheckHeapObject(object);
- Add<HCheckMaps>(object, maps);
- if (access == LOAD) {
- InlineGlobalPropertyLoad(&it, expr->id());
- } else {
- InlineGlobalPropertyStore(&it, value, expr->id());
- }
- return nullptr;
- }
- }
- }
-
PropertyAccessInfo info(this, access, maps->first(), name);
if (!info.CanAccessAsMonomorphic(maps)) {
HandlePolymorphicNamedFieldAccess(access, expr, slot, ast_id, return_id,
« no previous file with comments | « src/crankshaft/hydrogen.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698