[Interpreter] Reduce temporary register usage in generated bytecode.

src/interpreter/bytecode-generator.cc

Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 423c438cf2c17910227d08fd4b873294538d2dbb..2fbe93a4a36e6344a199a489ef981506fa570ce4 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -89,13 +89,113 @@ void BytecodeGenerator::ControlScope::PerformCommand(Command command,
 }
 
 
+// Scoped base class for determining where the result of an expression
+// is stored.
+class BytecodeGenerator::ExpressionResultScope {
+ public:
+  explicit ExpressionResultScope(BytecodeGenerator* generator)
+      : generator_(generator),
+        outer_(generator->result_scope()),
+        result_identified_(false) {
+    generator_->set_result_scope(this);
+  }
+
+  virtual ~ExpressionResultScope() {
+    generator_->set_result_scope(outer_);
+    DCHECK_EQ(result_identified(), true);
+  }
+
+  virtual void SetResultInAccumulator() = 0;
+  virtual void SetResultInRegister(Register reg) = 0;
+
+  BytecodeGenerator* generator() const { return generator_; }
+  BytecodeArrayBuilder* builder() const { return generator()->builder(); }
+
+ protected:
+  void set_result_identified() {
+    DCHECK(!result_identified());
+    result_identified_ = true;
+  }
+  bool result_identified() const { return result_identified_; }
+
+ private:
+  BytecodeGenerator* generator_;
+  ExpressionResultScope* outer_;
+  bool result_identified_;
+  DISALLOW_COPY_AND_ASSIGN(ExpressionResultScope);
+};
+
+
+// Scoped class used when the result of the current expression to be
+// evaluated should go into the interpreter's accumulator register.
+class BytecodeGenerator::AccumulatorResultScope final
+    : public ExpressionResultScope {
+ public:
+  explicit AccumulatorResultScope(BytecodeGenerator* generator)
+      : ExpressionResultScope(generator) {}
+  virtual void SetResultInAccumulator() {
+    if (!result_identified()) {
+      // TODO(oth): REVIEW.
+      set_result_identified();
+    }
+  }
+  virtual void SetResultInRegister(Register reg) {
+    builder()->LoadAccumulatorWithRegister(reg);
+    set_result_identified();
+  }
+};
+
+
+// Scoped class used when the result of the current expression is not
+// expected to produce a result.
+class BytecodeGenerator::EffectResultScope final
+    : public ExpressionResultScope {
+ public:
+  explicit EffectResultScope(BytecodeGenerator* generator)
+      : ExpressionResultScope(generator) { set_result_identified(); }
+  virtual void SetResultInAccumulator() {}
+  virtual void SetResultInRegister(Register reg) {}
+};
+
+
+// Scoped class used when the result of the current expression to be
+// evaluated should go into an interpreter register.
+class BytecodeGenerator::RegisterResultScope final
+    : public ExpressionResultScope {
+ public:
+  explicit RegisterResultScope(BytecodeGenerator* generator,
+                               TemporaryRegisterScope* register_scope)
+      : ExpressionResultScope(generator), register_scope_(register_scope) {
+    DCHECK(generator->ResultRegisterIsEmpty());
+  }
+  ~RegisterResultScope() { DCHECK(generator()->ResultRegisterIsEmpty()); }
+
+  virtual void SetResultInAccumulator() {
+    Register allocated = register_scope_->NewRegister();
+    builder()->StoreAccumulatorInRegister(allocated);
+    generator()->SetResultRegister(allocated);
+    set_result_identified();
+  }
+
+  virtual void SetResultInRegister(Register reg) {
+    generator()->SetResultRegister(reg);
+    set_result_identified();
+  }
+
+ private:
+  TemporaryRegisterScope* register_scope_;
+};
+
+
 BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone)
     : builder_(isolate, zone),
       info_(nullptr),
       scope_(nullptr),
       globals_(0, zone),
       control_scope_(nullptr),
