From 65e099e27705e22c08f7b03a0b22a64c8a6a784f Mon Sep 17 00:00:00 2001 From: "ricow@chromium.org" Date: Fri, 26 Mar 2010 07:55:38 +0000 Subject: [PATCH] Land http://codereview.chromium.org/1311003/diff/8001/9001 to allows us to push to trunk. Corrected the ASSERT from the review. Review URL: http://codereview.chromium.org/1404001 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4290 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/codegen-arm.cc | 27 ++++++++++++++++----------- src/arm/codegen-arm.h | 3 +++ src/ia32/codegen-ia32.cc | 17 ++++++++++++----- src/ia32/codegen-ia32.h | 3 +++ src/x64/codegen-x64.cc | 20 +++++++++++--------- src/x64/codegen-x64.h | 3 +++ 6 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index 5e0067716..0ca4d3560 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -3996,14 +3996,7 @@ void CodeGenerator::VisitCountOperation(CountOperation* node) { } -void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { -#ifdef DEBUG - int original_height = frame_->height(); -#endif - VirtualFrame::SpilledScope spilled_scope; - Comment cmnt(masm_, "[ BinaryOperation"); - Token::Value op = node->op(); - +void CodeGenerator::GenerateLogicalBooleanOperation(BinaryOperation* node) { // According to ECMA-262 section 11.11, page 58, the binary logical // operators must yield the result of one of the two expressions // before any ToBoolean() conversions. This means that the value @@ -4015,8 +4008,7 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { // after evaluating the left hand side (due to the shortcut // semantics), but the compiler must (statically) know if the result // of compiling the binary operation is materialized or not. - - if (op == Token::AND) { + if (node->op() == Token::AND) { JumpTarget is_true; LoadConditionAndSpill(node->left(), &is_true, @@ -4062,7 +4054,8 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { ASSERT(!has_valid_frame() && !has_cc() && !is_true.is_linked()); } - } else if (op == Token::OR) { + } else { + ASSERT(node->op() == Token::OR); JumpTarget is_false; LoadConditionAndSpill(node->left(), true_target(), @@ -4107,7 +4100,19 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { // Nothing to do. ASSERT(!has_valid_frame() && !has_cc() && !is_false.is_linked()); } + } +} + + +void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { +#ifdef DEBUG + int original_height = frame_->height(); +#endif + VirtualFrame::SpilledScope spilled_scope; + Comment cmnt(masm_, "[ BinaryOperation"); + if (node->op() == Token::AND || node->op() == Token::OR) { + GenerateLogicalBooleanOperation(node); } else { // Optimize for the case where (at least) one of the expressions // is a literal small integer. diff --git a/src/arm/codegen-arm.h b/src/arm/codegen-arm.h index 4bea3415a..0d1a38559 100644 --- a/src/arm/codegen-arm.h +++ b/src/arm/codegen-arm.h @@ -306,6 +306,9 @@ class CodeGenerator: public AstVisitor { void ToBoolean(JumpTarget* true_target, JumpTarget* false_target); + // Generate code that computes a shortcutting logical operation. + void GenerateLogicalBooleanOperation(BinaryOperation* node); + void GenericBinaryOperation(Token::Value op, OverwriteMode overwrite_mode, int known_rhs = kUnknownIntValue); diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 20b64636d..200e3ef4e 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -7424,10 +7424,8 @@ void CodeGenerator::Int32BinaryOperation(BinaryOperation* node) { } } -void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { - Comment cmnt(masm_, "[ BinaryOperation"); - Token::Value op = node->op(); +void CodeGenerator::GenerateLogicalBooleanOperation(BinaryOperation* node) { // According to ECMA-262 section 11.11, page 58, the binary logical // operators must yield the result of one of the two expressions // before any ToBoolean() conversions. This means that the value @@ -7437,7 +7435,7 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { // control flow), we force the right hand side to do the same. This // is necessary because we assume that if we get control flow on the // last path out of an expression we got it on all paths. - if (op == Token::AND) { + if (node->op() == Token::AND) { ASSERT(!in_safe_int32_mode()); JumpTarget is_true; ControlDestination dest(&is_true, destination()->false_target(), true); @@ -7501,7 +7499,8 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { exit.Bind(); } - } else if (op == Token::OR) { + } else { + ASSERT(node->op() == Token::OR); ASSERT(!in_safe_int32_mode()); JumpTarget is_false; ControlDestination dest(destination()->true_target(), &is_false, false); @@ -7563,7 +7562,15 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { // Exit (always with a materialized value). exit.Bind(); } + } +} + + +void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { + Comment cmnt(masm_, "[ BinaryOperation"); + if (node->op() == Token::AND || node->op() == Token::OR) { + GenerateLogicalBooleanOperation(node); } else if (in_safe_int32_mode()) { Visit(node->left()); Visit(node->right()); diff --git a/src/ia32/codegen-ia32.h b/src/ia32/codegen-ia32.h index ca4a44b85..9fcc466e3 100644 --- a/src/ia32/codegen-ia32.h +++ b/src/ia32/codegen-ia32.h @@ -489,6 +489,9 @@ class CodeGenerator: public AstVisitor { // control destination. void ToBoolean(ControlDestination* destination); + // Generate code that computes a shortcutting logical operation. + void GenerateLogicalBooleanOperation(BinaryOperation* node); + void GenericBinaryOperation( Token::Value op, StaticType* type, diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 108384cf1..dd8015f14 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -3281,13 +3281,7 @@ void CodeGenerator::VisitCountOperation(CountOperation* node) { } -void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { - // TODO(X64): This code was copied verbatim from codegen-ia32. - // Either find a reason to change it or move it to a shared location. - - Comment cmnt(masm_, "[ BinaryOperation"); - Token::Value op = node->op(); - +void CodeGenerator::GenerateLogicalBooleanOperation(BinaryOperation* node) { // According to ECMA-262 section 11.11, page 58, the binary logical // operators must yield the result of one of the two expressions // before any ToBoolean() conversions. This means that the value @@ -3297,7 +3291,7 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { // control flow), we force the right hand side to do the same. This // is necessary because we assume that if we get control flow on the // last path out of an expression we got it on all paths. - if (op == Token::AND) { + if (node->op() == Token::AND) { JumpTarget is_true; ControlDestination dest(&is_true, destination()->false_target(), true); LoadCondition(node->left(), &dest, false); @@ -3360,7 +3354,8 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { exit.Bind(); } - } else if (op == Token::OR) { + } else { + ASSERT(node->op() == Token::OR); JumpTarget is_false; ControlDestination dest(destination()->true_target(), &is_false, false); LoadCondition(node->left(), &dest, false); @@ -3421,7 +3416,14 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { // Exit (always with a materialized value). exit.Bind(); } + } +} + +void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) { + Comment cmnt(masm_, "[ BinaryOperation"); + if (node->op() == Token::AND || node->op() == Token::OR) { + GenerateLogicalBooleanOperation(node); } else { // NOTE: The code below assumes that the slow cases (calls to runtime) // never return a constant/immutable object. diff --git a/src/x64/codegen-x64.h b/src/x64/codegen-x64.h index 3f339181b..8eea47959 100644 --- a/src/x64/codegen-x64.h +++ b/src/x64/codegen-x64.h @@ -454,6 +454,9 @@ class CodeGenerator: public AstVisitor { // control destination. void ToBoolean(ControlDestination* destination); + // Generate code that computes a shortcutting logical operation. + void GenerateLogicalBooleanOperation(BinaryOperation* node); + void GenericBinaryOperation( Token::Value op, StaticType* type, -- 2.34.1