Simplify IA32 code generator API.

Cut down on the number of arguments passed to the various binary operation
code generator functions by passing along the expression itself, rather than
a subset of its fields.

Review URL: http://codereview.chromium.org/1592001

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4319 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/ast.cc

@@ -852,13 +852,11 @@ UnaryOperation::UnaryOperation(UnaryOperation* other, Expression* expression)
     : Expression(other), op_(other->op_), expression_(expression) {}
 
 
-BinaryOperation::BinaryOperation(BinaryOperation* other,
+BinaryOperation::BinaryOperation(Expression* other,
+                                 Token::Value op,
                                  Expression* left,
                                  Expression* right)
-    : Expression(other),
-      op_(other->op_),
-      left_(left),
-      right_(right) {}
+    : Expression(other), op_(op), left_(left), right_(right) {}
 
 
 CountOperation::CountOperation(CountOperation* other, Expression* expression)
@@ -1110,6 +1108,7 @@ void CopyAstVisitor::VisitCountOperation(CountOperation* expr) {
 
 void CopyAstVisitor::VisitBinaryOperation(BinaryOperation* expr) {
   expr_ = new BinaryOperation(expr,
+                              expr->op(),
                               DeepCopyExpr(expr->left()),
                               DeepCopyExpr(expr->right()));
 }

src/ast.h

@@ -1369,7 +1369,13 @@ class BinaryOperation: public Expression {
     ASSERT(Token::IsBinaryOp(op));
   }
 
-  BinaryOperation(BinaryOperation* other, Expression* left, Expression* right);
+  // Construct a binary operation with a given operator and left and right
+  // subexpressions.  The rest of the expression state is copied from
+  // another expression.
+  BinaryOperation(Expression* other,
+                  Token::Value op,
+                  Expression* left,
+                  Expression* right);
 
   virtual void Accept(AstVisitor* v);
 

src/ia32/codegen-ia32.cc

@@ -1221,11 +1221,10 @@ static TypeInfo CalculateTypeInfo(TypeInfo operands_type,
 }
 
 
-void CodeGenerator::GenericBinaryOperation(Token::Value op,
-                                           StaticType* type,
-                                           OverwriteMode overwrite_mode,
-                                           bool no_negative_zero) {
+void CodeGenerator::GenericBinaryOperation(BinaryOperation* expr,
+                                           OverwriteMode overwrite_mode) {
   Comment cmnt(masm_, "[ BinaryOperation");
+  Token::Value op = expr->op();
   Comment cmnt_token(masm_, Token::String(op));
 
   if (op == Token::COMMA) {
@@ -1298,13 +1297,11 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
                              operands_type);
     answer = stub.GenerateCall(masm_, frame_, &left, &right);
   } else if (right_is_smi_constant) {
-    answer = ConstantSmiBinaryOperation(op, &left, right.handle(),
-                                        type, false, overwrite_mode,
-                                        no_negative_zero);
+    answer = ConstantSmiBinaryOperation(expr, &left, right.handle(),
+                                        false, overwrite_mode);
   } else if (left_is_smi_constant) {
-    answer = ConstantSmiBinaryOperation(op, &right, left.handle(),
-                                        type, true, overwrite_mode,
-                                        no_negative_zero);
+    answer = ConstantSmiBinaryOperation(expr, &right, left.handle(),
+                                        true, overwrite_mode);
   } else {
     // Set the flags based on the operation, type and loop nesting level.
     // Bit operations always assume they likely operate on Smis. Still only
@@ -1314,9 +1311,8 @@ void CodeGenerator::GenericBinaryOperation(Token::Value op,
     if (loop_nesting() > 0 &&
         (Token::IsBitOp(op) ||
          operands_type.IsInteger32() ||
-         type->IsLikelySmi())) {
-      answer = LikelySmiBinaryOperation(op, &left, &right,
-                                        overwrite_mode, no_negative_zero);
+         expr->type()->IsLikelySmi())) {
+      answer = LikelySmiBinaryOperation(expr, &left, &right, overwrite_mode);
     } else {
       GenericBinaryOpStub stub(op,
                                overwrite_mode,
@@ -1420,11 +1416,11 @@ static void CheckTwoForSminess(MacroAssembler* masm,
 
 // Implements a binary operation using a deferred code object and some
 // inline code to operate on smis quickly.
-Result CodeGenerator::LikelySmiBinaryOperation(Token::Value op,
+Result CodeGenerator::LikelySmiBinaryOperation(BinaryOperation* expr,
                                                Result* left,
                                                Result* right,
-                                               OverwriteMode overwrite_mode,
-                                               bool no_negative_zero) {
+                                               OverwriteMode overwrite_mode) {
+  Token::Value op = expr->op();
   Result answer;
   // Special handling of div and mod because they use fixed registers.
   if (op == Token::DIV || op == Token::MOD) {
@@ -1530,7 +1526,7 @@ Result CodeGenerator::LikelySmiBinaryOperation(Token::Value op,
       // virtual frame is unchanged in this block, so local control flow
       // can use a Label rather than a JumpTarget.  If the context of this
       // expression will treat -0 like 0, do not do this test.
-      if (!no_negative_zero) {
+      if (!expr->no_negative_zero()) {
         Label non_zero_result;
         __ test(left->reg(), Operand(left->reg()));
         __ j(not_zero, &non_zero_result);
@@ -1559,7 +1555,7 @@ Result CodeGenerator::LikelySmiBinaryOperation(Token::Value op,
       // the dividend is negative, return a floating point negative
       // zero.  The frame is unchanged in this block, so local control
       // flow can use a Label rather than a JumpTarget.
-      if (!no_negative_zero) {
+      if (!expr->no_negative_zero()) {
         Label non_zero_result;
         __ test(edx, Operand(edx));
         __ j(not_zero, &non_zero_result, taken);
@@ -1743,7 +1739,7 @@ Result CodeGenerator::LikelySmiBinaryOperation(Token::Value op,
       // argument is negative, go to slow case.  The frame is unchanged
       // in this block, so local control flow can use a Label rather
       // than a JumpTarget.
-      if (!no_negative_zero) {
+      if (!expr->no_negative_zero()) {
         Label non_zero_result;
         __ test(answer.reg(), Operand(answer.reg()));
         __ j(not_zero, &non_zero_result, taken);
@@ -1986,13 +1982,12 @@ void DeferredInlineSmiSub::Generate() {
 }
 
 
-Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
-                                                 Result* operand,
-                                                 Handle<Object> value,
-                                                 StaticType* type,
-                                                 bool reversed,
-                                                 OverwriteMode overwrite_mode,
-                                                 bool no_negative_zero) {
+Result CodeGenerator::ConstantSmiBinaryOperation(
+    BinaryOperation* expr,
+    Result* operand,
+    Handle<Object> value,
+    bool reversed,
+    OverwriteMode overwrite_mode) {
   // NOTE: This is an attempt to inline (a bit) more of the code for
   // some possible smi operations (like + and -) when (at least) one
   // of the operands is a constant smi.
@@ -2002,11 +1997,11 @@ Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
   if (IsUnsafeSmi(value)) {
     Result unsafe_operand(value);
     if (reversed) {
-      return LikelySmiBinaryOperation(op, &unsafe_operand, operand,
-                                      overwrite_mode, no_negative_zero);
+      return LikelySmiBinaryOperation(expr, &unsafe_operand, operand,
+                                      overwrite_mode);
     } else {
-      return LikelySmiBinaryOperation(op, operand, &unsafe_operand,
-                                      overwrite_mode, no_negative_zero);
+      return LikelySmiBinaryOperation(expr, operand, &unsafe_operand,
+                                      overwrite_mode);
     }
   }
 
@@ -2014,6 +2009,7 @@ Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
   Smi* smi_value = Smi::cast(*value);
   int int_value = smi_value->value();
 
+  Token::Value op = expr->op();
   Result answer;
   switch (op) {
     case Token::ADD: {
@@ -2089,8 +2085,8 @@ Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
     case Token::SAR:
       if (reversed) {
         Result constant_operand(value);
-        answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
-                                          overwrite_mode, no_negative_zero);
+        answer = LikelySmiBinaryOperation(expr, &constant_operand, operand,
+                                          overwrite_mode);
       } else {
         // Only the least significant 5 bits of the shift value are used.
         // In the slow case, this masking is done inside the runtime call.
@@ -2126,8 +2122,8 @@ Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
     case Token::SHR:
       if (reversed) {
         Result constant_operand(value);
-        answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
-                                          overwrite_mode, no_negative_zero);
+        answer = LikelySmiBinaryOperation(expr, &constant_operand, operand,
+                                          overwrite_mode);
       } else {
         // Only the least significant 5 bits of the shift value are used.
         // In the slow case, this masking is done inside the runtime call.
@@ -2327,11 +2323,11 @@ Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
         // default case here.
         Result constant_operand(value);
         if (reversed) {
-          answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
-                                            overwrite_mode, no_negative_zero);
+          answer = LikelySmiBinaryOperation(expr, &constant_operand, operand,
+                                            overwrite_mode);
         } else {
-          answer = LikelySmiBinaryOperation(op, operand, &constant_operand,
-                                            overwrite_mode, no_negative_zero);
+          answer = LikelySmiBinaryOperation(expr, operand, &constant_operand,
+                                            overwrite_mode);
         }
       }
       break;
@@ -2367,11 +2363,11 @@ Result CodeGenerator::ConstantSmiBinaryOperation(Token::Value op,
     default: {
       Result constant_operand(value);
       if (reversed) {
-        answer = LikelySmiBinaryOperation(op, &constant_operand, operand,
-                                          overwrite_mode, no_negative_zero);
+        answer = LikelySmiBinaryOperation(expr, &constant_operand, operand,
+                                          overwrite_mode);
       } else {
-        answer = LikelySmiBinaryOperation(op, operand, &constant_operand,
-                                          overwrite_mode, no_negative_zero);
+        answer = LikelySmiBinaryOperation(expr, operand, &constant_operand,
+                                          overwrite_mode);
       }
       break;
     }
@@ -5371,10 +5367,11 @@ void CodeGenerator::EmitSlotAssignment(Assignment* node) {
     bool overwrite_value =
         (node->value()->AsBinaryOperation() != NULL &&
          node->value()->AsBinaryOperation()->ResultOverwriteAllowed());
-    GenericBinaryOperation(node->binary_op(),
-                           node->type(),
-                           overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE,
-                           node->no_negative_zero());
+    // Construct the implicit binary operation.
+    BinaryOperation expr(node, node->binary_op(), node->target(),
+                         node->value());
+    GenericBinaryOperation(&expr,
+                           overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE);
   } else {
     Load(node->value());
   }
@@ -5449,10 +5446,11 @@ void CodeGenerator::EmitNamedPropertyAssignment(Assignment* node) {
     bool overwrite_value =
         (node->value()->AsBinaryOperation() != NULL &&
          node->value()->AsBinaryOperation()->ResultOverwriteAllowed());
-    GenericBinaryOperation(node->binary_op(),
-                           node->type(),
-                           overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE,
-                           node->no_negative_zero());
+    // Construct the implicit binary operation.
+    BinaryOperation expr(node, node->binary_op(), node->target(),
+                         node->value());
+    GenericBinaryOperation(&expr,
+                           overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE);
   } else {
     Load(node->value());
   }
@@ -5529,10 +5527,10 @@ void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) {
     bool overwrite_value =
         (node->value()->AsBinaryOperation() != NULL &&
          node->value()->AsBinaryOperation()->ResultOverwriteAllowed());
-    GenericBinaryOperation(node->binary_op(),
-                           node->type(),
-                           overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE,
-                           node->no_negative_zero());
+    BinaryOperation expr(node, node->binary_op(), node->target(),
+                         node->value());
+    GenericBinaryOperation(&expr,
+                           overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE);
   } else {
     Load(node->value());
   }
@@ -7636,8 +7634,7 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) {
       Load(node->left());
       Load(node->right());
     }
-    GenericBinaryOperation(node->op(), node->type(),
-                           overwrite_mode, node->no_negative_zero());
+    GenericBinaryOperation(node, overwrite_mode);
   }
 }
 

src/ia32/codegen-ia32.h

@@ -492,11 +492,8 @@ class CodeGenerator: public AstVisitor {
   // Generate code that computes a shortcutting logical operation.
   void GenerateLogicalBooleanOperation(BinaryOperation* node);
 
-  void GenericBinaryOperation(
-      Token::Value op,
-      StaticType* type,
-      OverwriteMode overwrite_mode,
-      bool no_negative_zero);
+  void GenericBinaryOperation(BinaryOperation* expr,
+                              OverwriteMode overwrite_mode);
 
   // If possible, combine two constant smi values using op to produce
   // a smi result, and push it on the virtual frame, all at compile time.
@@ -505,22 +502,19 @@ class CodeGenerator: public AstVisitor {
 
   // Emit code to perform a binary operation on a constant
   // smi and a likely smi.  Consumes the Result operand.
-  Result ConstantSmiBinaryOperation(Token::Value op,
+  Result ConstantSmiBinaryOperation(BinaryOperation* expr,
                                     Result* operand,
                                     Handle<Object> constant_operand,
-                                    StaticType* type,
                                     bool reversed,
-                                    OverwriteMode overwrite_mode,
-                                    bool no_negative_zero);
+                                    OverwriteMode overwrite_mode);
 
   // Emit code to perform a binary operation on two likely smis.
   // The code to handle smi arguments is produced inline.
   // Consumes the Results left and right.
-  Result LikelySmiBinaryOperation(Token::Value op,
+  Result LikelySmiBinaryOperation(BinaryOperation* expr,
                                   Result* left,
                                   Result* right,
-                                  OverwriteMode overwrite_mode,
-                                  bool no_negative_zero);
+                                  OverwriteMode overwrite_mode);
 
 
   // Emit code to perform a binary operation on two untagged int32 values.