-      current_context_(Register::function_context()) {
+      current_context_(Register::function_context()),
+      result_scope_(nullptr),
+      result_register_(Nothing<Register>()) {
   InitializeAstVisitor(isolate, zone);
 }
 
@@ -235,7 +335,7 @@ void BytecodeGenerator::VisitDeclarations(
 
 
 void BytecodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) {
-  Visit(stmt->expression());
+  VisitForAccumulatorValue(stmt->expression());
 }
 
 
@@ -251,7 +351,7 @@ void BytecodeGenerator::VisitIfStatement(IfStatement* stmt) {
 
   BytecodeLabel else_label, end_label;
 
-  Visit(stmt->condition());
+  VisitForAccumulatorValue(stmt->condition());
   builder()->CastAccumulatorToBoolean();
   builder()->JumpIfFalse(&else_label);
   Visit(stmt->then_statement());
@@ -283,7 +383,7 @@ void BytecodeGenerator::VisitBreakStatement(BreakStatement* stmt) {
 
 
 void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
-  Visit(stmt->expression());
+  VisitForAccumulatorValue(stmt->expression());
   builder()->Return();
 }
 
@@ -309,7 +409,7 @@ void BytecodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) {
   builder()->Bind(&body_label);
   Visit(stmt->body());
   builder()->Bind(&condition_label);
-  Visit(stmt->cond());
+  VisitForAccumulatorValue(stmt->cond());
   builder()->JumpIfTrue(&body_label);
   builder()->Bind(&done_label);
 
@@ -327,7 +427,7 @@ void BytecodeGenerator::VisitWhileStatement(WhileStatement* stmt) {
   builder()->Bind(&body_label);
   Visit(stmt->body());
   builder()->Bind(&condition_label);
-  Visit(stmt->cond());
+  VisitForAccumulatorValue(stmt->cond());
   builder()->JumpIfTrue(&body_label);
   builder()->Bind(&done_label);
 
@@ -356,7 +456,7 @@ void BytecodeGenerator::VisitForStatement(ForStatement* stmt) {
   }
   if (stmt->cond()) {
     builder()->Bind(&condition_label);
-    Visit(stmt->cond());
+    VisitForAccumulatorValue(stmt->cond());
     builder()->JumpIfTrue(&body_label);
   } else {
     builder()->Jump(&body_label);
@@ -429,6 +529,7 @@ void BytecodeGenerator::VisitLiteral(Literal* expr) {
   } else {
     builder()->LoadLiteral(value);
   }
+  result_scope()->SetResultInAccumulator();
 }
 
 
@@ -456,15 +557,14 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
                                           FeedbackVectorSlot slot) {
   switch (variable->location()) {
     case VariableLocation::LOCAL: {
-      Register source(variable->index());
-      builder()->LoadAccumulatorWithRegister(source);
+      result_scope()->SetResultInRegister(Register(variable->index()));
       break;
     }
     case VariableLocation::PARAMETER: {
       // The parameter indices are shifted by 1 (receiver is variable
       // index -1 but is parameter index 0 in BytecodeArrayBuilder).
-      Register source(builder()->Parameter(variable->index() + 1));
-      builder()->LoadAccumulatorWithRegister(source);
+      result_scope()->SetResultInRegister(
+          builder()->Parameter(variable->index() + 1));
       break;
     }
     case VariableLocation::GLOBAL: {
@@ -474,6 +574,7 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
       // runtime.
       DCHECK(variable->IsStaticGlobalObjectProperty());
       builder()->LoadGlobal(variable->index());
+      result_scope()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::UNALLOCATED: {
@@ -484,10 +585,12 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
       builder()->StoreAccumulatorInRegister(obj);
       builder()->LoadLiteral(variable->name());
       builder()->LoadNamedProperty(obj, feedback_index(slot), language_mode());
+      result_scope()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::CONTEXT:
     case VariableLocation::LOOKUP:
+    default:

[rmcilroy, 2015/10/09 13:02:56]

Is this required? I'd prefer to avoid default: if possible so that it would
cause a compile-time error if a new VariableLocation enum value is added.

[oth, 2015/10/09 13:27:39]

Acknowledged.

       UNIMPLEMENTED();
   }
 }
@@ -538,6 +641,7 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
     }
     case VariableLocation::CONTEXT:
     case VariableLocation::LOOKUP:
+    default:
       UNIMPLEMENTED();
   }
 }
@@ -545,7 +649,7 @@ void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
 
 void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   DCHECK(expr->target()->IsValidReferenceExpression());
-  TemporaryRegisterScope temporary_register_scope(&builder_);
+  TemporaryRegisterScope temporary_register_scope(builder());
   Register object, key;
 
   // Left-hand side can only be a property, a global or a variable slot.
@@ -558,23 +662,20 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
       // Nothing to do to evaluate variable assignment LHS.
       break;
     case NAMED_PROPERTY:
-      object = temporary_register_scope.NewRegister();
-      key = temporary_register_scope.NewRegister();
-      Visit(property->obj());
-      builder()->StoreAccumulatorInRegister(object);
+      object =
+          VisitForRegisterValue(property->obj(), &temporary_register_scope);
       builder()->LoadLiteral(property->key()->AsLiteral()->AsPropertyName());
+      key = temporary_register_scope.NewRegister();
       builder()->StoreAccumulatorInRegister(key);
       break;
     case KEYED_PROPERTY:
-      object = temporary_register_scope.NewRegister();
-      key = temporary_register_scope.NewRegister();
-      Visit(property->obj());
-      builder()->StoreAccumulatorInRegister(object);
-      Visit(property->key());
-      builder()->StoreAccumulatorInRegister(key);
+      object =
+          VisitForRegisterValue(property->obj(), &temporary_register_scope);
+      key = VisitForRegisterValue(property->key(), &temporary_register_scope);
       break;
     case NAMED_SUPER_PROPERTY:
     case KEYED_SUPER_PROPERTY:
+    default:
       UNIMPLEMENTED();
   }
 
