Add support for (some) assignment expressions to the CFG builder and

fast-mode compiler.

1. We avoid generating a useless temporary for assignments with
nontrivial right-hand sides.  Instead of translating id = expr into:

...
tmp = <last expr instruction>
id = tmp

we generate directly

...
id = <last expr instruction>

by passing a data destination ('hint') down the AST.  The semantics is
to use the destination as a result location if a temp is needed.  It
may be ignored.  NULL indicates I don't care and you should generate a
temp.

2. We correctly handle assignments as subexpressions.  When building
the CFG for an expression we accumulate the assigned variables and we
emit a move to a fresh temporary if a value in a variable is in
jeopardy of being overwritten.

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

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

src/arm/cfg-arm.cc

@@ -108,26 +108,31 @@ void PositionInstr::Compile(MacroAssembler* masm) {
 }
 
 
+void MoveInstr::Compile(MacroAssembler* masm) {
+  location()->Move(masm, value());
+}
+
+
 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());
+  ASSERT(!value1()->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()) {
+  if (value0()->is_temporary()) {
     mode = OVERWRITE_LEFT;
-  } else if (val1_->is_temporary()) {
+  } else if (value1()->is_temporary()) {
     mode = OVERWRITE_RIGHT;
   }
 
   // Move left to r1 and right to r0.
-  val0_->Get(masm, r1);
+  value0()->Get(masm, r1);
-  val1_->Get(masm, r0);
+  value1()->Get(masm, r0);
-  GenericBinaryOpStub stub(op_, mode);
+  GenericBinaryOpStub stub(op(), mode);
   __ CallStub(&stub);
-  loc_->Set(masm, r0);
+  location()->Set(masm, r0);
 }
 
 
@@ -167,6 +172,12 @@ static MemOperand ToMemOperand(SlotLocation* loc) {
 }
 
 
+void Constant::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  __ mov(ip, Operand(handle_));
+  __ str(ip, ToMemOperand(loc));
+}
+
+
 void SlotLocation::Get(MacroAssembler* masm, Register reg) {
   __ ldr(reg, ToMemOperand(this));
 }
@@ -183,6 +194,18 @@ void SlotLocation::Push(MacroAssembler* masm) {
 }
 
 
