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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/compiler/ast-graph-builder.cc

Issue 1133303006: [turbofan] Add FrameStates before all property accesses. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 5 years, 7 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/compiler/ast-graph-builder.h ('k') | src/compiler/js-generic-lowering.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc
index 629ea18679b1979a42ce09bd6bbc37439b310bfc..2e6dad8ada32fca463bc1b86da099e4989076c2c 100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -632,6 +632,14 @@ static LhsKind DetermineLhsKind(Expression* expr) {
}
+// Gets the bailout id just before reading a variable proxy, but only for
+// unallocated variables.
+static BailoutId BeforeId(VariableProxy* proxy) {
+ return proxy->var()->location() == Variable::UNALLOCATED ? proxy->BeforeId()
+ : BailoutId::None();
+}
+
+
static const char* GetDebugParameterName(Zone* zone, Scope* scope, int index) {
#if DEBUG
const AstRawString* name = scope->parameter(index)->raw_name();
@@ -1643,7 +1651,9 @@ void AstGraphBuilder::VisitClassLiteralContents(ClassLiteral* expr) {
if (expr->scope() != NULL) {
DCHECK_NOT_NULL(expr->class_variable_proxy());
Variable* var = expr->class_variable_proxy()->var();
- BuildVariableAssignment(var, literal, Token::INIT_CONST, BailoutId::None());
+ FrameStateBeforeAndAfter states(this, BailoutId::None());
+ BuildVariableAssignment(states, var, literal, Token::INIT_CONST,
+ BailoutId::None());
}
ast_context()->ProduceValue(literal);
@@ -1671,7 +1681,9 @@ void AstGraphBuilder::VisitConditional(Conditional* expr) {
void AstGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
VectorSlotPair pair = CreateVectorSlotPair(expr->VariableFeedbackSlot());
- Node* value = BuildVariableLoad(expr->var(), expr->id(), pair);
+ FrameStateBeforeAndAfter states(this, BeforeId(expr));
+ Node* value = BuildVariableLoad(states, expr->var(), expr->id(), pair,
+ ast_context()->GetStateCombine());
ast_context()->ProduceValue(value);
}
@@ -1744,11 +1756,13 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
if (key->value()->IsInternalizedString()) {
if (property->emit_store()) {
VisitForValue(property->value());
+ FrameStateBeforeAndAfter states(this, property->value()->id());
Node* value = environment()->Pop();
Handle<Name> name = key->AsPropertyName();
Node* store =
BuildNamedStore(literal, name, value, TypeFeedbackId::None());
- PrepareFrameState(store, key->id());
+ states.AddToNode(store, key->id(),
+ OutputFrameStateCombine::Ignore());
BuildSetHomeObject(value, literal, property->value());
} else {
VisitForEffect(property->value());
@@ -1945,34 +1959,32 @@ void AstGraphBuilder::VisitForInAssignment(Expression* expr, Node* value,
switch (assign_type) {
case VARIABLE: {
Variable* var = expr->AsVariableProxy()->var();
- BuildVariableAssignment(var, value, Token::ASSIGN, bailout_id);
+ FrameStateBeforeAndAfter states(this, BailoutId::None());
+ BuildVariableAssignment(states, var, value, Token::ASSIGN, bailout_id);
break;
}
case NAMED_PROPERTY: {
environment()->Push(value);
VisitForValue(property->obj());
+ FrameStateBeforeAndAfter states(this, property->obj()->id());
Node* object = environment()->Pop();
value = environment()->Pop();
Handle<Name> name = property->key()->AsLiteral()->AsPropertyName();
Node* store =
BuildNamedStore(object, name, value, TypeFeedbackId::None());
- PrepareFrameState(store, bailout_id);
+ states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
break;
}
case KEYED_PROPERTY: {
environment()->Push(value);
VisitForValue(property->obj());
VisitForValue(property->key());
- {
- // TODO(jarin) Provide a real frame state before.
- FrameStateBeforeAndAfter states(this, BailoutId::None());
- Node* key = environment()->Pop();
- Node* object = environment()->Pop();
- value = environment()->Pop();
- Node* store =
- BuildKeyedStore(object, key, value, TypeFeedbackId::None());
- states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
- }
+ FrameStateBeforeAndAfter states(this, property->key()->id());
+ Node* key = environment()->Pop();
+ Node* object = environment()->Pop();
+ value = environment()->Pop();
+ Node* store = BuildKeyedStore(object, key, value, TypeFeedbackId::None());
+ states.AddToNode(store, bailout_id, OutputFrameStateCombine::Ignore());
break;
}
}
@@ -2018,7 +2030,10 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) {
VariableProxy* proxy = expr->target()->AsVariableProxy();
VectorSlotPair pair =
CreateVectorSlotPair(proxy->VariableFeedbackSlot());
- old_value = BuildVariableLoad(proxy->var(), expr->target()->id(), pair);
+ FrameStateBeforeAndAfter states(this, BeforeId(proxy));
+ old_value =
+ BuildVariableLoad(states, proxy->var(), expr->target()->id(), pair,
+ OutputFrameStateCombine::Push());
break;
}
case NAMED_PROPERTY: {
@@ -2026,10 +2041,11 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) {
Handle<Name> name = property->key()->AsLiteral()->AsPropertyName();
VectorSlotPair pair =
CreateVectorSlotPair(property->PropertyFeedbackSlot());
+ FrameStateBeforeAndAfter states(this, property->obj()->id());
old_value =
BuildNamedLoad(object, name, pair, property->PropertyFeedbackId());
- PrepareFrameState(old_value, property->LoadId(),
- OutputFrameStateCombine::Push());
+ states.AddToNode(old_value, property->LoadId(),
+ OutputFrameStateCombine::Push());
break;
}
case KEYED_PROPERTY: {
@@ -2037,10 +2053,11 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) {
Node* object = environment()->Peek(1);
VectorSlotPair pair =
CreateVectorSlotPair(property->PropertyFeedbackSlot());
+ FrameStateBeforeAndAfter states(this, property->key()->id());
old_value =
BuildKeyedLoad(object, key, pair, property->PropertyFeedbackId());
- PrepareFrameState(old_value, property->LoadId(),
- OutputFrameStateCombine::Push());
+ states.AddToNode(old_value, property->LoadId(),
+ OutputFrameStateCombine::Push());
break;
}
}
@@ -2072,8 +2089,8 @@ void AstGraphBuilder::VisitAssignment(Assignment* expr) {
switch (assign_type) {
case VARIABLE: {
Variable* variable = expr->target()->AsVariableProxy()->var();
- BuildVariableAssignment(variable, value, expr->op(), expr->id(),
- ast_context()->GetStateCombine());
+ BuildVariableAssignment(store_states, variable, value, expr->op(),
+ expr->id(), ast_context()->GetStateCombine());
break;
}
case NAMED_PROPERTY: {
@@ -2120,17 +2137,20 @@ void AstGraphBuilder::VisitProperty(Property* expr) {
VectorSlotPair pair = CreateVectorSlotPair(expr->PropertyFeedbackSlot());
if (expr->key()->IsPropertyName()) {
VisitForValue(expr->obj());
+ FrameStateBeforeAndAfter states(this, expr->obj()->id());
Node* object = environment()->Pop();
Handle<Name> name = expr->key()->AsLiteral()->AsPropertyName();
value = BuildNamedLoad(object, name, pair, expr->PropertyFeedbackId());
+ states.AddToNode(value, expr->id(), ast_context()->GetStateCombine());
} else {
VisitForValue(expr->obj());
VisitForValue(expr->key());
+ FrameStateBeforeAndAfter states(this, expr->key()->id());
Node* key = environment()->Pop();
Node* object = environment()->Pop();
value = BuildKeyedLoad(object, key, pair, expr->PropertyFeedbackId());
+ states.AddToNode(value, expr->id(), ast_context()->GetStateCombine());
}
- PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
ast_context()->ProduceValue(value);
}
@@ -2149,8 +2169,10 @@ void AstGraphBuilder::VisitCall(Call* expr) {
case Call::GLOBAL_CALL: {
VariableProxy* proxy = callee->AsVariableProxy();
VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot());
+ FrameStateBeforeAndAfter states(this, BeforeId(proxy));
callee_value =
- BuildVariableLoad(proxy->var(), expr->expression()->id(), pair);
+ BuildVariableLoad(states, proxy->var(), expr->expression()->id(),
+ pair, OutputFrameStateCombine::Push());
receiver_value = jsgraph()->UndefinedConstant();
break;
}
@@ -2175,17 +2197,21 @@ void AstGraphBuilder::VisitCall(Call* expr) {
VectorSlotPair pair =
CreateVectorSlotPair(property->PropertyFeedbackSlot());
if (property->key()->IsPropertyName()) {
+ FrameStateBeforeAndAfter states(this, property->obj()->id());
Handle<Name> name = property->key()->AsLiteral()->AsPropertyName();
callee_value =
BuildNamedLoad(object, name, pair, property->PropertyFeedbackId());
+ states.AddToNode(callee_value, property->LoadId(),
+ OutputFrameStateCombine::Push());
} else {
VisitForValue(property->key());
+ FrameStateBeforeAndAfter states(this, property->key()->id());
Node* key = environment()->Pop();
callee_value =
BuildKeyedLoad(object, key, pair, property->PropertyFeedbackId());
+ states.AddToNode(callee_value, property->LoadId(),
+ OutputFrameStateCombine::Push());
}
- PrepareFrameState(callee_value, property->LoadId(),
- OutputFrameStateCombine::Push());
receiver_value = environment()->Pop();
// Note that a PROPERTY_CALL requires the receiver to be wrapped into an
// object for sloppy callees. This could also be modeled explicitly here,
@@ -2295,12 +2321,12 @@ void AstGraphBuilder::VisitCallJSRuntime(CallRuntime* expr) {
CallFunctionFlags flags = NO_CALL_FUNCTION_FLAGS;
Node* receiver_value = BuildLoadBuiltinsObject();
VectorSlotPair pair = CreateVectorSlotPair(expr->CallRuntimeFeedbackSlot());
+ // TODO(jarin): bailout ids for runtime calls.
+ FrameStateBeforeAndAfter states(this, BailoutId::None());
Node* callee_value =
BuildNamedLoad(receiver_value, name, pair, expr->CallRuntimeFeedbackId());
- // TODO(jarin): Find/create a bailout id to deoptimize to (crankshaft
- // refuses to optimize functions with jsruntime calls).
- PrepareFrameState(callee_value, BailoutId::None(),
- OutputFrameStateCombine::Push());
+ states.AddToNode(callee_value, BailoutId::None(),
+ OutputFrameStateCombine::Push());
environment()->Push(callee_value);
environment()->Push(receiver_value);
@@ -2374,35 +2400,39 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
case VARIABLE: {
VariableProxy* proxy = expr->expression()->AsVariableProxy();
VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot());
+ FrameStateBeforeAndAfter states(this, BeforeId(proxy));
old_value =
- BuildVariableLoad(proxy->var(), expr->expression()->id(), pair);
+ BuildVariableLoad(states, proxy->var(), expr->expression()->id(),
+ pair, OutputFrameStateCombine::Push());
stack_depth = 0;
break;
}
case NAMED_PROPERTY: {
VisitForValue(property->obj());
+ FrameStateBeforeAndAfter states(this, property->obj()->id());
Node* object = environment()->Top();
Handle<Name> name = property->key()->AsLiteral()->AsPropertyName();
VectorSlotPair pair =
CreateVectorSlotPair(property->PropertyFeedbackSlot());
old_value =
BuildNamedLoad(object, name, pair, property->PropertyFeedbackId());
- PrepareFrameState(old_value, property->LoadId(),
- OutputFrameStateCombine::Push());
+ states.AddToNode(old_value, property->LoadId(),
+ OutputFrameStateCombine::Push());
stack_depth = 1;
break;
}
case KEYED_PROPERTY: {
VisitForValue(property->obj());
VisitForValue(property->key());
+ FrameStateBeforeAndAfter states(this, property->key()->id());
Node* key = environment()->Top();
Node* object = environment()->Peek(1);
VectorSlotPair pair =
CreateVectorSlotPair(property->PropertyFeedbackSlot());
old_value =
BuildKeyedLoad(object, key, pair, property->PropertyFeedbackId());
- PrepareFrameState(old_value, property->LoadId(),
- OutputFrameStateCombine::Push());
+ states.AddToNode(old_value, property->LoadId(),
+ OutputFrameStateCombine::Push());
stack_depth = 2;
break;
}
@@ -2438,7 +2468,7 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
case VARIABLE: {
Variable* variable = expr->expression()->AsVariableProxy()->var();
environment()->Push(value);
- BuildVariableAssignment(variable, value, expr->op(),
+ BuildVariableAssignment(store_states, variable, value, expr->op(),
expr->AssignmentId());
environment()->Pop();
break;
@@ -2449,7 +2479,8 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
Node* store =
BuildNamedStore(object, name, value, expr->CountStoreFeedbackId());
environment()->Push(value);
- PrepareFrameState(store, expr->AssignmentId());
+ store_states.AddToNode(store, expr->AssignmentId(),
+ OutputFrameStateCombine::Ignore());
environment()->Pop();
break;
}
@@ -2639,8 +2670,10 @@ void AstGraphBuilder::VisitTypeof(UnaryOperation* expr) {
// perform a non-contextual load in case the operand is a variable proxy.
VariableProxy* proxy = expr->expression()->AsVariableProxy();
VectorSlotPair pair = CreateVectorSlotPair(proxy->VariableFeedbackSlot());
- operand = BuildVariableLoad(proxy->var(), expr->expression()->id(), pair,
- NOT_CONTEXTUAL);
+ FrameStateBeforeAndAfter states(this, BeforeId(proxy));
+ operand =
+ BuildVariableLoad(states, proxy->var(), expr->expression()->id(), pair,
+ OutputFrameStateCombine::Push(), NOT_CONTEXTUAL);
} else {
VisitForValue(expr->expression());
operand = environment()->Pop();
@@ -2810,7 +2843,9 @@ Node* AstGraphBuilder::BuildArgumentsObject(Variable* arguments) {
// Assign the object to the arguments variable.
DCHECK(arguments->IsContextSlot() || arguments->IsStackAllocated());
// This should never lazy deopt, so it is fine to send invalid bailout id.
- BuildVariableAssignment(arguments, object, Token::ASSIGN, BailoutId::None());
+ FrameStateBeforeAndAfter states(this, BailoutId::None());
+ BuildVariableAssignment(states, arguments, object, Token::ASSIGN,
+ BailoutId::None());
return object;
}
@@ -2826,7 +2861,9 @@ Node* AstGraphBuilder::BuildRestArgumentsArray(Variable* rest, int index) {
// Assign the object to the rest array
DCHECK(rest->IsContextSlot() || rest->IsStackAllocated());
// This should never lazy deopt, so it is fine to send invalid bailout id.
- BuildVariableAssignment(rest, object, Token::ASSIGN, BailoutId::None());
+ FrameStateBeforeAndAfter states(this, BailoutId::None());
+ BuildVariableAssignment(states, rest, object, Token::ASSIGN,
+ BailoutId::None());
return object;
}
@@ -2875,9 +2912,11 @@ Node* AstGraphBuilder::BuildThrowIfStaticPrototype(Node* name,
}
-Node* AstGraphBuilder::BuildVariableLoad(Variable* variable,
+Node* AstGraphBuilder::BuildVariableLoad(FrameStateBeforeAndAfter& states,
+ Variable* variable,
BailoutId bailout_id,
const VectorSlotPair& feedback,
+ OutputFrameStateCombine combine,
ContextualMode contextual_mode) {
Node* the_hole = jsgraph()->TheHoleConstant();
VariableMode mode = variable->mode();
@@ -2888,7 +2927,7 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable* variable,
Handle<Name> name = variable->name();
Node* node = BuildNamedLoad(global, name, feedback,
TypeFeedbackId::None(), contextual_mode);
- PrepareFrameState(node, bailout_id, OutputFrameStateCombine::Push());
+ states.AddToNode(node, bailout_id, combine);
return node;
}
case Variable::PARAMETER:
@@ -2951,9 +2990,9 @@ Node* AstGraphBuilder::BuildVariableLoad(Variable* variable,
}
-Node* AstGraphBuilder::BuildVariableDelete(
- Variable* variable, BailoutId bailout_id,
- OutputFrameStateCombine state_combine) {
+Node* AstGraphBuilder::BuildVariableDelete(Variable* variable,
+ BailoutId bailout_id,
+ OutputFrameStateCombine combine) {
switch (variable->location()) {
case Variable::UNALLOCATED: {
// Global var, const, or let variable.
@@ -2961,7 +3000,7 @@ Node* AstGraphBuilder::BuildVariableDelete(
Node* name = jsgraph()->Constant(variable->name());
const Operator* op = javascript()->DeleteProperty(language_mode());
Node* result = NewNode(op, global, name);
- PrepareFrameState(result, bailout_id, state_combine);
+ PrepareFrameState(result, bailout_id, combine);
return result;
}
case Variable::PARAMETER:
@@ -2975,7 +3014,7 @@ Node* AstGraphBuilder::BuildVariableDelete(
const Operator* op =
javascript()->CallRuntime(Runtime::kDeleteLookupSlot, 2);
Node* result = NewNode(op, current_context(), name);
- PrepareFrameState(result, bailout_id, state_combine);
+ PrepareFrameState(result, bailout_id, combine);
return result;
}
}
@@ -2985,8 +3024,8 @@ Node* AstGraphBuilder::BuildVariableDelete(
Node* AstGraphBuilder::BuildVariableAssignment(
- Variable* variable, Node* value, Token::Value op, BailoutId bailout_id,
- OutputFrameStateCombine combine) {
+ FrameStateBeforeAndAfter& states, Variable* variable, Node* value,
+ Token::Value op, BailoutId bailout_id, OutputFrameStateCombine combine) {
Node* the_hole = jsgraph()->TheHoleConstant();
VariableMode mode = variable->mode();
switch (variable->location()) {
@@ -2996,7 +3035,7 @@ Node* AstGraphBuilder::BuildVariableAssignment(
Handle<Name> name = variable->name();
Node* store =
BuildNamedStore(global, name, value, TypeFeedbackId::None());
- PrepareFrameState(store, bailout_id, combine);
+ states.AddToNode(store, bailout_id, combine);
return store;
}
case Variable::PARAMETER:
@@ -3215,9 +3254,10 @@ Node* AstGraphBuilder::BuildSetHomeObject(Node* value, Node* home_object,
Expression* expr) {
if (!FunctionLiteral::NeedsHomeObject(expr)) return value;
Handle<Name> name = isolate()->factory()->home_object_symbol();
+ FrameStateBeforeAndAfter states(this, BailoutId::None());
Node* store =
BuildNamedStore(value, name, home_object, TypeFeedbackId::None());
- PrepareFrameState(store, BailoutId::None());
+ states.AddToNode(store, BailoutId::None(), OutputFrameStateCombine::Ignore());
return store;
}
« no previous file with comments | « src/compiler/ast-graph-builder.h ('k') | src/compiler/js-generic-lowering.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698