@@ -583,7 +684,8 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   if (expr->is_compound()) {
     UNIMPLEMENTED();
   } else {
-    Visit(expr->value());
+    VisitForAccumulatorValue(expr->value());
+    result_scope()->SetResultInAccumulator();
   }
 
   // Store the value.
@@ -624,11 +726,13 @@ void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) {
     case NAMED_PROPERTY: {
       builder()->LoadLiteral(expr->key()->AsLiteral()->AsPropertyName());
       builder()->LoadNamedProperty(obj, feedback_index(slot), language_mode());
+      result_scope()->SetResultInAccumulator();
       break;
     }
     case KEYED_PROPERTY: {
-      Visit(expr->key());
+      VisitForAccumulatorValue(expr->key());
       builder()->LoadKeyedProperty(obj, feedback_index(slot), language_mode());
+      result_scope()->SetResultInAccumulator();
       break;
     }
     case NAMED_SUPER_PROPERTY:
@@ -639,10 +743,8 @@ void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) {
 
 
 void BytecodeGenerator::VisitProperty(Property* expr) {
-  TemporaryRegisterScope temporary_register_scope(&builder_);
-  Register obj = temporary_register_scope.NewRegister();
-  Visit(expr->obj());
-  builder()->StoreAccumulatorInRegister(obj);
+  TemporaryRegisterScope temporary_register_scope(builder());
+  Register obj = VisitForRegisterValue(expr->obj(), &temporary_register_scope);
   VisitPropertyLoad(obj, expr);
 }
 
@@ -653,8 +755,12 @@ void BytecodeGenerator::VisitCall(Call* expr) {
 
   // Prepare the callee and the receiver to the function call. This depends on
   // the semantics of the underlying call type.
-  TemporaryRegisterScope temporary_register_scope(&builder_);
+  TemporaryRegisterScope temporary_register_scope(builder());
   Register callee = temporary_register_scope.NewRegister();
+
+  // The receiver and arguments need to allocated consecutively for
+  // Call(). Future optimizations could avoid this there are no
+  // arguments or the receiver and arguments are already consecutive.
   Register receiver = temporary_register_scope.NewRegister();
 
   switch (call_type) {
@@ -663,7 +769,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
       if (property->IsSuperAccess()) {
         UNIMPLEMENTED();
       }
-      Visit(property->obj());
+      VisitForAccumulatorValue(property->obj());
       builder()->StoreAccumulatorInRegister(receiver);
       // Perform a property load of the callee.
       VisitPropertyLoad(receiver, property);
@@ -690,7 +796,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
   // registers.
   ZoneList<Expression*>* args = expr->arguments();
   for (int i = 0; i < args->length(); ++i) {
-    Visit(args->at(i));
+    VisitForAccumulatorValue(args->at(i));
     Register arg = temporary_register_scope.NewRegister();
     DCHECK(arg.index() - i == receiver.index() + 1);
     builder()->StoreAccumulatorInRegister(arg);
@@ -699,6 +805,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
   // TODO(rmcilroy): Deal with possible direct eval here?
   // TODO(rmcilroy): Use CallIC to allow call type feedback.
   builder()->Call(callee, receiver, args->length());
+  result_scope()->SetResultInAccumulator();
 }
 
 
@@ -712,14 +819,14 @@ void BytecodeGenerator::VisitCallRuntime(CallRuntime* expr) {
 
   // Evaluate all arguments to the runtime call.
   ZoneList<Expression*>* args = expr->arguments();
-  TemporaryRegisterScope temporary_register_scope(&builder_);
+  TemporaryRegisterScope temporary_register_scope(builder());
   // Ensure we always have a valid first_arg register even if there are no
   // arguments to pass.
   Register first_arg = temporary_register_scope.NewRegister();
   for (int i = 0; i < args->length(); ++i) {
     Register arg =
         (i == 0) ? first_arg : temporary_register_scope.NewRegister();
-    Visit(args->at(i));
+    VisitForAccumulatorValue(args->at(i));
     DCHECK_EQ(arg.index() - i, first_arg.index());
     builder()->StoreAccumulatorInRegister(arg);
   }
@@ -728,24 +835,28 @@ void BytecodeGenerator::VisitCallRuntime(CallRuntime* expr) {
   DCHECK_LE(expr->function()->result_size, 1);
   Runtime::FunctionId function_id = expr->function()->function_id;
   builder()->CallRuntime(function_id, first_arg, args->length());
+  result_scope()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitVoid(UnaryOperation* expr) {
-  Visit(expr->expression());
+  VisitForEffect(expr->expression());
   builder()->LoadUndefined();
+  result_scope()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitTypeOf(UnaryOperation* expr) {
-  Visit(expr->expression());
+  VisitForAccumulatorValue(expr->expression());
   builder()->TypeOf();
+  result_scope()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitNot(UnaryOperation* expr) {
-  Visit(expr->expression());
+  VisitForAccumulatorValue(expr->expression());
   builder()->LogicalNot();
+  result_scope()->SetResultInAccumulator();
 }
 
 
@@ -789,17 +900,11 @@ void BytecodeGenerator::VisitBinaryOperation(BinaryOperation* binop) {
 
 
 void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
-  Token::Value op = expr->op();
-  Expression* left = expr->left();
-  Expression* right = expr->right();
-
-  TemporaryRegisterScope temporary_register_scope(&builder_);
-  Register temporary = temporary_register_scope.NewRegister();
-
-  Visit(left);
-  builder()->StoreAccumulatorInRegister(temporary);
-  Visit(right);
-  builder()->CompareOperation(op, temporary, language_mode_strength());
+  TemporaryRegisterScope temporary_register_scope(builder());
+  Register lhs = VisitForRegisterValue(expr->left(), &temporary_register_scope);
+  VisitForAccumulatorValue(expr->right());
+  builder()->CompareOperation(expr->op(), lhs, language_mode_strength());
+  result_scope()->SetResultInAccumulator();
 }
 
 
@@ -828,20 +933,51 @@ void BytecodeGenerator::VisitSuperPropertyReference(
 
 
 void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* binop) {
-  Token::Value op = binop->op();
-  Expression* left = binop->left();
-  Expression* right = binop->right();
+  TemporaryRegisterScope temporary_register_scope(builder());
+  Register lhs =
+      VisitForRegisterValue(binop->left(), &temporary_register_scope);
+  VisitForAccumulatorValue(binop->right());
+  builder()->BinaryOperation(binop->op(), lhs, language_mode_strength());
+  result_scope()->SetResultInAccumulator();
+}
 
-  TemporaryRegisterScope temporary_register_scope(&builder_);
-  Register temporary = temporary_register_scope.NewRegister();
 
-  Visit(left);
-  builder()->StoreAccumulatorInRegister(temporary);
-  Visit(right);
-  builder()->BinaryOperation(op, temporary, language_mode_strength());
+void BytecodeGenerator::VisitForAccumulatorValue(Expression* expr) {
+  AccumulatorResultScope accumulator_scope(this);
+  Visit(expr);
+}
+
+
+void BytecodeGenerator::VisitForEffect(Expression* node) {
+  EffectResultScope effect_scope(this);
+  Visit(node);
+}
+
+Register BytecodeGenerator::VisitForRegisterValue(
+    Expression* expr, TemporaryRegisterScope* temporary_register_scope) {
+  RegisterResultScope register_scope(this, temporary_register_scope);
+  Visit(expr);
+  return GetResultRegister();
 }
 
 
+void BytecodeGenerator::SetResultRegister(Register reg) {
+  DCHECK(result_register_.IsNothing());
+  result_register_ = Just<Register>(reg);
+}
+
+
+Register BytecodeGenerator::GetResultRegister() {
+  Register reg = result_register_.FromJust();
+  result_register_ = Nothing<Register>();
+  return reg;
+}
+
+
+bool BytecodeGenerator::ResultRegisterIsEmpty() const {
+  return result_register_.IsNothing();
+}
+
 LanguageMode BytecodeGenerator::language_mode() const {
   return info()->language_mode();
 }