+void SlotLocation::Move(MacroAssembler* masm, Value* value) {
+  // Double dispatch.
+  value->MoveToSlot(masm, this);
+}
+
+
+void SlotLocation::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  __ ldr(ip, ToMemOperand(this));
+  __ str(ip, ToMemOperand(loc));
+}
+
+
 void TempLocation::Get(MacroAssembler* masm, Register reg) {
   switch (where_) {
     case ACCUMULATOR:
@@ -191,9 +214,8 @@ void TempLocation::Get(MacroAssembler* masm, Register reg) {
     case STACK:
       __ pop(reg);
       break;
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
@@ -206,9 +228,8 @@ void TempLocation::Set(MacroAssembler* masm, Register reg) {
     case STACK:
       __ push(reg);
       break;
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
@@ -219,13 +240,38 @@ void TempLocation::Push(MacroAssembler* masm) {
       __ push(r0);
       break;
     case STACK:
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
 
+void TempLocation::Move(MacroAssembler* masm, Value* value) {
+  switch (where_) {
+    case ACCUMULATOR:
+      value->Get(masm, r0);
+    case STACK:
+      value->Push(masm);
+      break;
+    case NOT_ALLOCATED:
+      UNREACHABLE();
+  }
+}
+
+
+void TempLocation::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  switch (where_) {
+    case ACCUMULATOR:
+      __ str(r0, ToMemOperand(loc));
+    case STACK:
+      __ pop(ip);
+      __ str(ip, ToMemOperand(loc));
+      break;
+    case NOT_ALLOCATED:
+      UNREACHABLE();
+  }
+}
+
 #undef __
 
 } }  // namespace v8::internal

src/cfg.cc

@@ -42,7 +42,7 @@ CfgGlobals* CfgGlobals::top_ = NULL;
 CfgGlobals::CfgGlobals(FunctionLiteral* fun)
     : global_fun_(fun),
       global_exit_(new ExitNode()),
-      effect_(new Effect()),
+      nowhere_(new Nowhere()),
 #ifdef DEBUG
       node_counter_(0),
       temp_counter_(0),
@@ -60,6 +60,12 @@ Cfg* Cfg::Build() {
   if (fun->scope()->num_heap_slots() > 0) {
     BAILOUT("function has context slots");
   }
+  if (fun->scope()->num_stack_slots() > kPointerSize) {
+    BAILOUT("function has too many locals");
+  }
+  if (fun->scope()->num_parameters() > kPointerSize - 1) {
+    BAILOUT("function has too many parameters");
+  }
   if (fun->scope()->arguments() != NULL) {
     BAILOUT("function uses .arguments");
   }
@@ -71,18 +77,18 @@ Cfg* Cfg::Build() {
 
   StatementBuilder builder;
   builder.VisitStatements(body);
-  Cfg* cfg = builder.cfg();
+  Cfg* graph = builder.graph();
-  if (cfg == NULL) {
+  if (graph == NULL) {
     BAILOUT("unsupported statement type");
   }
-  if (cfg->is_empty()) {
+  if (graph->is_empty()) {
     BAILOUT("function body produces empty cfg");
   }
-  if (cfg->has_exit()) {
+  if (graph->has_exit()) {
     BAILOUT("control path without explicit return");
   }
-  cfg->PrependEntryNode();
+  graph->PrependEntryNode();
-  return cfg;
+  return graph;
 }
 
 #undef BAILOUT
@@ -194,9 +200,19 @@ Handle<Code> Cfg::Compile(Handle<Script> script) {
 }
 
 
+void MoveInstr::FastAllocate(TempLocation* temp) {
+  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
+  if (temp == value()) {
+    temp->set_where(TempLocation::ACCUMULATOR);
+  } else {
+    temp->set_where(TempLocation::STACK);
+  }
+}
+
+
 void BinaryOpInstr::FastAllocate(TempLocation* temp) {
-  ASSERT(temp->where() == TempLocation::NOWHERE);
+  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
-  if (temp == val0_ || temp == val1_) {
+  if (temp == value0() || temp == value1()) {
     temp->set_where(TempLocation::ACCUMULATOR);
   } else {
     temp->set_where(TempLocation::STACK);
@@ -205,7 +221,7 @@ void BinaryOpInstr::FastAllocate(TempLocation* temp) {
 
 
 void ReturnInstr::FastAllocate(TempLocation* temp) {
-  ASSERT(temp->where() == TempLocation::NOWHERE);
+  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
   if (temp == value_) {
     temp->set_where(TempLocation::ACCUMULATOR);
   } else {
@@ -226,7 +242,7 @@ STATEMENT_NODE_LIST(DEFINE_VISIT)
 // Macros (temporarily) handling unsupported expression types.
 #define BAILOUT(reason)                         \
   do {                                          \
-    cfg_ = NULL;                                \
+    graph_ = NULL;                              \
     return;                                     \
   } while (false)
 
@@ -260,11 +276,13 @@ void ExpressionBuilder::VisitVariableProxy(VariableProxy* expr) {
   if (slot->type() != Slot::PARAMETER && slot->type() != Slot::LOCAL) {
     BAILOUT("unsupported slot type (not a parameter or local)");
   }
+  // Ignore the passed destination.
   value_ = new SlotLocation(slot->type(), slot->index());
 }
 
 
 void ExpressionBuilder::VisitLiteral(Literal* expr) {
+  // Ignore the passed destination.
   value_ = new Constant(expr->handle());
 }
 
@@ -290,7 +308,42 @@ void ExpressionBuilder::VisitCatchExtensionObject(CatchExtensionObject* expr) {
 
 
 void ExpressionBuilder::VisitAssignment(Assignment* expr) {
-  BAILOUT("Assignment");
+  if (expr->op() != Token::ASSIGN && expr->op() != Token::INIT_VAR) {
+    BAILOUT("unsupported compound assignment");
+  }
+  Expression* lhs = expr->target();
+  if (lhs->AsProperty() != NULL) {
+    BAILOUT("unsupported property assignment");
+  }
+  Variable* var = lhs->AsVariableProxy()->AsVariable();
+  if (var == NULL) {
+    BAILOUT("unsupported invalid left-hand side");
+  }
+  if (var->is_global()) {
+    BAILOUT("unsupported global variable");
+  }
+  Slot* slot = var->slot();
+  ASSERT(slot != NULL);
+  if (slot->type() != Slot::PARAMETER && slot->type() != Slot::LOCAL) {
+    BAILOUT("unsupported slot lhs (not a parameter or local)");
+  }
+
+  ExpressionBuilder builder;
+  SlotLocation* loc = new SlotLocation(slot->type(), slot->index());
+  builder.Build(expr->value(), loc);
+  if (builder.graph() == NULL) {
+    BAILOUT("unsupported expression in assignment");
+  }
+  // If the expression did not come back in the slot location, append
+  // a move to the CFG.
+  graph_ = builder.graph();
+  if (builder.value() != loc) {
+    graph()->Append(new MoveInstr(loc, builder.value()));
+  }
+  // Record the assignment.
+  assigned_vars_.AddElement(loc);
+  // Ignore the destination passed to us.
+  value_ = loc;
 }
 
 
@@ -354,21 +407,35 @@ void ExpressionBuilder::VisitBinaryOperation(BinaryOperation* expr) {
     case Token::DIV:
     case Token::MOD: {
       ExpressionBuilder left, right;
-      left.Build(expr->left());
+      left.Build(expr->left(), NULL);
-      if (left.cfg() == NULL) {
+      if (left.graph() == NULL) {
         BAILOUT("unsupported left subexpression in binop");
       }
-      right.Build(expr->right());
+      right.Build(expr->right(), NULL);
-      if (right.cfg() == NULL) {
+      if (right.graph() == NULL) {
         BAILOUT("unsupported right subexpression in binop");
       }
 
-      Location* temp = new TempLocation();
+      if (destination_ == NULL) destination_ = new TempLocation();
-      cfg_ = left.cfg();
-      cfg_->Concatenate(right.cfg());
-      cfg_->Append(new BinaryOpInstr(temp, op, left.value(), right.value()));
 
-      value_ = temp;
+      graph_ = left.graph();
+      // Insert a move to a fresh temporary if the left value is in a
+      // slot that's assigned on the right.
+      Location* temp = NULL;
+      if (left.value()->is_slot() &&
+          right.assigned_vars()->Contains(SlotLocation::cast(left.value()))) {
+        temp = new TempLocation();
+        graph()->Append(new MoveInstr(temp, left.value()));
+      }
+      graph()->Concatenate(right.graph());
+      graph()->Append(new BinaryOpInstr(destination_, op,
+                                        temp == NULL ? left.value() : temp,
+                                        right.value()));
+
+      assigned_vars_ = *left.assigned_vars();
+      assigned_vars()->Union(right.assigned_vars());
+
+      value_ = destination_;
       return;
     }
 
@@ -393,18 +460,18 @@ void ExpressionBuilder::VisitThisFunction(ThisFunction* expr) {
 // Macros (temporarily) handling unsupported statement types.
 #define BAILOUT(reason)                         \
   do {                                          \
-    cfg_ = NULL;                                \
+    graph_ = NULL;                              \
     return;                                     \
   } while (false)
 
 #define CHECK_BAILOUT()                         \
-  if (cfg_ == NULL) { return; } else {}
+  if (graph() == NULL) { return; } else {}
 
 void StatementBuilder::VisitStatements(ZoneList<Statement*>* stmts) {
   for (int i = 0, len = stmts->length(); i < len; i++) {
     Visit(stmts->at(i));
     CHECK_BAILOUT();
-    if (!cfg_->has_exit()) return;
+    if (!graph()->has_exit()) return;
   }
 }
 
@@ -425,19 +492,12 @@ void StatementBuilder::VisitBlock(Block* stmt) {
 
 void StatementBuilder::VisitExpressionStatement(ExpressionStatement* stmt) {
   ExpressionBuilder builder;
-  builder.Build(stmt->expression());
+  builder.Build(stmt->expression(), CfgGlobals::current()->nowhere());
-  if (builder.cfg() == NULL) {
+  if (builder.graph() == NULL) {
     BAILOUT("unsupported expression in expression statement");
   }
-  // Here's a temporary hack: we bang on the last instruction of the
+  graph()->Append(new PositionInstr(stmt->statement_pos()));
-  // expression (if any) to set its location to Effect.
+  graph()->Concatenate(builder.graph());
-  if (!builder.cfg()->is_empty()) {
-    InstructionBlock* block = InstructionBlock::cast(builder.cfg()->exit());
-    Instruction* instr = block->instructions()->last();
-    instr->set_location(CfgGlobals::current()->effect_location());
-  }
-  cfg_->Append(new PositionInstr(stmt->statement_pos()));
-  cfg_->Concatenate(builder.cfg());
 }
 
 
@@ -463,14 +523,14 @@ void StatementBuilder::VisitBreakStatement(BreakStatement* stmt) {
 
 void StatementBuilder::VisitReturnStatement(ReturnStatement* stmt) {
   ExpressionBuilder builder;
-  builder.Build(stmt->expression());
+  builder.Build(stmt->expression(), NULL);
-  if (builder.cfg() == NULL) {
+  if (builder.graph() == NULL) {
     BAILOUT("unsupported expression in return statement");
   }
 
-  cfg_->Append(new PositionInstr(stmt->statement_pos()));
+  graph()->Append(new PositionInstr(stmt->statement_pos()));
-  cfg_->Concatenate(builder.cfg());
+  graph()->Concatenate(builder.graph());
-  cfg_->AppendReturnInstruction(builder.value());
+  graph()->AppendReturnInstruction(builder.value());
 }
 
 
@@ -530,8 +590,8 @@ void Constant::Print() {
 }
 
 
-void Effect::Print() {
+void Nowhere::Print() {
-  PrintF("Effect");
+  PrintF("Nowhere");
 }
 
 
@@ -555,13 +615,22 @@ void TempLocation::Print() {
 }
 
 
+void MoveInstr::Print() {
+  PrintF("Move(");
+  location()->Print();
+  PrintF(", ");
+  value_->Print();
+  PrintF(")\n");
+}
+
+
 void BinaryOpInstr::Print() {
   PrintF("BinaryOp(");
-  loc_->Print();
+  location()->Print();
-  PrintF(", %s, ", Token::Name(op_));
+  PrintF(", %s, ", Token::Name(op()));
-  val0_->Print();
+  value0()->Print();
   PrintF(", ");
-  val1_->Print();
+  value1()->Print();
   PrintF(")\n");
 }
 

src/cfg.h

@@ -43,21 +43,23 @@ class Location;
 // Instructions are described by the following grammar.
 //
 // <Instruction> ::=
-//     BinaryOpInstr <Location> Token::Value <Value> <Value>
+//     MoveInstr <Location> <Value>
-//   | ReturnInstr Effect <Value>
+//   | BinaryOpInstr <Location> Token::Value <Value> <Value>
+//   | ReturnInstr Nowhere <Value>
+//   | PositionInstr <Int>
 //
 // Values are trivial expressions:
 //
 // <Value> ::= Constant | <Location>
 //
 // Locations are storable values ('lvalues').  They can be slots,
-// compiler-generated temporaries, or the special location 'Effect'
+// compiler-generated temporaries, or the special location 'Nowhere'
 // indicating that no value is needed.
 //
 // <Location> ::=
 //     SlotLocation Slot::Type <Index>
 //   | TempLocation
-//   | Effect
+//   | Nowhere
 
 
 // Administrative nodes: There are several types of 'administrative' nodes
@@ -95,8 +97,8 @@ class CfgGlobals BASE_EMBEDDED {
   // The shared global exit node for all exits from the function.
   ExitNode* exit() { return global_exit_; }
 
-  // A singleton effect location.
+  // A singleton.
-  Location* effect_location() { return effect_; }
+  Location* nowhere() { return nowhere_; }
 
 #ifdef DEBUG
   int next_node_number() { return node_counter_++; }
@@ -107,7 +109,7 @@ class CfgGlobals BASE_EMBEDDED {
   static CfgGlobals* top_;
   FunctionLiteral* global_fun_;
   ExitNode* global_exit_;
-  Location* effect_;
+  Location* nowhere_;
 
 #ifdef DEBUG
   // Used to number nodes and temporaries when printing.
@@ -119,6 +121,8 @@ class CfgGlobals BASE_EMBEDDED {
 };
 
 
+class SlotLocation;
+
 // Values represent trivial source expressions: ones with no side effects
 // and that do not require code to be generated.
 class Value : public ZoneObject {
@@ -134,6 +138,9 @@ class Value : public ZoneObject {
   // True if the value is a compiler-generated temporary location.
   virtual bool is_temporary() { return false; }
 
+  // True if the value is a slot location.
+  virtual bool is_slot() { return false; }
+
   // Support for fast-compilation mode:
 
   // Move the value into a register.
@@ -142,6 +149,9 @@ class Value : public ZoneObject {
   // Push the value on the stack.
   virtual void Push(MacroAssembler* masm) = 0;
 
+  // Move the value into a slot location.
+  virtual void MoveToSlot(MacroAssembler* masm, SlotLocation* loc) = 0;
+
 #ifdef DEBUG
   virtual void Print() = 0;
 #endif
@@ -158,6 +168,7 @@ class Constant : public Value {
   // Support for fast-compilation mode.
   void Get(MacroAssembler* masm, Register reg);
   void Push(MacroAssembler* masm);
+  void MoveToSlot(MacroAssembler* masm, SlotLocation* loc);
 
 #ifdef DEBUG
   void Print();
@@ -173,9 +184,9 @@ class Location : public Value {
  public:
   virtual ~Location() {}
 
-  // Static factory function returning the singleton effect location.
+  // Static factory function returning the singleton nowhere location.
-  static Location* Effect() {
+  static Location* Nowhere() {
-    return CfgGlobals::current()->effect_location();
+    return CfgGlobals::current()->nowhere();
   }
 
   // Support for fast-compilation mode:
@@ -191,29 +202,34 @@ class Location : public Value {
   // temporary it was not allocated to the stack.
   virtual void Push(MacroAssembler* masm) = 0;
 
+  // Emit code to move a value into this location.
+  virtual void Move(MacroAssembler* masm, Value* value) = 0;
+
 #ifdef DEBUG
   virtual void Print() = 0;
 #endif
 };
 
 
-// Effect is a special (singleton) location that indicates the value of a
+// Nowhere is a special (singleton) location that indicates the value of a
 // computation is not needed (though its side effects are).
-class Effect : public Location {
+class Nowhere : public Location {
  public:
-  // We should not try to emit code to read Effect.
+  // We should not try to emit code to read Nowhere.
   void Get(MacroAssembler* masm, Register reg) { UNREACHABLE(); }
   void Push(MacroAssembler* masm) { UNREACHABLE(); }
+  void MoveToSlot(MacroAssembler* masm, SlotLocation* loc) { UNREACHABLE(); }
 
-  // Setting Effect is ignored.
+  // Setting Nowhere is ignored.
   void Set(MacroAssembler* masm, Register reg) {}
+  void Move(MacroAssembler* masm, Value* value) {}
 
 #ifdef DEBUG
   void Print();
 #endif
 
  private:
-  Effect() {}
+  Nowhere() {}
 
   friend class CfgGlobals;
 };
@@ -225,14 +241,25 @@ class SlotLocation : public Location {
  public:
   SlotLocation(Slot::Type type, int index) : type_(type), index_(index) {}
 
+  // Cast accessor.
+  static SlotLocation* cast(Value* value) {
+    ASSERT(value->is_slot());
+    return reinterpret_cast<SlotLocation*>(value);
+  }
+
   // Accessors.
   Slot::Type type() { return type_; }
   int index() { return index_; }
 
+  // Predicates.
+  bool is_slot() { return true; }
+
   // Support for fast-compilation mode.
   void Get(MacroAssembler* masm, Register reg);
   void Set(MacroAssembler* masm, Register reg);
   void Push(MacroAssembler* masm);
+  void Move(MacroAssembler* masm, Value* value);
+  void MoveToSlot(MacroAssembler* masm, SlotLocation* loc);
 
 #ifdef DEBUG
   void Print();
@@ -252,21 +279,21 @@ class TempLocation : public Location {
  public:
   // Fast-compilation mode allocation decisions.
   enum Where {
-    NOWHERE,      // Not yet allocated.
+    NOT_ALLOCATED,  // Not yet allocated.
-    ACCUMULATOR,  // Allocated to the dedicated accumulator register.
+    ACCUMULATOR,    // Allocated to the dedicated accumulator register.
-    STACK         //   "   "   "   "  stack.
+    STACK           //   "   "   "   "  stack.
   };
 
-  TempLocation() : where_(NOWHERE) {
+  TempLocation() : where_(NOT_ALLOCATED) {
 #ifdef DEBUG
     number_ = -1;
 #endif
   }
 
   // Cast accessor.
-  static TempLocation* cast(Location* loc) {
+  static TempLocation* cast(Value* value) {
-    ASSERT(loc->is_temporary());
+    ASSERT(value->is_temporary());
-    return reinterpret_cast<TempLocation*>(loc);
+    return reinterpret_cast<TempLocation*>(value);
   }
 
   // Accessors.
@@ -281,6 +308,8 @@ class TempLocation : public Location {
   void Get(MacroAssembler* masm, Register reg);
   void Set(MacroAssembler* masm, Register reg);
   void Push(MacroAssembler* masm);
+  void Move(MacroAssembler* masm, Value* value);
+  void MoveToSlot(MacroAssembler* masm, SlotLocation* loc);
 
 #ifdef DEBUG
   int number() {
@@ -306,16 +335,16 @@ class TempLocation : public Location {
 class Instruction : public ZoneObject {
  public:
   // Every instruction has a location where its result is stored (which may
-  // be Effect, the default).
+  // be Nowhere, the default).
-  Instruction() : loc_(CfgGlobals::current()->effect_location()) {}
+  Instruction() : location_(CfgGlobals::current()->nowhere()) {}
 
-  explicit Instruction(Location* loc) : loc_(loc) {}
+  explicit Instruction(Location* location) : location_(location) {}
 
   virtual ~Instruction() {}
 
   // Accessors.
-  Location* location() { return loc_; }
+  Location* location() { return location_; }
-  void set_location(Location* loc) { loc_ = loc; }
+  void set_location(Location* location) { location_ = location; }
 
   // Support for fast-compilation mode:
 
@@ -332,7 +361,7 @@ class Instruction : public ZoneObject {
 #endif
 
  protected:
-  Location* loc_;
+  Location* location_;
 };
 
 
@@ -360,14 +389,40 @@ class PositionInstr : public Instruction {
 };
 
 
+// Move a value to a location.
+class MoveInstr : public Instruction {
+ public:
+  MoveInstr(Location* loc, Value* value) : Instruction(loc), value_(value) {}
+
+  // Accessors.
+  Value* value() { return value_; }
+
+  // Support for fast-compilation mode.
+  void Compile(MacroAssembler* masm);
+  void FastAllocate(TempLocation* temp);
+
+#ifdef DEBUG
+  void Print();
+#endif
+
+ private:
+  Value* value_;
+};
+
+
 // Perform a (non-short-circuited) binary operation on a pair of values,
 // leaving the result in a location.
 class BinaryOpInstr : public Instruction {
  public:
-  BinaryOpInstr(Location* loc, Token::Value op, Value* val0, Value* val1)
+  BinaryOpInstr(Location* loc, Token::Value op, Value* value0, Value* value1)
-      : Instruction(loc), op_(op), val0_(val0), val1_(val1) {
+      : Instruction(loc), op_(op), value0_(value0), value1_(value1) {
   }
 
+  // Accessors.
+  Token::Value op() { return op_; }
+  Value* value0() { return value0_; }
+  Value* value1() { return value1_; }
+
   // Support for fast-compilation mode.
   void Compile(MacroAssembler* masm);
   void FastAllocate(TempLocation* temp);
@@ -378,8 +433,8 @@ class BinaryOpInstr : public Instruction {
 
  private:
   Token::Value op_;
-  Value* val0_;
+  Value* value0_;
-  Value* val1_;
+  Value* value1_;
 };
 
 
@@ -390,7 +445,6 @@ class BinaryOpInstr : public Instruction {
 // successor is the global exit node for the current function.
 class ReturnInstr : public Instruction {
  public:
-  // Location is always Effect.
   explicit ReturnInstr(Value* value) : value_(value) {}
 
   virtual ~ReturnInstr() {}
@@ -605,6 +659,52 @@ class Cfg : public ZoneObject {
 };
 
 
+// An implementation of a set of locations (currently slot locations).
+class LocationSet BASE_EMBEDDED {
+ public:
+  // Construct an empty location set.
+  LocationSet() : parameters_(0), locals_(0) {}
+
+  // Raw accessors.
+  uintptr_t parameters() { return parameters_; }
+  uintptr_t locals() { return locals_; }
+
+  // Insert an element.
+  void AddElement(SlotLocation* location) {
+    if (location->type() == Slot::PARAMETER) {
+      // Parameter indexes begin with -1 ('this').
+      ASSERT(location->index() < kPointerSize - 1);
+      parameters_ |= (1 << (location->index() + 1));
+    } else {
+      ASSERT(location->type() == Slot::LOCAL);
+      ASSERT(location->index() < kPointerSize);
+      locals_ |= (1 << location->index());
+    }
+  }
+
+  // (Destructively) compute the union with another set.
+  void Union(LocationSet* other) {
+    parameters_ |= other->parameters();
+    locals_ |= other->locals();
+  }
+
+  bool Contains(SlotLocation* location) {
+    if (location->type() == Slot::PARAMETER) {
+      ASSERT(location->index() < kPointerSize - 1);
+      return (parameters_ & (1 << (location->index() + 1)));
+    } else {
+      ASSERT(location->type() == Slot::LOCAL);
+      ASSERT(location->index() < kPointerSize);
+      return (locals_ & (1 << location->index()));
+    }
+  }
+
+ private:
+  uintptr_t parameters_;
+  uintptr_t locals_;
+};
+
+
 // An ExpressionBuilder traverses an expression and returns an open CFG
 // fragment (currently a possibly empty list of instructions represented by
 // a singleton instruction block) and the expression's value.
@@ -612,15 +712,23 @@ class Cfg : public ZoneObject {
 // Failure is to build the CFG is indicated by a NULL CFG.
 class ExpressionBuilder : public AstVisitor {
  public:
-  ExpressionBuilder() : value_(NULL), cfg_(NULL) {}
+  ExpressionBuilder() : value_(NULL), graph_(NULL), destination_(NULL) {}
 
   // Result accessors.
   Value* value() { return value_; }
-  Cfg* cfg() { return cfg_; }
+  Cfg* graph() { return graph_; }
+  LocationSet* assigned_vars() { return &assigned_vars_; }
 
-  void Build(Expression* expr) {
+  // Build the cfg for an expression and remember its value.  The
+  // destination is a 'hint' where the value should go which may be ignored.
+  // NULL is used to indicate no preference.
+  //
+  // Concretely, if the expression needs to generate a temporary for its
+  // value, it should use the passed destination or generate one if NULL.
+  void Build(Expression* expr, Location* destination) {
     value_ = NULL;
-    cfg_ = new Cfg();
+    graph_ = new Cfg();
+    destination_ = destination;
     Visit(expr);
   }
 
@@ -630,8 +738,13 @@ class ExpressionBuilder : public AstVisitor {
 #undef DECLARE_VISIT
 
  private:
+  // State for the visitor.  Output parameters.
   Value* value_;
-  Cfg* cfg_;
+  Cfg* graph_;
+  LocationSet assigned_vars_;
+
+  // Input parameters.
+  Location* destination_;
 };
 
 
@@ -640,9 +753,9 @@ class ExpressionBuilder : public AstVisitor {
 // accumulator.
 class StatementBuilder : public AstVisitor {
  public:
-  StatementBuilder() : cfg_(new Cfg()) {}
+  StatementBuilder() : graph_(new Cfg()) {}
 
-  Cfg* cfg() { return cfg_; }
+  Cfg* graph() { return graph_; }
 
   void VisitStatements(ZoneList<Statement*>* stmts);
 
@@ -652,7 +765,7 @@ class StatementBuilder : public AstVisitor {
 #undef DECLARE_VISIT
 
  private:
-  Cfg* cfg_;
+  Cfg* graph_;
 };
 
 

src/ia32/cfg-ia32.cc

@@ -121,28 +121,33 @@ void PositionInstr::Compile(MacroAssembler* masm) {
 }
 
 
+void MoveInstr::Compile(MacroAssembler* masm) {
+  location()->Move(masm, value());
+}
+
+
 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());
+  ASSERT(!value1()->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()) {
+  if (value0()->is_temporary()) {
     mode = OVERWRITE_LEFT;
-  } else if (val1_->is_temporary()) {
+  } else if (value1()->is_temporary()) {
     mode = OVERWRITE_RIGHT;
   }
 
   // Push both operands and call the specialized stub.
-  if (!val0_->is_on_stack()) {
+  if (!value0()->is_on_stack()) {
-    val0_->Push(masm);
+    value0()->Push(masm);
   }
-  val1_->Push(masm);
+  value1()->Push(masm);
-  GenericBinaryOpStub stub(op_, mode, SMI_CODE_IN_STUB);
+  GenericBinaryOpStub stub(op(), mode, SMI_CODE_IN_STUB);
   __ CallStub(&stub);
-  loc_->Set(masm, eax);
+  location()->Set(masm, eax);
 }
 
 
@@ -181,6 +186,11 @@ static Operand ToOperand(SlotLocation* loc) {
 }
 
 
+void Constant::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  __ mov(ToOperand(loc), Immediate(handle_));
+}
+
+
 void SlotLocation::Get(MacroAssembler* masm, Register reg) {
   __ mov(reg, ToOperand(this));
 }
@@ -196,6 +206,19 @@ void SlotLocation::Push(MacroAssembler* masm) {
 }
 
 
+void SlotLocation::Move(MacroAssembler* masm, Value* value) {
+  // We dispatch to the value because in some cases (temp or constant)
+  // we can use a single instruction.
+  value->MoveToSlot(masm, this);
+}
+
+void SlotLocation::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  // The accumulator is not live across a MoveInstr.
+  __ mov(eax, ToOperand(this));
+  __ mov(ToOperand(loc), eax);
+}
+
+
 void TempLocation::Get(MacroAssembler* masm, Register reg) {
   switch (where_) {
     case ACCUMULATOR:
@@ -204,9 +227,8 @@ void TempLocation::Get(MacroAssembler* masm, Register reg) {
     case STACK:
       __ pop(reg);
       break;
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
@@ -219,9 +241,8 @@ void TempLocation::Set(MacroAssembler* masm, Register reg) {
     case STACK:
       __ push(reg);
       break;
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
@@ -232,9 +253,36 @@ void TempLocation::Push(MacroAssembler* masm) {
       __ push(eax);
       break;
     case STACK:
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
+  }
+}
+
+
+void TempLocation::Move(MacroAssembler* masm, Value* value) {
+  switch (where_) {
+    case ACCUMULATOR:
+      value->Get(masm, eax);
       break;
+    case STACK:
+      value->Push(masm);
+      break;
+    case NOT_ALLOCATED:
+      UNREACHABLE();
+  }
+}
+
+
+void TempLocation::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  switch (where_) {
+    case ACCUMULATOR:
+      __ mov(ToOperand(loc), eax);
+      break;
+    case STACK:
+      __ pop(ToOperand(loc));
+      break;
+    case NOT_ALLOCATED:
+      UNREACHABLE();
   }
 }
 

src/x64/cfg-x64.cc

@@ -131,28 +131,33 @@ void PositionInstr::Compile(MacroAssembler* masm) {
 }
 
 
+void MoveInstr::Compile(MacroAssembler* masm) {
+  location()->Move(masm, value());
+}
+
+
 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());
+  ASSERT(!value1()->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()) {
+  if (value0()->is_temporary()) {
     mode = OVERWRITE_LEFT;
-  } else if (val1_->is_temporary()) {
+  } else if (value1()->is_temporary()) {
     mode = OVERWRITE_RIGHT;
   }
 
   // Push both operands and call the specialized stub.
-  if (!val0_->is_on_stack()) {
+  if (!value0()->is_on_stack()) {
-    val0_->Push(masm);
+    value0()->Push(masm);
   }
-  val1_->Push(masm);
+  value1()->Push(masm);
-  GenericBinaryOpStub stub(op_, mode, SMI_CODE_IN_STUB);
+  GenericBinaryOpStub stub(op(), mode, SMI_CODE_IN_STUB);
   __ CallStub(&stub);
-  loc_->Set(masm, rax);
+  location()->Set(masm, rax);
 }
 
 
@@ -191,6 +196,11 @@ static Operand ToOperand(SlotLocation* loc) {
 }
 
 
+void Constant::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  __ Move(ToOperand(loc), handle_);
+}
+
+
 void SlotLocation::Get(MacroAssembler* masm, Register reg) {
   __ movq(reg, ToOperand(this));
 }
@@ -201,6 +211,19 @@ void SlotLocation::Set(MacroAssembler* masm, Register reg) {
 }
 
 
+void SlotLocation::Move(MacroAssembler* masm, Value* value) {
+  // We dispatch to the value because in some cases (temp or constant) we
+  // can use special instruction sequences.
+  value->MoveToSlot(masm, this);
+}
+
+
+void SlotLocation::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  __ movq(kScratchRegister, ToOperand(this));
+  __ movq(ToOperand(loc), kScratchRegister);
+}
+
+
 void SlotLocation::Push(MacroAssembler* masm) {
   __ push(ToOperand(this));
 }
@@ -214,9 +237,8 @@ void TempLocation::Get(MacroAssembler* masm, Register reg) {
     case STACK:
       __ pop(reg);
       break;
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
@@ -229,9 +251,8 @@ void TempLocation::Set(MacroAssembler* masm, Register reg) {
     case STACK:
       __ push(reg);
       break;
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
-      break;
   }
 }
 
@@ -242,9 +263,36 @@ void TempLocation::Push(MacroAssembler* masm) {
       __ push(rax);
       break;
     case STACK:
-    case NOWHERE:
+    case NOT_ALLOCATED:
       UNREACHABLE();
+  }
+}
+
+
+void TempLocation::Move(MacroAssembler* masm, Value* value) {
+  switch (where_) {
+    case ACCUMULATOR:
+      value->Get(masm, rax);
       break;
+    case STACK:
+      value->Push(masm);
+      break;
+    case NOT_ALLOCATED:
+      UNREACHABLE();
+  }
+}
+
+
+void TempLocation::MoveToSlot(MacroAssembler* masm, SlotLocation* loc) {
+  switch (where_) {
+    case ACCUMULATOR:
+      __ movq(ToOperand(loc), rax);
+      break;
+    case STACK:
+      __ pop(ToOperand(loc));
+      break;
+    case NOT_ALLOCATED:
+      UNREACHABLE();
   }
 }