Reduced the code ping-pong between the full code generator and contexts a bit.

* Centralized AND/OR handling, keeping related code together.

* Removed HandleExpression/HandleInNonTestContext and introduced VisitInSameContext instead, making it more obvious what's actually going on.

* Consistently use a new context when visiting the left sub-expression of an AND/OR. Note that the context stacks in the full code generator and crankshaft are still a bit out of sync for the right sub-expression.
Review URL: http://codereview.chromium.org/6976028

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8124 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
svenpanne@chromium.org 2011-05-31 14:37:34 +00:00
parent d71adb6d22
commit 6453056bb6
6 changed files with 132 additions and 203 deletions

View File

@ -4029,7 +4029,7 @@ void FullCodeGenerator::VisitForTypeofValue(Expression* expr) {
context()->Plug(r0);
} else {
// This expression cannot throw a reference error at the top level.
context()->HandleExpression(expr);
VisitInCurrentContext(expr);
}
}

View File

@ -701,143 +701,116 @@ void FullCodeGenerator::EmitInlineRuntimeCall(CallRuntime* node) {
void FullCodeGenerator::VisitBinaryOperation(BinaryOperation* expr) {
Comment cmnt(masm_, "[ BinaryOperation");
Token::Value op = expr->op();
Expression* left = expr->left();
Expression* right = expr->right();
OverwriteMode mode = NO_OVERWRITE;
if (left->ResultOverwriteAllowed()) {
mode = OVERWRITE_LEFT;
} else if (right->ResultOverwriteAllowed()) {
mode = OVERWRITE_RIGHT;
}
switch (op) {
switch (expr->op()) {
case Token::COMMA:
VisitForEffect(left);
if (context()->IsTest()) ForwardBailoutToChild(expr);
context()->HandleExpression(right);
break;
return VisitComma(expr);
case Token::OR:
case Token::AND:
EmitLogicalOperation(expr);
break;
case Token::ADD:
case Token::SUB:
case Token::DIV:
case Token::MOD:
case Token::MUL:
case Token::BIT_OR:
case Token::BIT_AND:
case Token::BIT_XOR:
case Token::SHL:
case Token::SHR:
case Token::SAR: {
// Load both operands.
VisitForStackValue(left);
VisitForAccumulatorValue(right);
SetSourcePosition(expr->position());
if (ShouldInlineSmiCase(op)) {
EmitInlineSmiBinaryOp(expr, op, mode, left, right);
} else {
EmitBinaryOp(expr, op, mode);
}
break;
}
return VisitLogicalExpression(expr);
default:
UNREACHABLE();
return VisitArithmeticExpression(expr);
}
}
void FullCodeGenerator::EmitLogicalOperation(BinaryOperation* expr) {
Label eval_right, done;
context()->EmitLogicalLeft(expr, &eval_right, &done);
PrepareForBailoutForId(expr->RightId(), NO_REGISTERS);
__ bind(&eval_right);
void FullCodeGenerator::VisitComma(BinaryOperation* expr) {
Comment cmnt(masm_, "[ Comma");
VisitForEffect(expr->left());
if (context()->IsTest()) ForwardBailoutToChild(expr);
context()->HandleExpression(expr->right());
VisitInCurrentContext(expr->right());
}
void FullCodeGenerator::VisitLogicalExpression(BinaryOperation* expr) {
bool is_logical_and = expr->op() == Token::AND;
Comment cmnt(masm_, is_logical_and ? "[ Logical AND" : "[ Logical OR");
Expression* left = expr->left();
Expression* right = expr->right();
int right_id = expr->RightId();
Label done;
if (context()->IsTest()) {
Label eval_right;
const TestContext* test = TestContext::cast(context());
if (is_logical_and) {
VisitForControl(left, &eval_right, test->false_label(), &eval_right);
} else {
VisitForControl(left, test->true_label(), &eval_right, &eval_right);
}
PrepareForBailoutForId(right_id, NO_REGISTERS);
__ bind(&eval_right);
ForwardBailoutToChild(expr);
} else if (context()->IsAccumulatorValue()) {
VisitForAccumulatorValue(left);
// We want the value in the accumulator for the test, and on the stack in
// case we need it.
__ push(result_register());
Label discard, restore;
PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
if (is_logical_and) {
DoTest(&discard, &restore, &restore);
} else {
DoTest(&restore, &discard, &restore);
}
__ bind(&restore);
__ pop(result_register());
__ jmp(&done);
__ bind(&discard);
__ Drop(1);
PrepareForBailoutForId(right_id, NO_REGISTERS);
} else if (context()->IsStackValue()) {
VisitForAccumulatorValue(left);
// We want the value in the accumulator for the test, and on the stack in
// case we need it.
__ push(result_register());
Label discard;
PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
if (is_logical_and) {
DoTest(&discard, &done, &discard);
} else {
DoTest(&done, &discard, &discard);
}
__ bind(&discard);
__ Drop(1);
PrepareForBailoutForId(right_id, NO_REGISTERS);
} else {
ASSERT(context()->IsEffect());
Label eval_right;
if (is_logical_and) {
VisitForControl(left, &eval_right, &done, &eval_right);
} else {
VisitForControl(left, &done, &eval_right, &eval_right);
}
PrepareForBailoutForId(right_id, NO_REGISTERS);
__ bind(&eval_right);
}
VisitInCurrentContext(right);
__ bind(&done);
}
void FullCodeGenerator::EffectContext::EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const {
if (expr->op() == Token::OR) {
codegen()->VisitForControl(expr->left(), done, eval_right, eval_right);
void FullCodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) {
Token::Value op = expr->op();
Comment cmnt(masm_, "[ ArithmeticExpression");
Expression* left = expr->left();
Expression* right = expr->right();
OverwriteMode mode =
left->ResultOverwriteAllowed()
? OVERWRITE_LEFT
: (right->ResultOverwriteAllowed() ? OVERWRITE_RIGHT : NO_OVERWRITE);
VisitForStackValue(left);
VisitForAccumulatorValue(right);
SetSourcePosition(expr->position());
if (ShouldInlineSmiCase(op)) {
EmitInlineSmiBinaryOp(expr, op, mode, left, right);
} else {
ASSERT(expr->op() == Token::AND);
codegen()->VisitForControl(expr->left(), eval_right, done, eval_right);
}
}
void FullCodeGenerator::AccumulatorValueContext::EmitLogicalLeft(
BinaryOperation* expr,
Label* eval_right,
Label* done) const {
HandleExpression(expr->left());
// We want the value in the accumulator for the test, and on the stack in case
// we need it.
__ push(result_register());
Label discard, restore;
if (expr->op() == Token::OR) {
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
codegen()->DoTest(&restore, &discard, &restore);
} else {
ASSERT(expr->op() == Token::AND);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
codegen()->DoTest(&discard, &restore, &restore);
}
__ bind(&restore);
__ pop(result_register());
__ jmp(done);
__ bind(&discard);
__ Drop(1);
}
void FullCodeGenerator::StackValueContext::EmitLogicalLeft(
BinaryOperation* expr,
Label* eval_right,
Label* done) const {
codegen()->VisitForAccumulatorValue(expr->left());
// We want the value in the accumulator for the test, and on the stack in case
// we need it.
__ push(result_register());
Label discard;
if (expr->op() == Token::OR) {
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
codegen()->DoTest(done, &discard, &discard);
} else {
ASSERT(expr->op() == Token::AND);
codegen()->PrepareForBailoutBeforeSplit(TOS_REG, false, NULL, NULL);
codegen()->DoTest(&discard, done, &discard);
}
__ bind(&discard);
__ Drop(1);
}
void FullCodeGenerator::TestContext::EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const {
if (expr->op() == Token::OR) {
codegen()->VisitForControl(expr->left(),
true_label_, eval_right, eval_right);
} else {
ASSERT(expr->op() == Token::AND);
codegen()->VisitForControl(expr->left(),
eval_right, false_label_, eval_right);
EmitBinaryOp(expr, op, mode);
}
}
@ -850,46 +823,23 @@ void FullCodeGenerator::ForwardBailoutToChild(Expression* expr) {
}
void FullCodeGenerator::EffectContext::HandleExpression(
Expression* expr) const {
codegen()->HandleInNonTestContext(expr, NO_REGISTERS);
}
void FullCodeGenerator::AccumulatorValueContext::HandleExpression(
Expression* expr) const {
codegen()->HandleInNonTestContext(expr, TOS_REG);
}
void FullCodeGenerator::StackValueContext::HandleExpression(
Expression* expr) const {
codegen()->HandleInNonTestContext(expr, NO_REGISTERS);
}
void FullCodeGenerator::TestContext::HandleExpression(Expression* expr) const {
codegen()->VisitInTestContext(expr);
}
void FullCodeGenerator::HandleInNonTestContext(Expression* expr, State state) {
ASSERT(forward_bailout_pending_ == NULL);
AstVisitor::Visit(expr);
PrepareForBailout(expr, state);
// Forwarding bailouts to children is a one shot operation. It
// should have been processed at this point.
ASSERT(forward_bailout_pending_ == NULL);
}
void FullCodeGenerator::VisitInTestContext(Expression* expr) {
ForwardBailoutStack stack(expr, forward_bailout_pending_);
ForwardBailoutStack* saved = forward_bailout_stack_;
forward_bailout_pending_ = NULL;
forward_bailout_stack_ = &stack;
AstVisitor::Visit(expr);
forward_bailout_stack_ = saved;
void FullCodeGenerator::VisitInCurrentContext(Expression* expr) {
if (context()->IsTest()) {
ForwardBailoutStack stack(expr, forward_bailout_pending_);
ForwardBailoutStack* saved = forward_bailout_stack_;
forward_bailout_pending_ = NULL;
forward_bailout_stack_ = &stack;
Visit(expr);
forward_bailout_stack_ = saved;
} else {
ASSERT(forward_bailout_pending_ == NULL);
Visit(expr);
State state = context()->IsAccumulatorValue() ? TOS_REG : NO_REGISTERS;
PrepareForBailout(expr, state);
// Forwarding bailouts to children is a one shot operation. It should have
// been processed at this point.
ASSERT(forward_bailout_pending_ == NULL);
}
}
@ -1287,7 +1237,7 @@ void FullCodeGenerator::VisitConditional(Conditional* expr) {
for_test->false_label(),
NULL);
} else {
context()->HandleExpression(expr->then_expression());
VisitInCurrentContext(expr->then_expression());
__ jmp(&done);
}
@ -1296,7 +1246,7 @@ void FullCodeGenerator::VisitConditional(Conditional* expr) {
if (context()->IsTest()) ForwardBailoutToChild(expr);
SetExpressionPosition(expr->else_expression(),
expr->else_expression_position());
context()->HandleExpression(expr->else_expression());
VisitInCurrentContext(expr->else_expression());
// If control flow falls through Visit, merge it with true case here.
if (!context()->IsTest()) {
__ bind(&done);

View File

@ -328,17 +328,17 @@ class FullCodeGenerator: public AstVisitor {
void VisitForEffect(Expression* expr) {
EffectContext context(this);
HandleInNonTestContext(expr, NO_REGISTERS);
VisitInCurrentContext(expr);
}
void VisitForAccumulatorValue(Expression* expr) {
AccumulatorValueContext context(this);
HandleInNonTestContext(expr, TOS_REG);
VisitInCurrentContext(expr);
}
void VisitForStackValue(Expression* expr) {
StackValueContext context(this);
HandleInNonTestContext(expr, NO_REGISTERS);
VisitInCurrentContext(expr);
}
void VisitForControl(Expression* expr,
@ -346,15 +346,9 @@ class FullCodeGenerator: public AstVisitor {
Label* if_false,
Label* fall_through) {
TestContext context(this, if_true, if_false, fall_through);
VisitInTestContext(expr);
// Forwarding bailouts to children is a one shot operation. It
// should have been processed at this point.
ASSERT(forward_bailout_pending_ == NULL);
VisitInCurrentContext(expr);
}
void HandleInNonTestContext(Expression* expr, State state);
void VisitInTestContext(Expression* expr);
void VisitDeclarations(ZoneList<Declaration*>* declarations);
void DeclareGlobals(Handle<FixedArray> pairs);
@ -549,8 +543,10 @@ class FullCodeGenerator: public AstVisitor {
void EmitUnaryOperation(UnaryOperation* expr, const char* comment);
// Handles the shortcutted logical binary operations in VisitBinaryOperation.
void EmitLogicalOperation(BinaryOperation* expr);
void VisitComma(BinaryOperation* expr);
void VisitLogicalExpression(BinaryOperation* expr);
void VisitArithmeticExpression(BinaryOperation* expr);
void VisitInCurrentContext(Expression* expr);
void VisitForTypeofValue(Expression* expr);
@ -598,11 +594,6 @@ class FullCodeGenerator: public AstVisitor {
// context.
virtual void DropAndPlug(int count, Register reg) const = 0;
// For shortcutting operations || and &&.
virtual void EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const = 0;
// Set up branch labels for a test expression. The three Label** parameters
// are output parameters.
virtual void PrepareTest(Label* materialize_true,
@ -611,12 +602,14 @@ class FullCodeGenerator: public AstVisitor {
Label** if_false,
Label** fall_through) const = 0;
virtual void HandleExpression(Expression* expr) const = 0;
// Returns true if we are evaluating only for side effects (ie if the result
// will be discarded).
virtual bool IsEffect() const { return false; }
// Returns true if we are evaluating for the value (in accu/on stack).
virtual bool IsAccumulatorValue() const { return false; }
virtual bool IsStackValue() const { return false; }
// Returns true if we are branching on the value rather than materializing
// it. Only used for asserts.
virtual bool IsTest() const { return false; }
@ -644,15 +637,12 @@ class FullCodeGenerator: public AstVisitor {
virtual void Plug(Heap::RootListIndex) const;
virtual void PlugTOS() const;
virtual void DropAndPlug(int count, Register reg) const;
virtual void EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const;
virtual void PrepareTest(Label* materialize_true,
Label* materialize_false,
Label** if_true,
Label** if_false,
Label** fall_through) const;
virtual void HandleExpression(Expression* expr) const;
virtual bool IsAccumulatorValue() const { return true; }
};
class StackValueContext : public ExpressionContext {
@ -668,15 +658,12 @@ class FullCodeGenerator: public AstVisitor {
virtual void Plug(Heap::RootListIndex) const;
virtual void PlugTOS() const;
virtual void DropAndPlug(int count, Register reg) const;
virtual void EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const;
virtual void PrepareTest(Label* materialize_true,
Label* materialize_false,
Label** if_true,
Label** if_false,
Label** fall_through) const;
virtual void HandleExpression(Expression* expr) const;
virtual bool IsStackValue() const { return true; }
};
class TestContext : public ExpressionContext {
@ -707,15 +694,11 @@ class FullCodeGenerator: public AstVisitor {
virtual void Plug(Heap::RootListIndex) const;
virtual void PlugTOS() const;
virtual void DropAndPlug(int count, Register reg) const;
virtual void EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const;
virtual void PrepareTest(Label* materialize_true,
Label* materialize_false,
Label** if_true,
Label** if_false,
Label** fall_through) const;
virtual void HandleExpression(Expression* expr) const;
virtual bool IsTest() const { return true; }
private:
@ -737,15 +720,11 @@ class FullCodeGenerator: public AstVisitor {
virtual void Plug(Heap::RootListIndex) const;
virtual void PlugTOS() const;
virtual void DropAndPlug(int count, Register reg) const;
virtual void EmitLogicalLeft(BinaryOperation* expr,
Label* eval_right,
Label* done) const;
virtual void PrepareTest(Label* materialize_true,
Label* materialize_false,
Label** if_true,
Label** if_false,
Label** fall_through) const;
virtual void HandleExpression(Expression* expr) const;
virtual bool IsEffect() const { return true; }
};

View File

@ -4007,7 +4007,7 @@ void FullCodeGenerator::VisitForTypeofValue(Expression* expr) {
context()->Plug(eax);
} else {
// This expression cannot throw a reference error at the top level.
context()->HandleExpression(expr);
VisitInCurrentContext(expr);
}
}

View File

@ -4039,7 +4039,7 @@ void FullCodeGenerator::VisitForTypeofValue(Expression* expr) {
context()->Plug(v0);
} else {
// This expression cannot throw a reference error at the top level.
context()->HandleExpression(expr);
VisitInCurrentContext(expr);
}
}

View File

@ -3983,7 +3983,7 @@ void FullCodeGenerator::VisitForTypeofValue(Expression* expr) {
context()->Plug(rax);
} else {
// This expression cannot throw a reference error at the top level.
context()->HandleExpression(expr);
VisitInCurrentContext(expr);
}
}