2009/11/1 <[email protected]>: > > Reviewers: Kevin Millikin, fschneider, > > Message: > Tested on all platorms. > Strangely, on ARM, the TestValue context seems not to be propagated to the > variable loads of y and z in > z = (x ? y : z) && (!x ? y : z) > > The ToBoolean call occurs after the control flow merge in the first ternary > operator, not before it. This seems wrong (more efficient, but wrong).
Can you make a test that shows it is wrong on ARM? > > Description: > Add conditional expressions (ternary choice operator) to fast compiler. > > Please review this at http://codereview.chromium.org/340058 > > SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ > > Affected files: > M src/compiler.cc > M src/fast-codegen.cc > > > Index: src/compiler.cc > =================================================================== > --- src/compiler.cc (revision 3191) > +++ src/compiler.cc (working copy) > @@ -625,7 +625,11 @@ > > > void CodeGenSelector::VisitConditional(Conditional* expr) { > - BAILOUT("Conditional"); > + ProcessExpression(expr->condition(), Expression::kTest); > + CHECK_BAILOUT; > + ProcessExpression(expr->then_expression(), context_); > + CHECK_BAILOUT; > + ProcessExpression(expr->else_expression(), context_); > } > > > Index: src/fast-codegen.cc > =================================================================== > --- src/fast-codegen.cc (revision 3191) > +++ src/fast-codegen.cc (working copy) > @@ -380,7 +380,36 @@ > > > void FastCodeGenerator::VisitConditional(Conditional* expr) { > - UNREACHABLE(); > + ASSERT_EQ(Expression::kTest, expr->condition()->context()); > + ASSERT_EQ(expr->context(), expr->then_expression()->context()); > + ASSERT_EQ(expr->context(), expr->else_expression()->context()); > + > + > + 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; > + > + __ bind(&true_case); > + Visit(expr->then_expression()); > + // If control flow falls through Visit, jump to done. > + if (expr->context() == Expression::kEffect || > + expr->context() == Expression::kValue) { > + __ jmp(&done); > + } > + > + __ bind(&false_case); > + Visit(expr->else_expression()); > + // If control flow falls through Visit, merge it with true case here. > + if (expr->context() == Expression::kEffect || > + expr->context() == Expression::kValue) { > + __ bind(&done); > + } > } > > > > > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
