Add support to the CFG builder for non-short-circuited binary

expressions.  Add compilation support in fast mode (optimized for
compilation time and code size).

This breaks one debugger test each on ARM and IA32 (expectedly).
Review URL: http://codereview.chromium.org/160584

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

src/arm/cfg-arm.cc

@@ -29,6 +29,7 @@
 
 #include "cfg.h"
 #include "codegen-inl.h"
+#include "codegen-arm.h"  // Include after codegen-inl.h.
 #include "macro-assembler-arm.h"
 
 namespace v8 {
@@ -42,6 +43,14 @@ void InstructionBlock::Compile(MacroAssembler* masm) {
   {
     Comment cmt(masm, "[ InstructionBlock");
     for (int i = 0, len = instructions_.length(); i < len; i++) {
+      // If the location of the current instruction is a temp, then the
+      // instruction cannot be in tail position in the block.  Allocate the
+      // temp based on peeking ahead to the next instruction.
+      Instruction* instr = instructions_[i];
+      Location* loc = instr->location();
+      if (loc->is_temporary()) {
+        instructions_[i+1]->FastAllocate(TempLocation::cast(loc));
+      }
       instructions_[i]->Compile(masm);
     }
   }
@@ -91,34 +100,124 @@ void ExitNode::Compile(MacroAssembler* masm) {
 }
 
 
+void BinaryOpInstr::Compile(MacroAssembler* masm) {
+  // The right-hand value should not be on the stack---if it is a
+  // compiler-generated temporary it is in the accumulator.
+  ASSERT(!val1_->is_on_stack());
+
+  Comment cmnt(masm, "[ BinaryOpInstr");
+  // We can overwrite one of the operands if it is a temporary.
+  OverwriteMode mode = NO_OVERWRITE;
+  if (val0_->is_temporary()) {
+    mode = OVERWRITE_LEFT;
+  } else if (val1_->is_temporary()) {
+    mode = OVERWRITE_RIGHT;
+  }
+
+  // Move left to r1 and right to r0.
+  val0_->Get(masm, r1);
+  val1_->Get(masm, r0);
+  GenericBinaryOpStub stub(op_, mode);
+  __ CallStub(&stub);
+  loc_->Set(masm, r0);
+}
+
+
 void ReturnInstr::Compile(MacroAssembler* masm) {
+  // The location should be 'Effect'.  As a side effect, move the value to
+  // the accumulator.
   Comment cmnt(masm, "[ ReturnInstr");
-  value_->ToRegister(masm, r0);
+  value_->Get(masm, r0);
 }
 
 
-void Constant::ToRegister(MacroAssembler* masm, Register reg) {
+void Constant::Get(MacroAssembler* masm, Register reg) {
   __ mov(reg, Operand(handle_));
 }
 
 
-void SlotLocation::ToRegister(MacroAssembler* masm, Register reg) {
-  switch (type_) {
+void Constant::Push(MacroAssembler* masm) {
+  __ mov(ip, Operand(handle_));
+  __ push(ip);
+}
+
+
+static MemOperand ToMemOperand(SlotLocation* loc) {
+  switch (loc->type()) {
     case Slot::PARAMETER: {
       int count = CfgGlobals::current()->fun()->scope()->num_parameters();
-      __ ldr(reg, MemOperand(fp, (1 + count - index_) * kPointerSize));
-      break;
+      return MemOperand(fp, (1 + count - loc->index()) * kPointerSize);
     }
     case Slot::LOCAL: {
       const int kOffset = JavaScriptFrameConstants::kLocal0Offset;
-      __ ldr(reg, MemOperand(fp, kOffset - index_ * kPointerSize));
-      break;
+      return MemOperand(fp, kOffset - loc->index() * kPointerSize);
     }
     default:
       UNREACHABLE();
+      return MemOperand(r0);
   }
 }
 
+
+void SlotLocation::Get(MacroAssembler* masm, Register reg) {
+  __ ldr(reg, ToMemOperand(this));
+}
+
+
+void SlotLocation::Set(MacroAssembler* masm, Register reg) {
+  __ str(reg, ToMemOperand(this));
+}
+
+
+void SlotLocation::Push(MacroAssembler* masm) {
+  __ ldr(ip, ToMemOperand(this));
+  __ push(ip);  // Push will not destroy ip.
+}
+
+
+void TempLocation::Get(MacroAssembler* masm, Register reg) {
+  switch (where_) {
+    case ACCUMULATOR:
+      if (!reg.is(r0)) __ mov(reg, r0);
+      break;
+    case STACK:
+      __ pop(reg);
+      break;
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
+void TempLocation::Set(MacroAssembler* masm, Register reg) {
+  switch (where_) {
+    case ACCUMULATOR:
+      if (!reg.is(r0)) __ mov(r0, reg);
+      break;
+    case STACK:
+      __ push(reg);
+      break;
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
+void TempLocation::Push(MacroAssembler* masm) {
+  switch (where_) {
+    case ACCUMULATOR:
+      __ push(r0);
+      break;
+    case STACK:
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
 #undef __
 
 } }  // namespace v8::internal

src/arm/codegen-arm.cc

@@ -697,96 +697,6 @@ void CodeGenerator::ToBoolean(JumpTarget* true_target,
 }
 
 
-class GenericBinaryOpStub : public CodeStub {
- public:
-  GenericBinaryOpStub(Token::Value op,
-                      OverwriteMode mode,
-                      int constant_rhs = CodeGenerator::kUnknownIntValue)
-      : op_(op),
-        mode_(mode),
-        constant_rhs_(constant_rhs),
-        specialized_on_rhs_(RhsIsOneWeWantToOptimizeFor(op, constant_rhs)) { }
-
- private:
-  Token::Value op_;
-  OverwriteMode mode_;
-  int constant_rhs_;
-  bool specialized_on_rhs_;
-
-  static const int kMaxKnownRhs = 0x40000000;
-
-  // Minor key encoding in 16 bits.
-  class ModeBits: public BitField<OverwriteMode, 0, 2> {};
-  class OpBits: public BitField<Token::Value, 2, 6> {};
-  class KnownIntBits: public BitField<int, 8, 8> {};
-
-  Major MajorKey() { return GenericBinaryOp; }
-  int MinorKey() {
-    // Encode the parameters in a unique 16 bit value.
-    return OpBits::encode(op_)
-           | ModeBits::encode(mode_)
-           | KnownIntBits::encode(MinorKeyForKnownInt());
-  }
-
-  void Generate(MacroAssembler* masm);
-  void HandleNonSmiBitwiseOp(MacroAssembler* masm);
-
-  static bool RhsIsOneWeWantToOptimizeFor(Token::Value op, int constant_rhs) {
-    if (constant_rhs == CodeGenerator::kUnknownIntValue) return false;
-    if (op == Token::DIV) return constant_rhs >= 2 && constant_rhs <= 3;
-    if (op == Token::MOD) {
-      if (constant_rhs <= 1) return false;
-      if (constant_rhs <= 10) return true;
-      if (constant_rhs <= kMaxKnownRhs && IsPowerOf2(constant_rhs)) return true;
-      return false;
-    }
-    return false;
-  }
-
-  int MinorKeyForKnownInt() {
-    if (!specialized_on_rhs_) return 0;
-    if (constant_rhs_ <= 10) return constant_rhs_ + 1;
-    ASSERT(IsPowerOf2(constant_rhs_));
-    int key = 12;
-    int d = constant_rhs_;
-    while ((d & 1) == 0) {
-      key++;
-      d >>= 1;
-    }
-    return key;
-  }
-
-  const char* GetName() {
-    switch (op_) {
-      case Token::ADD: return "GenericBinaryOpStub_ADD";
-      case Token::SUB: return "GenericBinaryOpStub_SUB";
-      case Token::MUL: return "GenericBinaryOpStub_MUL";
-      case Token::DIV: return "GenericBinaryOpStub_DIV";
-      case Token::MOD: return "GenericBinaryOpStub_MOD";
-      case Token::BIT_OR: return "GenericBinaryOpStub_BIT_OR";
-      case Token::BIT_AND: return "GenericBinaryOpStub_BIT_AND";
-      case Token::BIT_XOR: return "GenericBinaryOpStub_BIT_XOR";
-      case Token::SAR: return "GenericBinaryOpStub_SAR";
-      case Token::SHL: return "GenericBinaryOpStub_SHL";
-      case Token::SHR: return "GenericBinaryOpStub_SHR";
-      default:         return "GenericBinaryOpStub";
-    }
-  }
-
-#ifdef DEBUG
-  void Print() {
-    if (!specialized_on_rhs_) {
-      PrintF("GenericBinaryOpStub (%s)\n", Token::String(op_));
-    } else {
-      PrintF("GenericBinaryOpStub (%s by %d)\n",
-             Token::String(op_),
-             constant_rhs_);
-    }
-  }
-#endif
-};
-
-
 void CodeGenerator::GenericBinaryOperation(Token::Value op,
                                            OverwriteMode overwrite_mode,
                                            int constant_rhs) {

src/arm/codegen-arm.h

@@ -421,6 +421,96 @@ class CodeGenerator: public AstVisitor {
 };
 
 
+class GenericBinaryOpStub : public CodeStub {
+ public:
+  GenericBinaryOpStub(Token::Value op,
+                      OverwriteMode mode,
+                      int constant_rhs = CodeGenerator::kUnknownIntValue)
+      : op_(op),
+        mode_(mode),
+        constant_rhs_(constant_rhs),
+        specialized_on_rhs_(RhsIsOneWeWantToOptimizeFor(op, constant_rhs)) { }
+
+ private:
+  Token::Value op_;
+  OverwriteMode mode_;
+  int constant_rhs_;
+  bool specialized_on_rhs_;
+
+  static const int kMaxKnownRhs = 0x40000000;
+
+  // Minor key encoding in 16 bits.
+  class ModeBits: public BitField<OverwriteMode, 0, 2> {};
+  class OpBits: public BitField<Token::Value, 2, 6> {};
+  class KnownIntBits: public BitField<int, 8, 8> {};
+
+  Major MajorKey() { return GenericBinaryOp; }
+  int MinorKey() {
+    // Encode the parameters in a unique 16 bit value.
+    return OpBits::encode(op_)
+           | ModeBits::encode(mode_)
+           | KnownIntBits::encode(MinorKeyForKnownInt());
+  }
+
+  void Generate(MacroAssembler* masm);
+  void HandleNonSmiBitwiseOp(MacroAssembler* masm);
+
+  static bool RhsIsOneWeWantToOptimizeFor(Token::Value op, int constant_rhs) {
+    if (constant_rhs == CodeGenerator::kUnknownIntValue) return false;
+    if (op == Token::DIV) return constant_rhs >= 2 && constant_rhs <= 3;
+    if (op == Token::MOD) {
+      if (constant_rhs <= 1) return false;
+      if (constant_rhs <= 10) return true;
+      if (constant_rhs <= kMaxKnownRhs && IsPowerOf2(constant_rhs)) return true;
+      return false;
+    }
+    return false;
+  }
+
+  int MinorKeyForKnownInt() {
+    if (!specialized_on_rhs_) return 0;
+    if (constant_rhs_ <= 10) return constant_rhs_ + 1;
+    ASSERT(IsPowerOf2(constant_rhs_));
+    int key = 12;
+    int d = constant_rhs_;
+    while ((d & 1) == 0) {
+      key++;
+      d >>= 1;
+    }
+    return key;
+  }
+
+  const char* GetName() {
+    switch (op_) {
+      case Token::ADD: return "GenericBinaryOpStub_ADD";
+      case Token::SUB: return "GenericBinaryOpStub_SUB";
+      case Token::MUL: return "GenericBinaryOpStub_MUL";
+      case Token::DIV: return "GenericBinaryOpStub_DIV";
+      case Token::MOD: return "GenericBinaryOpStub_MOD";
+      case Token::BIT_OR: return "GenericBinaryOpStub_BIT_OR";
+      case Token::BIT_AND: return "GenericBinaryOpStub_BIT_AND";
+      case Token::BIT_XOR: return "GenericBinaryOpStub_BIT_XOR";
+      case Token::SAR: return "GenericBinaryOpStub_SAR";
+      case Token::SHL: return "GenericBinaryOpStub_SHL";
+      case Token::SHR: return "GenericBinaryOpStub_SHR";
+      default:         return "GenericBinaryOpStub";
+    }
+  }
+
+#ifdef DEBUG
+  void Print() {
+    if (!specialized_on_rhs_) {
+      PrintF("GenericBinaryOpStub (%s)\n", Token::String(op_));
+    } else {
+      PrintF("GenericBinaryOpStub (%s by %d)\n",
+             Token::String(op_),
+             constant_rhs_);
+    }
+  }
+#endif
+};
+
+
 } }  // namespace v8::internal
 
 #endif  // V8_ARM_CODEGEN_ARM_H_

src/cfg.cc

@@ -42,8 +42,10 @@ CfgGlobals* CfgGlobals::top_ = NULL;
 CfgGlobals::CfgGlobals(FunctionLiteral* fun)
     : global_fun_(fun),
       global_exit_(new ExitNode()),
+      effect_(new Effect()),
 #ifdef DEBUG
       node_counter_(0),
+      temp_counter_(0),
 #endif
       previous_(top_) {
   top_ = this;
@@ -73,6 +75,9 @@ Cfg* Cfg::Build() {
   if (cfg == NULL) {
     BAILOUT("unsupported statement type");
   }
+  if (cfg->is_empty()) {
+    BAILOUT("function body produces empty cfg");
+  }
   if (cfg->has_exit()) {
     BAILOUT("control path without explicit return");
   }
@@ -90,8 +95,10 @@ void Cfg::PrependEntryNode() {
 
 
 void Cfg::Append(Instruction* instr) {
-  ASSERT(has_exit());
-  ASSERT(!is_empty());
+  ASSERT(is_empty() || has_exit());
+  if (is_empty()) {
+    entry_ = exit_ = new InstructionBlock();
+  }
   InstructionBlock::cast(exit_)->Append(instr);
 }
 
@@ -104,6 +111,27 @@ void Cfg::AppendReturnInstruction(Value* value) {
 }
 
 
+void Cfg::Concatenate(Cfg* other) {
+  ASSERT(is_empty() || has_exit());
+  if (other->is_empty()) return;
+
+  if (is_empty()) {
+    entry_ = other->entry();
+    exit_ = other->exit();
+  } else {
+    // We have a pair of nonempty fragments and this has an available exit.
+    // Destructively glue the fragments together.
+    InstructionBlock* first = InstructionBlock::cast(exit_);
+    InstructionBlock* second = InstructionBlock::cast(other->entry());
+    first->instructions()->AddAll(*second->instructions());
+    if (second->successor() != NULL) {
+      first->set_successor(second->successor());
+      exit_ = other->exit();
+    }
+  }
+}
+
+
 void InstructionBlock::Unmark() {
   if (is_marked_) {
     is_marked_ = false;
@@ -166,6 +194,26 @@ Handle<Code> Cfg::Compile(Handle<Script> script) {
 }
 
 
+void BinaryOpInstr::FastAllocate(TempLocation* temp) {
+  ASSERT(temp->where() == TempLocation::NOWHERE);
+  if (temp == val0_ || temp == val1_) {
+    temp->set_where(TempLocation::ACCUMULATOR);
+  } else {
+    temp->set_where(TempLocation::STACK);
+  }
+}
+
+
+void ReturnInstr::FastAllocate(TempLocation* temp) {
+  ASSERT(temp->where() == TempLocation::NOWHERE);
+  if (temp == value_) {
+    temp->set_where(TempLocation::ACCUMULATOR);
+  } else {
+    temp->set_where(TempLocation::STACK);
+  }
+}
+
+
 // The expression builder should not be used for declarations or statements.
 void ExpressionBuilder::VisitDeclaration(Declaration* decl) { UNREACHABLE(); }
 
@@ -178,13 +226,10 @@ STATEMENT_NODE_LIST(DEFINE_VISIT)
 // Macros (temporarily) handling unsupported expression types.
 #define BAILOUT(reason)                         \
   do {                                          \
-    value_ = NULL;                              \
+    cfg_ = NULL;                                \
     return;                                     \
   } while (false)
 
-#define CHECK_BAILOUT()                         \
-  if (value_ == NULL) { return; } else {}
-
 void ExpressionBuilder::VisitFunctionLiteral(FunctionLiteral* expr) {
   BAILOUT("FunctionLiteral");
 }
@@ -290,7 +335,46 @@ void ExpressionBuilder::VisitCountOperation(CountOperation* expr) {
 
 
 void ExpressionBuilder::VisitBinaryOperation(BinaryOperation* expr) {
-  BAILOUT("BinaryOperation");
+  Token::Value op = expr->op();
+  switch (op) {
+    case Token::COMMA:
+    case Token::OR:
+    case Token::AND:
+      BAILOUT("unsupported binary operation");
+
+    case Token::BIT_OR:
+    case Token::BIT_XOR:
+    case Token::BIT_AND:
+    case Token::SHL:
+    case Token::SAR:
+    case Token::SHR:
+    case Token::ADD:
+    case Token::SUB:
+    case Token::MUL:
+    case Token::DIV:
+    case Token::MOD: {
+      ExpressionBuilder left, right;
+      left.Build(expr->left());
+      if (left.cfg() == NULL) {
+        BAILOUT("unsupported left subexpression in binop");
+      }
+      right.Build(expr->right());
+      if (right.cfg() == NULL) {
+        BAILOUT("unsupported right subexpression in binop");
+      }
+
+      Location* temp = new TempLocation();
+      cfg_ = left.cfg();
+      cfg_->Concatenate(right.cfg());
+      cfg_->Append(new BinaryOpInstr(temp, op, left.value(), right.value()));
+
+      value_ = temp;
+      return;
+    }
+
+    default:
+      UNREACHABLE();
+  }
 }
 
 
@@ -304,7 +388,6 @@ void ExpressionBuilder::VisitThisFunction(ThisFunction* expr) {
 }
 
 #undef BAILOUT
-#undef CHECK_BAILOUT
 
 
 // Macros (temporarily) handling unsupported statement types.
@@ -367,10 +450,13 @@ void StatementBuilder::VisitBreakStatement(BreakStatement* stmt) {
 
 void StatementBuilder::VisitReturnStatement(ReturnStatement* stmt) {
   ExpressionBuilder builder;
-  builder.Visit(stmt->expression());
-  Value* value = builder.value();
-  if (value == NULL) BAILOUT("unsupported expression type");
-  cfg_->AppendReturnInstruction(value);
+  builder.Build(stmt->expression());
+  if (builder.cfg() == NULL) {
+    BAILOUT("unsupported expression in return statement");
+  }
+
+  cfg_->Concatenate(builder.cfg());
+  cfg_->AppendReturnInstruction(builder.value());
 }
 
 
@@ -430,6 +516,11 @@ void Constant::Print() {
 }
 
 
+void Effect::Print() {
+  PrintF("Effect");
+}
+
+
 void SlotLocation::Print() {
   PrintF("Slot(");
   switch (type_) {
@@ -445,10 +536,26 @@ void SlotLocation::Print() {
 }
 
 
+void TempLocation::Print() {
+  PrintF("Temp(%d)", number());
+}
+
+
+void BinaryOpInstr::Print() {
+  PrintF("BinaryOp(");
+  loc_->Print();
+  PrintF(", %s, ", Token::Name(op_));
+  val0_->Print();
+  PrintF(", ");
+  val1_->Print();
+  PrintF(")\n");
+}
+
+
 void ReturnInstr::Print() {
-  PrintF("Return ");
+  PrintF("Return(");
   value_->Print();
-  PrintF("\n");
+  PrintF(")\n");
 }
 
 

src/ia32/cfg-ia32.cc

@@ -29,6 +29,7 @@
 
 #include "cfg.h"
 #include "codegen-inl.h"
+#include "codegen-ia32.h"
 #include "macro-assembler-ia32.h"
 
 namespace v8 {
@@ -42,6 +43,14 @@ void InstructionBlock::Compile(MacroAssembler* masm) {
   {
     Comment cmt(masm, "[ InstructionBlock");
     for (int i = 0, len = instructions_.length(); i < len; i++) {
+      // If the location of the current instruction is a temp, then the
+      // instruction cannot be in tail position in the block.  Allocate the
+      // temp based on peeking ahead to the next instruction.
+      Instruction* instr = instructions_[i];
+      Location* loc = instr->location();
+      if (loc->is_temporary()) {
+        instructions_[i+1]->FastAllocate(TempLocation::cast(loc));
+      }
       instructions_[i]->Compile(masm);
     }
   }
@@ -79,6 +88,7 @@ void EntryNode::Compile(MacroAssembler* masm) {
   }
   successor_->Compile(masm);
   if (FLAG_check_stack) {
+    Comment cmnt(masm, "[ Deferred Stack Check");
     __ bind(&deferred_enter);
     StackCheckStub stub;
     __ CallStub(&stub);
@@ -103,31 +113,120 @@ void ExitNode::Compile(MacroAssembler* masm) {
 }
 
 
+void BinaryOpInstr::Compile(MacroAssembler* masm) {
+  // The right-hand value should not be on the stack---if it is a
+  // compiler-generated temporary it is in the accumulator.
+  ASSERT(!val1_->is_on_stack());
+
+  Comment cmnt(masm, "[ BinaryOpInstr");
+  // We can overwrite one of the operands if it is a temporary.
+  OverwriteMode mode = NO_OVERWRITE;
+  if (val0_->is_temporary()) {
+    mode = OVERWRITE_LEFT;
+  } else if (val1_->is_temporary()) {
+    mode = OVERWRITE_RIGHT;
+  }
+
+  // Push both operands and call the specialized stub.
+  if (!val0_->is_on_stack()) {
+    val0_->Push(masm);
+  }
+  val1_->Push(masm);
+  GenericBinaryOpStub stub(op_, mode, SMI_CODE_IN_STUB);
+  __ CallStub(&stub);
+  loc_->Set(masm, eax);
+}
+
+
 void ReturnInstr::Compile(MacroAssembler* masm) {
+  // The location should be 'Effect'.  As a side effect, move the value to
+  // the accumulator.
   Comment cmnt(masm, "[ ReturnInstr");
-  value_->ToRegister(masm, eax);
+  value_->Get(masm, eax);
 }
 
 
-void Constant::ToRegister(MacroAssembler* masm, Register reg) {
+void Constant::Get(MacroAssembler* masm, Register reg) {
   __ mov(reg, Immediate(handle_));
 }
 
 
-void SlotLocation::ToRegister(MacroAssembler* masm, Register reg) {
-  switch (type_) {
+void Constant::Push(MacroAssembler* masm) {
+  __ push(Immediate(handle_));
+}
+
+
+static Operand ToOperand(SlotLocation* loc) {
+  switch (loc->type()) {
     case Slot::PARAMETER: {
       int count = CfgGlobals::current()->fun()->scope()->num_parameters();
-      __ mov(reg, Operand(ebp, (1 + count - index_) * kPointerSize));
-      break;
+      return Operand(ebp, (1 + count - loc->index()) * kPointerSize);
     }
     case Slot::LOCAL: {
       const int kOffset = JavaScriptFrameConstants::kLocal0Offset;
-      __ mov(reg, Operand(ebp, kOffset - index_ * kPointerSize));
-      break;
+      return Operand(ebp, kOffset - loc->index() * kPointerSize);
     }
     default:
       UNREACHABLE();
+      return Operand(eax);
+  }
+}
+
+
+void SlotLocation::Get(MacroAssembler* masm, Register reg) {
+  __ mov(reg, ToOperand(this));
+}
+
+
+void SlotLocation::Set(MacroAssembler* masm, Register reg) {
+  __ mov(ToOperand(this), reg);
+}
+
+
+void SlotLocation::Push(MacroAssembler* masm) {
+  __ push(ToOperand(this));
+}
+
+
+void TempLocation::Get(MacroAssembler* masm, Register reg) {
+  switch (where_) {
+    case ACCUMULATOR:
+      if (!reg.is(eax)) __ mov(reg, eax);
+      break;
+    case STACK:
+      __ pop(reg);
+      break;
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
+void TempLocation::Set(MacroAssembler* masm, Register reg) {
+  switch (where_) {
+    case ACCUMULATOR:
+      if (!reg.is(eax)) __ mov(eax, reg);
+      break;
+    case STACK:
+      __ push(reg);
+      break;
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
+void TempLocation::Push(MacroAssembler* masm) {
+  switch (where_) {
+    case ACCUMULATOR:
+      __ push(eax);
+      break;
+    case STACK:
+    case NOWHERE:
+      UNREACHABLE();
+      break;
   }
 }
 

src/ia32/codegen-ia32.cc

@@ -778,57 +778,6 @@ class FloatingPointHelper : public AllStatic {
 };
 
 
-// Flag that indicates whether or not the code that handles smi arguments
-// should be placed in the stub, inlined, or omitted entirely.
-enum GenericBinaryFlags {
-  SMI_CODE_IN_STUB,
-  SMI_CODE_INLINED
-};
-
-
-class GenericBinaryOpStub: public CodeStub {
- public:
-  GenericBinaryOpStub(Token::Value op,
-                      OverwriteMode mode,
-                      GenericBinaryFlags flags)
-      : op_(op), mode_(mode), flags_(flags) {
-    ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
-  }
-
-  void GenerateSmiCode(MacroAssembler* masm, Label* slow);
-
- private:
-  Token::Value op_;
-  OverwriteMode mode_;
-  GenericBinaryFlags flags_;
-
-  const char* GetName();
-
-#ifdef DEBUG
-  void Print() {
-    PrintF("GenericBinaryOpStub (op %s), (mode %d, flags %d)\n",
-           Token::String(op_),
-           static_cast<int>(mode_),
-           static_cast<int>(flags_));
-  }
-#endif
-
-  // Minor key encoding in 16 bits FOOOOOOOOOOOOOMM.
-  class ModeBits: public BitField<OverwriteMode, 0, 2> {};
-  class OpBits: public BitField<Token::Value, 2, 13> {};
-  class FlagBits: public BitField<GenericBinaryFlags, 15, 1> {};
-
-  Major MajorKey() { return GenericBinaryOp; }
-  int MinorKey() {
-    // Encode the parameters in a unique 16 bit value.
-    return OpBits::encode(op_)
-           | ModeBits::encode(mode_)
-           | FlagBits::encode(flags_);
-  }
-  void Generate(MacroAssembler* masm);
-};
-
-
 const char* GenericBinaryOpStub::GetName() {
   switch (op_) {
     case Token::ADD: return "GenericBinaryOpStub_ADD";

src/ia32/codegen-ia32.h

@@ -609,6 +609,57 @@ class CodeGenerator: public AstVisitor {
 };
 
 
+// Flag that indicates whether or not the code that handles smi arguments
+// should be placed in the stub, inlined, or omitted entirely.
+enum GenericBinaryFlags {
+  SMI_CODE_IN_STUB,
+  SMI_CODE_INLINED
+};
+
+
+class GenericBinaryOpStub: public CodeStub {
+ public:
+  GenericBinaryOpStub(Token::Value op,
+                      OverwriteMode mode,
+                      GenericBinaryFlags flags)
+      : op_(op), mode_(mode), flags_(flags) {
+    ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
+  }
+
+  void GenerateSmiCode(MacroAssembler* masm, Label* slow);
+
+ private:
+  Token::Value op_;
+  OverwriteMode mode_;
+  GenericBinaryFlags flags_;
+
+  const char* GetName();
+
+#ifdef DEBUG
+  void Print() {
+    PrintF("GenericBinaryOpStub (op %s), (mode %d, flags %d)\n",
+           Token::String(op_),
+           static_cast<int>(mode_),
+           static_cast<int>(flags_));
+  }
+#endif
+
+  // Minor key encoding in 16 bits FOOOOOOOOOOOOOMM.
+  class ModeBits: public BitField<OverwriteMode, 0, 2> {};
+  class OpBits: public BitField<Token::Value, 2, 13> {};
+  class FlagBits: public BitField<GenericBinaryFlags, 15, 1> {};
+
+  Major MajorKey() { return GenericBinaryOp; }
+  int MinorKey() {
+    // Encode the parameters in a unique 16 bit value.
+    return OpBits::encode(op_)
+           | ModeBits::encode(mode_)
+           | FlagBits::encode(flags_);
+  }
+  void Generate(MacroAssembler* masm);
+};
+
+
 } }  // namespace v8::internal
 
 #endif  // V8_IA32_CODEGEN_IA32_H_

src/x64/cfg-x64.cc

@@ -29,6 +29,7 @@
 
 #include "cfg.h"
 #include "codegen-inl.h"
+#include "codegen-x64.h"
 #include "debug.h"
 #include "macro-assembler-x64.h"
 
@@ -43,6 +44,14 @@ void InstructionBlock::Compile(MacroAssembler* masm) {
   {
     Comment cmt(masm, "[ InstructionBlock");
     for (int i = 0, len = instructions_.length(); i < len; i++) {
+      // If the location of the current instruction is a temp, then the
+      // instruction cannot be in tail position in the block.  Allocate the
+      // temp based on peeking ahead to the next instruction.
+      Instruction* instr = instructions_[i];
+      Location* loc = instr->location();
+      if (loc->is_temporary()) {
+        instructions_[i+1]->FastAllocate(TempLocation::cast(loc));
+      }
       instructions_[i]->Compile(masm);
     }
   }
@@ -82,6 +91,7 @@ void EntryNode::Compile(MacroAssembler* masm) {
   }
   successor_->Compile(masm);
   if (FLAG_check_stack) {
+    Comment cmnt(masm, "[ Deferred Stack Check");
     __ bind(&deferred_enter);
     StackCheckStub stub;
     __ CallStub(&stub);
@@ -113,34 +123,124 @@ void ExitNode::Compile(MacroAssembler* masm) {
 }
 
 
+void BinaryOpInstr::Compile(MacroAssembler* masm) {
+  // The right-hand value should not be on the stack---if it is a
+  // compiler-generated temporary it is in the accumulator.
+  ASSERT(!val1_->is_on_stack());
+
+  Comment cmnt(masm, "[ BinaryOpInstr");
+  // We can overwrite one of the operands if it is a temporary.
+  OverwriteMode mode = NO_OVERWRITE;
+  if (val0_->is_temporary()) {
+    mode = OVERWRITE_LEFT;
+  } else if (val1_->is_temporary()) {
+    mode = OVERWRITE_RIGHT;
+  }
+
+  // Push both operands and call the specialized stub.
+  if (!val0_->is_on_stack()) {
+    val0_->Push(masm);
+  }
+  val1_->Push(masm);
+  GenericBinaryOpStub stub(op_, mode, SMI_CODE_IN_STUB);
+  __ CallStub(&stub);
+  loc_->Set(masm, rax);
+}
+
+
 void ReturnInstr::Compile(MacroAssembler* masm) {
+  // The location should be 'Effect'.  As a side effect, move the value to
+  // the accumulator.
   Comment cmnt(masm, "[ ReturnInstr");
-  value_->ToRegister(masm, rax);
+  value_->Get(masm, rax);
 }
 
 
-void Constant::ToRegister(MacroAssembler* masm, Register reg) {
+void Constant::Get(MacroAssembler* masm, Register reg) {
   __ Move(reg, handle_);
 }
 
 
-void SlotLocation::ToRegister(MacroAssembler* masm, Register reg) {
-  switch (type_) {
+void Constant::Push(MacroAssembler* masm) {
+  __ Push(handle_);
+}
+
+
+static Operand ToOperand(SlotLocation* loc) {
+  switch (loc->type()) {
     case Slot::PARAMETER: {
       int count = CfgGlobals::current()->fun()->scope()->num_parameters();
-      __ movq(reg, Operand(rbp, (1 + count - index_) * kPointerSize));
-      break;
+      return Operand(rbp, (1 + count - loc->index()) * kPointerSize);
     }
     case Slot::LOCAL: {
       const int kOffset = JavaScriptFrameConstants::kLocal0Offset;
-      __ movq(reg, Operand(rbp, kOffset - index_ * kPointerSize));
-      break;
+      return Operand(rbp, kOffset - loc->index() * kPointerSize);
     }
     default:
       UNREACHABLE();
+      return Operand(rax, 0);
+  }
+}
+
+
+void SlotLocation::Get(MacroAssembler* masm, Register reg) {
+  __ movq(reg, ToOperand(this));
+}
+
+
+void SlotLocation::Set(MacroAssembler* masm, Register reg) {
+  __ movq(ToOperand(this), reg);
+}
+
+
+void SlotLocation::Push(MacroAssembler* masm) {
+  __ push(ToOperand(this));
+}
+
+
+void TempLocation::Get(MacroAssembler* masm, Register reg) {
+  switch (where_) {
+    case ACCUMULATOR:
+      if (!reg.is(rax)) __ movq(reg, rax);
+      break;
+    case STACK:
+      __ pop(reg);
+      break;
+    case NOWHERE:
+      UNREACHABLE();
+      break;
   }
 }
 
+
+void TempLocation::Set(MacroAssembler* masm, Register reg) {
+  switch (where_) {
+    case ACCUMULATOR:
+      if (!reg.is(rax)) __ movq(rax, reg);
+      break;
+    case STACK:
+      __ push(reg);
+      break;
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
+void TempLocation::Push(MacroAssembler* masm) {
+  switch (where_) {
+    case ACCUMULATOR:
+      __ push(rax);
+      break;
+    case STACK:
+    case NOWHERE:
+      UNREACHABLE();
+      break;
+  }
+}
+
+
 #undef __
 
 } }  // namespace v8::internal

src/x64/codegen-x64.cc

@@ -4534,14 +4534,6 @@ void CodeGenerator::Comparison(Condition cc,
 }
 
 
-// Flag that indicates whether or not the code that handles smi arguments
-// should be placed in the stub, inlined, or omitted entirely.
-enum GenericBinaryFlags {
-  SMI_CODE_IN_STUB,
-  SMI_CODE_INLINED
-};
-
-
 class FloatingPointHelper : public AllStatic {
  public:
   // Code pattern for loading a floating point value. Input value must
@@ -4593,49 +4585,6 @@ class FloatingPointHelper : public AllStatic {
 };
 
 
-class GenericBinaryOpStub: public CodeStub {
- public:
-  GenericBinaryOpStub(Token::Value op,
-                      OverwriteMode mode,
-                      GenericBinaryFlags flags)
-      : op_(op), mode_(mode), flags_(flags) {
-    ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
-  }
-
-  void GenerateSmiCode(MacroAssembler* masm, Label* slow);
-
- private:
-  Token::Value op_;
-  OverwriteMode mode_;
-  GenericBinaryFlags flags_;
-
-  const char* GetName();
-
-#ifdef DEBUG
-  void Print() {
-    PrintF("GenericBinaryOpStub (op %s), (mode %d, flags %d)\n",
-           Token::String(op_),
-           static_cast<int>(mode_),
-           static_cast<int>(flags_));
-  }
-#endif
-
-  // Minor key encoding in 16 bits FOOOOOOOOOOOOOMM.
-  class ModeBits: public BitField<OverwriteMode, 0, 2> {};
-  class OpBits: public BitField<Token::Value, 2, 13> {};
-  class FlagBits: public BitField<GenericBinaryFlags, 15, 1> {};
-
-  Major MajorKey() { return GenericBinaryOp; }
-  int MinorKey() {
-    // Encode the parameters in a unique 16 bit value.
-    return OpBits::encode(op_)
-           | ModeBits::encode(mode_)
-           | FlagBits::encode(flags_);
-  }
-  void Generate(MacroAssembler* masm);
-};
-
-
 class DeferredInlineBinaryOperation: public DeferredCode {
  public:
   DeferredInlineBinaryOperation(Token::Value op,

src/x64/codegen-x64.h

@@ -601,6 +601,57 @@ class CodeGenerator: public AstVisitor {
 };
 
 
+// Flag that indicates whether or not the code that handles smi arguments
+// should be placed in the stub, inlined, or omitted entirely.
+enum GenericBinaryFlags {
+  SMI_CODE_IN_STUB,
+  SMI_CODE_INLINED
+};
+
+
+class GenericBinaryOpStub: public CodeStub {
+ public:
+  GenericBinaryOpStub(Token::Value op,
+                      OverwriteMode mode,
+                      GenericBinaryFlags flags)
+      : op_(op), mode_(mode), flags_(flags) {
+    ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
+  }
+
+  void GenerateSmiCode(MacroAssembler* masm, Label* slow);
+
+ private:
+  Token::Value op_;
+  OverwriteMode mode_;
+  GenericBinaryFlags flags_;
+
+  const char* GetName();
+
+#ifdef DEBUG
+  void Print() {
+    PrintF("GenericBinaryOpStub (op %s), (mode %d, flags %d)\n",
+           Token::String(op_),
+           static_cast<int>(mode_),
+           static_cast<int>(flags_));
+  }
+#endif
+
+  // Minor key encoding in 16 bits FOOOOOOOOOOOOOMM.
+  class ModeBits: public BitField<OverwriteMode, 0, 2> {};
+  class OpBits: public BitField<Token::Value, 2, 13> {};
+  class FlagBits: public BitField<GenericBinaryFlags, 15, 1> {};
+
+  Major MajorKey() { return GenericBinaryOp; }
+  int MinorKey() {
+    // Encode the parameters in a unique 16 bit value.
+    return OpBits::encode(op_)
+           | ModeBits::encode(mode_)
+           | FlagBits::encode(flags_);
+  }
+  void Generate(MacroAssembler* masm);
+};
+
+
 } }  // namespace v8::internal
 
 #endif  // V8_X64_CODEGEN_X64_H_