Revision: 3594
Author: [email protected]
Date: Wed Jan 13 03:29:08 2010
Log: Cleanup the handling of control flow in the toplevel code generator.
Do abstract the setting and restoring of 'argument' state into a
function that takes arguments.
Do not set the argument state in the code generator unless it
represents arguments to a recursive call to Visit.
Review URL: http://codereview.chromium.org/550010
http://code.google.com/p/v8/source/detail?r=3594
Modified:
/branches/bleeding_edge/src/arm/fast-codegen-arm.cc
/branches/bleeding_edge/src/fast-codegen.cc
/branches/bleeding_edge/src/fast-codegen.h
/branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc
/branches/bleeding_edge/src/x64/fast-codegen-x64.cc
=======================================
--- /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Tue Jan 12 07:16:23
2010
+++ /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Wed Jan 13 03:29:08
2010
@@ -1265,20 +1265,19 @@
Comment cmnt(masm_, "[ UnaryOperation (NOT)");
ASSERT_EQ(Expression::kTest, expr->expression()->context());
- Label push_true;
- Label push_false;
- Label done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
+ Label push_true, push_false, done;
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
+ case Expression::kEffect:
+ VisitForControl(expr->expression(), &done, &done);
+ __ bind(&done);
+ break;
+
case Expression::kValue:
- true_label_ = &push_false;
- false_label_ = &push_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &push_false, &push_true);
__ bind(&push_true);
__ LoadRoot(ip, Heap::kTrueValueRootIndex);
__ push(ip);
@@ -1289,41 +1288,26 @@
__ bind(&done);
break;
- case Expression::kEffect:
- true_label_ = &done;
- false_label_ = &done;
- Visit(expr->expression());
- __ bind(&done);
- break;
-
case Expression::kTest:
- true_label_ = saved_false;
- false_label_ = saved_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), false_label_, true_label_);
break;
case Expression::kValueTest:
- true_label_ = saved_false;
- false_label_ = &push_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), false_label_, &push_true);
__ bind(&push_true);
__ LoadRoot(ip, Heap::kTrueValueRootIndex);
__ push(ip);
- __ jmp(saved_true);
+ __ jmp(true_label_);
break;
case Expression::kTestValue:
- true_label_ = &push_false;
- false_label_ = saved_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &push_false, true_label_);
__ bind(&push_false);
__ LoadRoot(ip, Heap::kFalseValueRootIndex);
__ push(ip);
- __ jmp(saved_false);
+ __ jmp(false_label_);
break;
}
- true_label_ = saved_true;
- false_label_ = saved_false;
break;
}
@@ -1549,47 +1533,41 @@
Visit(expr->left());
Visit(expr->right());
- // Convert current context to test context: Pre-test code.
- Label push_true;
- Label push_false;
- Label done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
+ // Always perform the comparison for its control flow. Pack the result
+ // into the expression's context after the comparison is performed.
+ Label push_true, push_false, done;
+ // Initially assume we are in a test context.
+ Label* if_true = true_label_;
+ Label* if_false = false_label_;
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
-
- case Expression::kValue:
- true_label_ = &push_true;
- false_label_ = &push_false;
- break;
-
case Expression::kEffect:
- true_label_ = &done;
- false_label_ = &done;
+ if_true = &done;
+ if_false = &done;
break;
-
+ case Expression::kValue:
+ if_true = &push_true;
+ if_false = &push_false;
+ break;
case Expression::kTest:
break;
-
case Expression::kValueTest:
- true_label_ = &push_true;
+ if_true = &push_true;
break;
-
case Expression::kTestValue:
- false_label_ = &push_false;
+ if_false = &push_false;
break;
}
- // Convert current context to test context: End pre-test code.
switch (expr->op()) {
case Token::IN: {
__ InvokeBuiltin(Builtins::IN, CALL_JS);
__ LoadRoot(ip, Heap::kTrueValueRootIndex);
__ cmp(r0, ip);
- __ b(eq, true_label_);
- __ jmp(false_label_);
+ __ b(eq, if_true);
+ __ jmp(if_false);
break;
}
@@ -1597,8 +1575,8 @@
InstanceofStub stub;
__ CallStub(&stub);
__ tst(r0, r0);
- __ b(eq, true_label_); // The stub returns 0 for true.
- __ jmp(false_label_);
+ __ b(eq, if_true); // The stub returns 0 for true.
+ __ jmp(if_false);
break;
}
@@ -1649,24 +1627,29 @@
__ tst(r2, Operand(kSmiTagMask));
__ b(ne, &slow_case);
__ cmp(r1, r0);
- __ b(cc, true_label_);
- __ jmp(false_label_);
+ __ b(cc, if_true);
+ __ jmp(if_false);
__ bind(&slow_case);
CompareStub stub(cc, strict);
__ CallStub(&stub);
__ tst(r0, r0);
- __ b(cc, true_label_);
- __ jmp(false_label_);
+ __ b(cc, if_true);
+ __ jmp(if_false);
}
}
- // Convert current context to test context: Post-test code.
+ // Convert the result of the comparison into one expected for this
+ // expression's context.
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
+ case Expression::kEffect:
+ __ bind(&done);
+ break;
+
case Expression::kValue:
__ bind(&push_true);
__ LoadRoot(ip, Heap::kTrueValueRootIndex);
@@ -1678,10 +1661,6 @@
__ bind(&done);
break;
- case Expression::kEffect:
- __ bind(&done);
- break;
-
case Expression::kTest:
break;
@@ -1689,19 +1668,16 @@
__ bind(&push_true);
__ LoadRoot(ip, Heap::kTrueValueRootIndex);
__ push(ip);
- __ jmp(saved_true);
+ __ jmp(true_label_);
break;
case Expression::kTestValue:
__ bind(&push_false);
__ LoadRoot(ip, Heap::kFalseValueRootIndex);
__ push(ip);
- __ jmp(saved_false);
+ __ jmp(false_label_);
break;
}
- true_label_ = saved_true;
- false_label_ = saved_false;
- // Convert current context to test context: End post-test code.
}
=======================================
--- /branches/bleeding_edge/src/fast-codegen.cc Tue Jan 12 09:22:57 2010
+++ /branches/bleeding_edge/src/fast-codegen.cc Wed Jan 13 03:29:08 2010
@@ -226,36 +226,33 @@
#endif
Label eval_right, done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
-
- // Set up the appropriate context for the left subexpression based on the
- // operation and our own context.
+
+ // Set up the appropriate context for the left subexpression based
+ // on the operation and our own context. Initially assume we can
+ // inherit both true and false labels from our context.
+ Label* if_true = true_label_;
+ Label* if_false = false_label_;
if (expr->op() == Token::OR) {
- // If there is no usable true label in the OR expression's context, use
- // the end of this expression, otherwise inherit the same true label.
+ // If we are not in some kind of a test context, we did not inherit a
+ // true label from our context. Use the end of the expression.
if (expr->context() == Expression::kEffect ||
expr->context() == Expression::kValue) {
- true_label_ = &done;
- }
- // The false label is the label of the second subexpression.
- false_label_ = &eval_right;
+ if_true = &done;
+ }
+ // The false label is the label of the right subexpression.
+ if_false = &eval_right;
} else {
ASSERT_EQ(Token::AND, expr->op());
- // The true label is the label of the second subexpression.
- true_label_ = &eval_right;
- // If there is no usable false label in the AND expression's context,
- // use the end of the expression, otherwise inherit the same false
- // label.
+ // The true label is the label of the right subexpression.
+ if_true = &eval_right;
+ // If we are not in some kind of a test context, we did not inherit a
+ // false label from our context. Use the end of the expression.
if (expr->context() == Expression::kEffect ||
expr->context() == Expression::kValue) {
- false_label_ = &done;
+ if_false = &done;
}
}
-
- Visit(expr->left());
- true_label_ = saved_true;
- false_label_ = saved_false;
+ VisitForControl(expr->left(), if_true, if_false);
__ bind(&eval_right);
Visit(expr->right());
@@ -289,19 +286,10 @@
void FastCodeGenerator::VisitIfStatement(IfStatement* stmt) {
Comment cmnt(masm_, "[ IfStatement");
SetStatementPosition(stmt);
- // Expressions cannot recursively enter statements, there are no labels
in
- // the state.
- ASSERT_EQ(NULL, true_label_);
- ASSERT_EQ(NULL, false_label_);
Label then_part, else_part, done;
// Do not worry about optimizing for empty then or else bodies.
- true_label_ = &then_part;
- false_label_ = &else_part;
- ASSERT(stmt->condition()->context() == Expression::kTest);
- Visit(stmt->condition());
- true_label_ = NULL;
- false_label_ = NULL;
+ VisitForControl(stmt->condition(), &then_part, &else_part);
__ bind(&then_part);
Visit(stmt->then_statement());
@@ -423,17 +411,8 @@
__ StackLimitCheck(&stack_limit_hit);
__ bind(&stack_check_success);
- // We are not in an expression context because we have been compiling
- // statements. Set up a test expression context for the condition.
__ bind(loop_statement.continue_target());
- ASSERT_EQ(NULL, true_label_);
- ASSERT_EQ(NULL, false_label_);
- true_label_ = &body;
- false_label_ = loop_statement.break_target();
- ASSERT(stmt->cond()->context() == Expression::kTest);
- Visit(stmt->cond());
- true_label_ = NULL;
- false_label_ = NULL;
+ VisitForControl(stmt->cond(), &body, loop_statement.break_target());
__ bind(&stack_limit_hit);
StackCheckStub stack_stub;
@@ -465,16 +444,7 @@
__ StackLimitCheck(&stack_limit_hit);
__ bind(&stack_check_success);
- // We are not in an expression context because we have been compiling
- // statements. Set up a test expression context for the condition.
- ASSERT_EQ(NULL, true_label_);
- ASSERT_EQ(NULL, false_label_);
- true_label_ = &body;
- false_label_ = loop_statement.break_target();
- ASSERT(stmt->cond()->context() == Expression::kTest);
- Visit(stmt->cond());
- true_label_ = NULL;
- false_label_ = NULL;
+ VisitForControl(stmt->cond(), &body, loop_statement.break_target());
__ bind(&stack_limit_hit);
StackCheckStub stack_stub;
@@ -628,14 +598,7 @@
Label true_case, false_case, done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
-
- true_label_ = &true_case;
- false_label_ = &false_case;
- Visit(expr->condition());
- true_label_ = saved_true;
- false_label_ = saved_false;
+ VisitForControl(expr->condition(), &true_case, &false_case);
__ bind(&true_case);
Visit(expr->then_expression());
=======================================
--- /branches/bleeding_edge/src/fast-codegen.h Tue Jan 12 00:48:26 2010
+++ /branches/bleeding_edge/src/fast-codegen.h Wed Jan 13 03:29:08 2010
@@ -241,6 +241,19 @@
// control flow to a pair of labels.
void TestAndBranch(Register source, Label* true_label, Label*
false_label);
+ void VisitForControl(Expression* expr, Label* if_true, Label* if_false) {
+ ASSERT(expr->context() == Expression::kTest ||
+ expr->context() == Expression::kValueTest ||
+ expr->context() == Expression::kTestValue);
+ Label* saved_true = true_label_;
+ Label* saved_false = false_label_;
+ true_label_ = if_true;
+ false_label_ = if_false;
+ Visit(expr);
+ true_label_ = saved_true;
+ false_label_ = saved_false;
+ }
+
void VisitDeclarations(ZoneList<Declaration*>* declarations);
void DeclareGlobals(Handle<FixedArray> pairs);
=======================================
--- /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc Tue Jan 12
01:58:50 2010
+++ /branches/bleeding_edge/src/ia32/fast-codegen-ia32.cc Wed Jan 13
03:29:08 2010
@@ -1240,20 +1240,19 @@
Comment cmnt(masm_, "[ UnaryOperation (NOT)");
ASSERT_EQ(Expression::kTest, expr->expression()->context());
- Label push_true;
- Label push_false;
- Label done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
+ Label push_true, push_false, done;
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
+ case Expression::kEffect:
+ VisitForControl(expr->expression(), &done, &done);
+ __ bind(&done);
+ break;
+
case Expression::kValue:
- true_label_ = &push_false;
- false_label_ = &push_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &push_false, &push_true);
__ bind(&push_true);
__ push(Immediate(Factory::true_value()));
__ jmp(&done);
@@ -1262,39 +1261,24 @@
__ bind(&done);
break;
- case Expression::kEffect:
- true_label_ = &done;
- false_label_ = &done;
- Visit(expr->expression());
- __ bind(&done);
- break;
-
case Expression::kTest:
- true_label_ = saved_false;
- false_label_ = saved_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), false_label_, true_label_);
break;
case Expression::kValueTest:
- true_label_ = saved_false;
- false_label_ = &push_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), false_label_, &push_true);
__ bind(&push_true);
__ push(Immediate(Factory::true_value()));
- __ jmp(saved_true);
+ __ jmp(true_label_);
break;
case Expression::kTestValue:
- true_label_ = &push_false;
- false_label_ = saved_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &push_false, true_label_);
__ bind(&push_false);
__ push(Immediate(Factory::false_value()));
- __ jmp(saved_false);
+ __ jmp(false_label_);
break;
}
- true_label_ = saved_true;
- false_label_ = saved_false;
break;
}
@@ -1523,46 +1507,40 @@
Visit(expr->left());
Visit(expr->right());
- // Convert current context to test context: Pre-test code.
- Label push_true;
- Label push_false;
- Label done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
+ // Always perform the comparison for its control flow. Pack the result
+ // into the expression's context after the comparison is performed.
+ Label push_true, push_false, done;
+ // Initially assume we are in a test context.
+ Label* if_true = true_label_;
+ Label* if_false = false_label_;
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
-
- case Expression::kValue:
- true_label_ = &push_true;
- false_label_ = &push_false;
- break;
-
case Expression::kEffect:
- true_label_ = &done;
- false_label_ = &done;
+ if_true = &done;
+ if_false = &done;
break;
-
+ case Expression::kValue:
+ if_true = &push_true;
+ if_false = &push_false;
+ break;
case Expression::kTest:
break;
-
case Expression::kValueTest:
- true_label_ = &push_true;
+ if_true = &push_true;
break;
-
case Expression::kTestValue:
- false_label_ = &push_false;
+ if_false = &push_false;
break;
}
- // Convert current context to test context: End pre-test code.
switch (expr->op()) {
case Token::IN: {
__ InvokeBuiltin(Builtins::IN, CALL_FUNCTION);
__ cmp(eax, Factory::true_value());
- __ j(equal, true_label_);
- __ jmp(false_label_);
+ __ j(equal, if_true);
+ __ jmp(if_false);
break;
}
@@ -1570,8 +1548,8 @@
InstanceofStub stub;
__ CallStub(&stub);
__ test(eax, Operand(eax));
- __ j(zero, true_label_); // The stub returns 0 for true.
- __ jmp(false_label_);
+ __ j(zero, if_true); // The stub returns 0 for true.
+ __ jmp(if_false);
break;
}
@@ -1623,24 +1601,29 @@
__ test(ecx, Immediate(kSmiTagMask));
__ j(not_zero, &slow_case, not_taken);
__ cmp(edx, Operand(eax));
- __ j(cc, true_label_);
- __ jmp(false_label_);
+ __ j(cc, if_true);
+ __ jmp(if_false);
__ bind(&slow_case);
CompareStub stub(cc, strict);
__ CallStub(&stub);
__ test(eax, Operand(eax));
- __ j(cc, true_label_);
- __ jmp(false_label_);
+ __ j(cc, if_true);
+ __ jmp(if_false);
}
}
- // Convert current context to test context: Post-test code.
+ // Convert the result of the comparison into one expected for this
+ // expression's context.
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
+ case Expression::kEffect:
+ __ bind(&done);
+ break;
+
case Expression::kValue:
__ bind(&push_true);
__ push(Immediate(Factory::true_value()));
@@ -1650,28 +1633,21 @@
__ bind(&done);
break;
- case Expression::kEffect:
- __ bind(&done);
- break;
-
case Expression::kTest:
break;
case Expression::kValueTest:
__ bind(&push_true);
__ push(Immediate(Factory::true_value()));
- __ jmp(saved_true);
+ __ jmp(true_label_);
break;
case Expression::kTestValue:
__ bind(&push_false);
__ push(Immediate(Factory::false_value()));
- __ jmp(saved_false);
+ __ jmp(false_label_);
break;
}
- true_label_ = saved_true;
- false_label_ = saved_false;
- // Convert current context to test context: End post-test code.
}
=======================================
--- /branches/bleeding_edge/src/x64/fast-codegen-x64.cc Tue Jan 12 00:48:26
2010
+++ /branches/bleeding_edge/src/x64/fast-codegen-x64.cc Wed Jan 13 03:29:08
2010
@@ -1259,20 +1259,14 @@
Comment cmnt(masm_, "[ UnaryOperation (NOT)");
ASSERT_EQ(Expression::kTest, expr->expression()->context());
- Label push_true;
- Label push_false;
- Label done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
+ Label push_true, push_false, done;
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
case Expression::kValue:
- true_label_ = &push_false;
- false_label_ = &push_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &push_false, &push_true);
__ bind(&push_true);
__ PushRoot(Heap::kTrueValueRootIndex);
__ jmp(&done);
@@ -1282,38 +1276,28 @@
break;
case Expression::kEffect:
- true_label_ = &done;
- false_label_ = &done;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &done, &done);
__ bind(&done);
break;
case Expression::kTest:
- true_label_ = saved_false;
- false_label_ = saved_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), false_label_, true_label_);
break;
case Expression::kValueTest:
- true_label_ = saved_false;
- false_label_ = &push_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), false_label_, &push_true);
__ bind(&push_true);
__ PushRoot(Heap::kTrueValueRootIndex);
- __ jmp(saved_true);
+ __ jmp(true_label_);
break;
case Expression::kTestValue:
- true_label_ = &push_false;
- false_label_ = saved_true;
- Visit(expr->expression());
+ VisitForControl(expr->expression(), &push_false, true_label_);
__ bind(&push_false);
__ PushRoot(Heap::kFalseValueRootIndex);
- __ jmp(saved_false);
+ __ jmp(false_label_);
break;
}
- true_label_ = saved_true;
- false_label_ = saved_false;
break;
}
@@ -1541,46 +1525,40 @@
Visit(expr->left());
Visit(expr->right());
- // Convert current context to test context: Pre-test code.
- Label push_true;
- Label push_false;
- Label done;
- Label* saved_true = true_label_;
- Label* saved_false = false_label_;
+ // Always perform the comparison for its control flow. Pack the result
+ // into the expression's context after the comparison is performed.
+ Label push_true, push_false, done;
+ // Initially assume we are in a test context.
+ Label* if_true = true_label_;
+ Label* if_false = false_label_;
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
-
case Expression::kValue:
- true_label_ = &push_true;
- false_label_ = &push_false;
+ if_true = &push_true;
+ if_false = &push_false;
break;
-
case Expression::kEffect:
- true_label_ = &done;
- false_label_ = &done;
+ if_true = &done;
+ if_false = &done;
break;
-
case Expression::kTest:
break;
-
case Expression::kValueTest:
- true_label_ = &push_true;
+ if_true = &push_true;
break;
-
case Expression::kTestValue:
- false_label_ = &push_false;
+ if_false = &push_false;
break;
}
- // Convert current context to test context: End pre-test code.
switch (expr->op()) {
case Token::IN: {
__ InvokeBuiltin(Builtins::IN, CALL_FUNCTION);
__ CompareRoot(rax, Heap::kTrueValueRootIndex);
- __ j(equal, true_label_);
- __ jmp(false_label_);
+ __ j(equal, if_true);
+ __ jmp(if_false);
break;
}
@@ -1588,8 +1566,8 @@
InstanceofStub stub;
__ CallStub(&stub);
__ testq(rax, rax);
- __ j(zero, true_label_); // The stub returns 0 for true.
- __ jmp(false_label_);
+ __ j(zero, if_true); // The stub returns 0 for true.
+ __ jmp(if_false);
break;
}
@@ -1638,24 +1616,29 @@
Label slow_case;
__ JumpIfNotBothSmi(rax, rdx, &slow_case);
__ SmiCompare(rdx, rax);
- __ j(cc, true_label_);
- __ jmp(false_label_);
+ __ j(cc, if_true);
+ __ jmp(if_false);
__ bind(&slow_case);
CompareStub stub(cc, strict);
__ CallStub(&stub);
__ testq(rax, rax);
- __ j(cc, true_label_);
- __ jmp(false_label_);
+ __ j(cc, if_true);
+ __ jmp(if_false);
}
}
- // Convert current context to test context: Post-test code.
+ // Convert the result of the comparison into one expected for this
+ // expression's context.
switch (expr->context()) {
case Expression::kUninitialized:
UNREACHABLE();
break;
+ case Expression::kEffect:
+ __ bind(&done);
+ break;
+
case Expression::kValue:
__ bind(&push_true);
__ PushRoot(Heap::kTrueValueRootIndex);
@@ -1665,28 +1648,21 @@
__ bind(&done);
break;
- case Expression::kEffect:
- __ bind(&done);
- break;
-
case Expression::kTest:
break;
case Expression::kValueTest:
__ bind(&push_true);
__ PushRoot(Heap::kTrueValueRootIndex);
- __ jmp(saved_true);
+ __ jmp(true_label_);
break;
case Expression::kTestValue:
__ bind(&push_false);
__ PushRoot(Heap::kFalseValueRootIndex);
- __ jmp(saved_false);
+ __ jmp(false_label_);
break;
}
- true_label_ = saved_true;
- false_label_ = saved_false;
- // Convert current context to test context: End post-test code.
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
