Revision: 23096
Author: ja...@chromium.org
Date: Wed Aug 13 09:23:35 2014 UTC
Log: Refactor building of lazy bailouts in AstGraphBuilder.
BUG=
R=mstarzin...@chromium.org
Review URL: https://codereview.chromium.org/464033002
http://code.google.com/p/v8/source/detail?r=23096
Modified:
/branches/bleeding_edge/src/compiler/ast-graph-builder.cc
/branches/bleeding_edge/src/compiler/ast-graph-builder.h
/branches/bleeding_edge/src/compiler/graph-builder.cc
=======================================
--- /branches/bleeding_edge/src/compiler/ast-graph-builder.cc Mon Aug 11
12:26:17 2014 UTC
+++ /branches/bleeding_edge/src/compiler/ast-graph-builder.cc Wed Aug 13
09:23:35 2014 UTC
@@ -238,12 +238,8 @@
AstGraphBuilder::AstContext::AstContext(AstGraphBuilder* own,
- Expression::Context kind,
- BailoutId bailout_id)
- : bailout_id_(bailout_id),
- kind_(kind),
- owner_(own),
- outer_(own->ast_context()) {
+ Expression::Context kind)
+ : kind_(kind), owner_(own), outer_(own->ast_context()) {
owner()->set_ast_context(this); // Push.
#ifdef DEBUG
original_height_ = environment()->stack_height();
@@ -269,28 +265,6 @@
AstGraphBuilder::AstTestContext::~AstTestContext() {
DCHECK(environment()->stack_height() == original_height_ + 1);
}
-
-
-void AstGraphBuilder::AstEffectContext::ProduceValueWithLazyBailout(
- Node* value) {
- ProduceValue(value);
- owner()->BuildLazyBailout(value, bailout_id_);
-}
-
-
-void AstGraphBuilder::AstValueContext::ProduceValueWithLazyBailout(
- Node* value) {
- ProduceValue(value);
- owner()->BuildLazyBailout(value, bailout_id_);
-}
-
-
-void AstGraphBuilder::AstTestContext::ProduceValueWithLazyBailout(Node*
value) {
- environment()->Push(value);
- owner()->BuildLazyBailout(value, bailout_id_);
- environment()->Pop();
- ProduceValue(value);
-}
void AstGraphBuilder::AstEffectContext::ProduceValue(Node* value) {
@@ -359,7 +333,7 @@
void AstGraphBuilder::VisitForValue(Expression* expr) {
- AstValueContext for_value(this, expr->id());
+ AstValueContext for_value(this);
if (!HasStackOverflow()) {
expr->Accept(this);
}
@@ -367,7 +341,7 @@
void AstGraphBuilder::VisitForEffect(Expression* expr) {
- AstEffectContext for_effect(this, expr->id());
+ AstEffectContext for_effect(this);
if (!HasStackOverflow()) {
expr->Accept(this);
}
@@ -375,7 +349,7 @@
void AstGraphBuilder::VisitForTest(Expression* expr) {
- AstTestContext for_condition(this, expr->id());
+ AstTestContext for_condition(this);
if (!HasStackOverflow()) {
expr->Accept(this);
}
@@ -713,7 +687,7 @@
Node* exit_cond =
NewNode(javascript()->LessThan(), index, cache_length);
// TODO(jarin): provide real bailout id.
- BuildLazyBailout(exit_cond, BailoutId::None());
+ PrepareFrameState(exit_cond, BailoutId::None());
for_loop.BreakUnless(exit_cond);
// TODO(dcarney): this runtime call should be a handful of
// simplified instructions that
@@ -753,7 +727,7 @@
Node* res = ProcessArguments(
javascript()->Call(3, NO_CALL_FUNCTION_FLAGS), 3);
// TODO(jarin): provide real bailout id.
- BuildLazyBailout(res, BailoutId::None());
+ PrepareFrameState(res, BailoutId::None());
Node* property_missing = NewNode(javascript()->StrictEqual(),
res,
jsgraph()->ZeroConstant());
{
@@ -765,7 +739,7 @@
NewNode(javascript()->Add(), index,
jsgraph()->OneConstant());
environment()->Poke(0, index_inc);
// TODO(jarin): provide real bailout id.
- BuildLazyBailout(index_inc, BailoutId::None());
+ PrepareFrameState(index_inc, BailoutId::None());
for_loop.Continue();
is_property_missing.Else();
is_property_missing.End();
@@ -784,7 +758,7 @@
NewNode(javascript()->Add(), index, jsgraph()->OneConstant());
environment()->Poke(0, index_inc);
// TODO(jarin): provide real bailout id.
- BuildLazyBailout(index_inc, BailoutId::None());
+ PrepareFrameState(index_inc, BailoutId::None());
for_loop.EndBody();
for_loop.EndLoop();
environment()->Drop(5);
@@ -932,7 +906,7 @@
PrintableUnique<Name> name = MakeUnique(key->AsPropertyName());
Node* store =
NewNode(javascript()->StoreNamed(name), literal, value);
- BuildLazyBailout(store, key->id());
+ PrepareFrameState(store, key->id());
} else {
VisitForEffect(property->value());
}
@@ -1024,7 +998,7 @@
Node* value = environment()->Pop();
Node* index = jsgraph()->Constant(i);
Node* store = NewNode(javascript()->StoreProperty(), literal, index,
value);
- BuildLazyBailout(store, expr->GetIdForElement(i));
+ PrepareFrameState(store, expr->GetIdForElement(i));
}
environment()->Pop(); // Array literal index.
@@ -1056,7 +1030,7 @@
MakeUnique(property->key()->AsLiteral()->AsPropertyName());
Node* store = NewNode(javascript()->StoreNamed(name), object, value);
// TODO(jarin) Fill in the correct bailout id.
- BuildLazyBailout(store, BailoutId::None());
+ PrepareFrameState(store, BailoutId::None());
break;
}
case KEYED_PROPERTY: {
@@ -1068,7 +1042,7 @@
value = environment()->Pop();
Node* store = NewNode(javascript()->StoreProperty(), object, key,
value);
// TODO(jarin) Fill in the correct bailout id.
- BuildLazyBailout(store, BailoutId::None());
+ PrepareFrameState(store, BailoutId::None());
break;
}
}
@@ -1112,14 +1086,14 @@
PrintableUnique<Name> name =
MakeUnique(property->key()->AsLiteral()->AsPropertyName());
old_value = NewNode(javascript()->LoadNamed(name), object);
- BuildLazyBailoutWithPushedNode(old_value, property->LoadId());
+ PrepareFrameState(old_value, property->LoadId(), PUSH_OUTPUT);
break;
}
case KEYED_PROPERTY: {
Node* key = environment()->Top();
Node* object = environment()->Peek(1);
old_value = NewNode(javascript()->LoadProperty(), object, key);
- BuildLazyBailoutWithPushedNode(old_value, property->LoadId());
+ PrepareFrameState(old_value, property->LoadId(), PUSH_OUTPUT);
break;
}
}
@@ -1129,7 +1103,7 @@
Node* left = environment()->Pop();
Node* value = BuildBinaryOp(left, right, expr->binary_op());
environment()->Push(value);
- BuildLazyBailout(value, expr->binary_operation()->id());
+ PrepareFrameState(value, expr->binary_operation()->id());
} else {
VisitForValue(expr->value());
}
@@ -1148,14 +1122,14 @@
PrintableUnique<Name> name =
MakeUnique(property->key()->AsLiteral()->AsPropertyName());
Node* store = NewNode(javascript()->StoreNamed(name), object, value);
- BuildLazyBailout(store, expr->AssignmentId());
+ PrepareFrameState(store, expr->AssignmentId());
break;
}
case KEYED_PROPERTY: {
Node* key = environment()->Pop();
Node* object = environment()->Pop();
Node* store = NewNode(javascript()->StoreProperty(), object, key,
value);
- BuildLazyBailout(store, expr->AssignmentId());
+ PrepareFrameState(store, expr->AssignmentId());
break;
}
}
@@ -1198,7 +1172,8 @@
Node* object = environment()->Pop();
value = NewNode(javascript()->LoadProperty(), object, key);
}
- ast_context()->ProduceValueWithLazyBailout(value);
+ PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
+ ast_context()->ProduceValue(value);
}
@@ -1242,7 +1217,7 @@
Node* key = environment()->Pop();
callee_value = NewNode(javascript()->LoadProperty(), object, key);
}
- BuildLazyBailoutWithPushedNode(callee_value, property->LoadId());
+ PrepareFrameState(callee_value, property->LoadId(), PUSH_OUTPUT);
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,
@@ -1297,7 +1272,8 @@
// Create node to perform the function call.
Operator* call = javascript()->Call(args->length() + 2, flags);
Node* value = ProcessArguments(call, args->length() + 2);
- ast_context()->ProduceValueWithLazyBailout(value);
+ PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
+ ast_context()->ProduceValue(value);
}
@@ -1311,7 +1287,8 @@
// Create node to perform the construct call.
Operator* call = javascript()->CallNew(args->length() + 1);
Node* value = ProcessArguments(call, args->length() + 1);
- ast_context()->ProduceValueWithLazyBailout(value);
+ PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
+ ast_context()->ProduceValue(value);
}
@@ -1327,7 +1304,7 @@
environment()->Push(callee_value);
// TODO(jarin): Find/create a bailout id to deoptimize to (crankshaft
// refuses to optimize functions with jsruntime calls).
- BuildLazyBailout(callee_value, BailoutId::None());
+ PrepareFrameState(callee_value, BailoutId::None());
environment()->Push(receiver_value);
// Evaluate all arguments to the JS runtime call.
@@ -1337,7 +1314,8 @@
// Create node to perform the JS runtime call.
Operator* call = javascript()->Call(args->length() + 2, flags);
Node* value = ProcessArguments(call, args->length() + 2);
- ast_context()->ProduceValueWithLazyBailout(value);
+ PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
+ ast_context()->ProduceValue(value);
}
@@ -1359,7 +1337,8 @@
Runtime::FunctionId functionId = function->function_id;
Operator* call = javascript()->Runtime(functionId, args->length());
Node* value = ProcessArguments(call, args->length());
- ast_context()->ProduceValueWithLazyBailout(value);
+ PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
+ ast_context()->ProduceValue(value);
}
@@ -1406,7 +1385,7 @@
PrintableUnique<Name> name =
MakeUnique(property->key()->AsLiteral()->AsPropertyName());
old_value = NewNode(javascript()->LoadNamed(name), object);
- BuildLazyBailoutWithPushedNode(old_value, property->LoadId());
+ PrepareFrameState(old_value, property->LoadId(), PUSH_OUTPUT);
stack_depth = 1;
break;
}
@@ -1416,7 +1395,7 @@
Node* key = environment()->Top();
Node* object = environment()->Peek(1);
old_value = NewNode(javascript()->LoadProperty(), object, key);
- BuildLazyBailoutWithPushedNode(old_value, property->LoadId());
+ PrepareFrameState(old_value, property->LoadId(), PUSH_OUTPUT);
stack_depth = 2;
break;
}
@@ -1433,7 +1412,7 @@
BuildBinaryOp(old_value, jsgraph()->OneConstant(),
expr->binary_op());
// TODO(jarin) Insert proper bailout id here (will need to change
// full code generator).
- BuildLazyBailout(value, BailoutId::None());
+ PrepareFrameState(value, BailoutId::None());
// Store the value.
switch (assign_type) {
@@ -1448,14 +1427,14 @@
PrintableUnique<Name> name =
MakeUnique(property->key()->AsLiteral()->AsPropertyName());
Node* store = NewNode(javascript()->StoreNamed(name), object, value);
- BuildLazyBailout(store, expr->AssignmentId());
+ PrepareFrameState(store, expr->AssignmentId());
break;
}
case KEYED_PROPERTY: {
Node* key = environment()->Pop();
Node* object = environment()->Pop();
Node* store = NewNode(javascript()->StoreProperty(), object, key,
value);
- BuildLazyBailout(store, expr->AssignmentId());
+ PrepareFrameState(store, expr->AssignmentId());
break;
}
}
@@ -1480,7 +1459,8 @@
Node* right = environment()->Pop();
Node* left = environment()->Pop();
Node* value = BuildBinaryOp(left, right, expr->op());
- ast_context()->ProduceValueWithLazyBailout(value);
+ PrepareFrameState(value, expr->id(),
ast_context()->GetStateCombine());
+ ast_context()->ProduceValue(value);
}
}
}
@@ -1530,7 +1510,7 @@
Node* value = NewNode(op, left, right);
ast_context()->ProduceValue(value);
- BuildLazyBailout(value, expr->id());
+ PrepareFrameState(value, expr->id());
}
@@ -1760,7 +1740,7 @@
PrintableUnique<Name> name = MakeUnique(variable->name());
Operator* op = javascript()->LoadNamed(name, contextual_mode);
Node* node = NewNode(op, global);
- BuildLazyBailoutWithPushedNode(node, bailout_id);
+ PrepareFrameState(node, bailout_id, PUSH_OUTPUT);
return node;
}
case Variable::PARAMETER:
@@ -1861,7 +1841,7 @@
PrintableUnique<Name> name = MakeUnique(variable->name());
Operator* op = javascript()->StoreNamed(name);
Node* store = NewNode(op, global, value);
- BuildLazyBailout(store, bailout_id);
+ PrepareFrameState(store, bailout_id);
return store;
}
case Variable::PARAMETER:
@@ -2013,11 +1993,16 @@
}
-void AstGraphBuilder::BuildLazyBailout(Node* node, BailoutId ast_id) {
+void AstGraphBuilder::PrepareFrameState(Node* node, BailoutId ast_id,
+ OutputFrameStateCombine combine) {
if (OperatorProperties::CanLazilyDeoptimize(node->op())) {
// The deopting node should have an outgoing control dependency.
DCHECK(environment()->GetControlDependency() == node);
+ if (combine == PUSH_OUTPUT) {
+ environment()->Push(node);
+ }
+
StructuredGraphBuilder::Environment* continuation_env = environment();
// Create environment for the deoptimization block, and build the
block.
StructuredGraphBuilder::Environment* deopt_env =
@@ -2039,17 +2024,14 @@
// Continue with the original environment.
set_environment(continuation_env);
+ if (combine == PUSH_OUTPUT) {
+ environment()->Pop();
+ }
+
NewNode(common()->Continuation());
}
}
-
-void AstGraphBuilder::BuildLazyBailoutWithPushedNode(Node* node,
- BailoutId ast_id) {
- environment()->Push(node);
- BuildLazyBailout(node, ast_id);
- environment()->Pop();
-}
}
}
} // namespace v8::internal::compiler
=======================================
--- /branches/bleeding_edge/src/compiler/ast-graph-builder.h Mon Aug 11
12:26:17 2014 UTC
+++ /branches/bleeding_edge/src/compiler/ast-graph-builder.h Wed Aug 13
09:23:35 2014 UTC
@@ -171,8 +171,18 @@
// Dispatched from VisitForInStatement.
void VisitForInAssignment(Expression* expr, Node* value);
- void BuildLazyBailout(Node* node, BailoutId ast_id);
- void BuildLazyBailoutWithPushedNode(Node* node, BailoutId ast_id);
+ // Flag that describes how to combine the current environment with
+ // the output of a node to obtain a framestate for lazy bailout.
+ enum OutputFrameStateCombine {
+ PUSH_OUTPUT, // Push the output on the expression stack.
+ IGNORE_OUTPUT // Use the frame state as-is.
+ };
+
+ // Builds deoptimization for a given node.
+ void PrepareFrameState(Node* node, BailoutId ast_id,
+ OutputFrameStateCombine combine = IGNORE_OUTPUT);
+
+ OutputFrameStateCombine StateCombineFromAstContext();
DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
DISALLOW_COPY_AND_ASSIGN(AstGraphBuilder);
@@ -281,11 +291,16 @@
bool IsEffect() const { return kind_ == Expression::kEffect; }
bool IsValue() const { return kind_ == Expression::kValue; }
bool IsTest() const { return kind_ == Expression::kTest; }
+
+ // Determines how to combine the frame state with the value
+ // that is about to be plugged into this AstContext.
+ AstGraphBuilder::OutputFrameStateCombine GetStateCombine() {
+ return IsEffect() ? IGNORE_OUTPUT : PUSH_OUTPUT;
+ }
// Plug a node into this expression context. Call this function in tail
// position in the Visit functions for expressions.
virtual void ProduceValue(Node* value) = 0;
- virtual void ProduceValueWithLazyBailout(Node* value) = 0;
// Unplugs a node from this expression context. Call this to retrieve
the
// result of another Visit function that already plugged the context.
@@ -295,8 +310,7 @@
void ReplaceValue() { ProduceValue(ConsumeValue()); }
protected:
- AstContext(AstGraphBuilder* owner, Expression::Context kind,
- BailoutId bailout_id);
+ AstContext(AstGraphBuilder* owner, Expression::Context kind);
virtual ~AstContext();
AstGraphBuilder* owner() const { return owner_; }
@@ -308,8 +322,6 @@
int original_height_;
#endif
- BailoutId bailout_id_;
-
private:
Expression::Context kind_;
AstGraphBuilder* owner_;
@@ -320,11 +332,10 @@
// Context to evaluate expression for its side effects only.
class AstGraphBuilder::AstEffectContext V8_FINAL : public AstContext {
public:
- explicit AstEffectContext(AstGraphBuilder* owner, BailoutId bailout_id)
- : AstContext(owner, Expression::kEffect, bailout_id) {}
+ explicit AstEffectContext(AstGraphBuilder* owner)
+ : AstContext(owner, Expression::kEffect) {}
virtual ~AstEffectContext();
virtual void ProduceValue(Node* value) V8_OVERRIDE;
- virtual void ProduceValueWithLazyBailout(Node* value) V8_OVERRIDE;
virtual Node* ConsumeValue() V8_OVERRIDE;
};
@@ -332,11 +343,10 @@
// Context to evaluate expression for its value (and side effects).
class AstGraphBuilder::AstValueContext V8_FINAL : public AstContext {
public:
- explicit AstValueContext(AstGraphBuilder* owner, BailoutId bailout_id)
- : AstContext(owner, Expression::kValue, bailout_id) {}
+ explicit AstValueContext(AstGraphBuilder* owner)
+ : AstContext(owner, Expression::kValue) {}
virtual ~AstValueContext();
virtual void ProduceValue(Node* value) V8_OVERRIDE;
- virtual void ProduceValueWithLazyBailout(Node* value) V8_OVERRIDE;
virtual Node* ConsumeValue() V8_OVERRIDE;
};
@@ -344,11 +354,10 @@
// Context to evaluate expression for a condition value (and side effects).
class AstGraphBuilder::AstTestContext V8_FINAL : public AstContext {
public:
- explicit AstTestContext(AstGraphBuilder* owner, BailoutId bailout_id)
- : AstContext(owner, Expression::kTest, bailout_id) {}
+ explicit AstTestContext(AstGraphBuilder* owner)
+ : AstContext(owner, Expression::kTest) {}
virtual ~AstTestContext();
virtual void ProduceValue(Node* value) V8_OVERRIDE;
- virtual void ProduceValueWithLazyBailout(Node* value) V8_OVERRIDE;
virtual Node* ConsumeValue() V8_OVERRIDE;
};
=======================================
--- /branches/bleeding_edge/src/compiler/graph-builder.cc Fri Aug 8
13:51:30 2014 UTC
+++ /branches/bleeding_edge/src/compiler/graph-builder.cc Wed Aug 13
09:23:35 2014 UTC
@@ -30,6 +30,8 @@
Node* StructuredGraphBuilder::MakeNode(Operator* op, int value_input_count,
Node** value_inputs) {
+ DCHECK(op->InputCount() == value_input_count);
+
bool has_context = OperatorProperties::HasContextInput(op);
bool has_control = OperatorProperties::GetControlInputCount(op) == 1;
bool has_effect = OperatorProperties::GetEffectInputCount(op) == 1;
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
