[Interpreter] Reduce temporary register usage in generated bytecode.

This change adds new flavors of Visit() methods for obtaining
expression results:

- VisitForAccumulatorValue() which places result in the accumulator.
- VisitForRegisterValue() which places the result in a register.
- VisitForEffect() which evaluates the expression and discards the result.

The targets of these calls place the expression result with
result_scope()->SetResultInRegister() or
result_scope()->SetResultInAccumulator().

By being smarter about result locations, there's less temporary
register usage. However, we now have a hazard with assignments
in binary expressions that didn't exist before. This change detects and
DCHECK's when a hazard is detected. A follow on CL will address this.

There are consequential changes to test-bytecode-generator.cc and
this change also adds new bytecode macros A(x, n) and THIS(n) for
register file entries for arguments and this.

BUG=v8:4280
LOG=NO

Review URL: https://codereview.chromium.org/1392933002

Cr-Commit-Position: refs/heads/master@{#31445}

src/interpreter/bytecode-array-builder.cc

@@ -22,13 +22,12 @@ BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone)
       local_register_count_(-1),
       context_register_count_(-1),
       temporary_register_count_(0),
-      temporary_register_next_(0) {}
+      free_temporaries_(zone) {}
 
 
 void BytecodeArrayBuilder::set_locals_count(int number_of_locals) {
   local_register_count_ = number_of_locals;
   DCHECK_LE(context_register_count_, 0);
-  temporary_register_next_ = local_register_count_;
 }
 
 
@@ -40,7 +39,6 @@ void BytecodeArrayBuilder::set_parameter_count(int number_of_parameters) {
 void BytecodeArrayBuilder::set_context_count(int number_of_contexts) {
   context_register_count_ = number_of_contexts;
   DCHECK_GE(local_register_count_, 0);
-  temporary_register_next_ = local_register_count_ + context_register_count_;
 }
 
 
@@ -56,12 +54,35 @@ Register BytecodeArrayBuilder::last_context_register() const {
 }
 
 
+Register BytecodeArrayBuilder::first_temporary_register() const {
+  DCHECK_GT(temporary_register_count_, 0);
+  return Register(fixed_register_count());
+}
+
+
+Register BytecodeArrayBuilder::last_temporary_register() const {
+  DCHECK_GT(temporary_register_count_, 0);
+  return Register(fixed_register_count() + temporary_register_count_ - 1);
+}
+
+
 Register BytecodeArrayBuilder::Parameter(int parameter_index) const {
   DCHECK_GE(parameter_index, 0);
   return Register::FromParameterIndex(parameter_index, parameter_count());
 }
 
 
+bool BytecodeArrayBuilder::RegisterIsParameterOrLocal(Register reg) const {
+  return reg.is_parameter() || reg.index() < locals_count();
+}
+
+
+bool BytecodeArrayBuilder::RegisterIsTemporary(Register reg) const {
+  return temporary_register_count_ > 0 && first_temporary_register() <= reg &&
+         reg <= last_temporary_register();
+}
+
+
 Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() {
   DCHECK_EQ(bytecode_generated_, false);
 
@@ -607,6 +628,7 @@ void BytecodeArrayBuilder::EnsureReturn() {
   }
 }
 
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::Call(Register callable,
                                                  Register receiver,
                                                  size_t arg_count) {
@@ -661,18 +683,74 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
 
 
 int BytecodeArrayBuilder::BorrowTemporaryRegister() {
-  int temporary_reg_index = temporary_register_next_++;
-  int count = temporary_register_next_ - fixed_register_count();
-  if (count > temporary_register_count_) {
-    temporary_register_count_ = count;
+  if (free_temporaries_.empty()) {
+    temporary_register_count_ += 1;
+    return last_temporary_register().index();
+  } else {
+    auto pos = free_temporaries_.begin();
+    int retval = *pos;
+    free_temporaries_.erase(pos);
+    return retval;
   }
-  return temporary_reg_index;
+}
+
+
+void BytecodeArrayBuilder::BorrowConsecutiveTemporaryRegister(int reg_index) {
+  DCHECK(free_temporaries_.find(reg_index) != free_temporaries_.end());
+  free_temporaries_.erase(reg_index);
 }
 
 
 void BytecodeArrayBuilder::ReturnTemporaryRegister(int reg_index) {
-  DCHECK_EQ(reg_index, temporary_register_next_ - 1);
-  temporary_register_next_ = reg_index;
+  DCHECK(free_temporaries_.find(reg_index) == free_temporaries_.end());
+  free_temporaries_.insert(reg_index);
+}
+
+
+int BytecodeArrayBuilder::PrepareForConsecutiveTemporaryRegisters(
+    size_t count) {
+  if (count == 0) {
+    return -1;
+  }
+
+  // Search within existing temporaries for a run.
+  auto start = free_temporaries_.begin();
+  size_t run_length = 0;
+  for (auto run_end = start; run_end != free_temporaries_.end(); run_end++) {
+    if (*run_end != *start + static_cast<int>(run_length)) {
+      start = run_end;
+      run_length = 0;
+    }
+    if (++run_length == count) {
+      return *start;
+    }
+  }
+
+  // Continue run if possible across existing last temporary.
+  if (temporary_register_count_ > 0 &&
+      (start == free_temporaries_.end() ||
+       *start + static_cast<int>(run_length) !=
+           last_temporary_register().index() + 1)) {
+    run_length = 0;
+  }
+
+  // Ensure enough registers for run.
+  while (run_length++ < count) {
+    temporary_register_count_++;
+    free_temporaries_.insert(last_temporary_register().index());
+  }
+  return last_temporary_register().index() - static_cast<int>(count) + 1;
+}
+
+
+bool BytecodeArrayBuilder::TemporaryRegisterIsLive(Register reg) const {
+  if (temporary_register_count_ > 0) {
+    DCHECK(reg.index() >= first_temporary_register().index() &&
+           reg.index() <= last_temporary_register().index());
+    return free_temporaries_.find(reg.index()) == free_temporaries_.end();
+  } else {
+    return false;
+  }
 }
 
 
@@ -693,10 +771,12 @@ bool BytecodeArrayBuilder::OperandIsValid(Bytecode bytecode, int operand_index,
       if (reg.is_function_context() || reg.is_function_closure()) {
         return true;
       } else if (reg.is_parameter()) {
-        int parameter_index = reg.ToParameterIndex(parameter_count());
-        return parameter_index >= 0 && parameter_index < parameter_count();
+        int parameter_index = reg.ToParameterIndex(parameter_count_);
+        return parameter_index >= 0 && parameter_index < parameter_count_;
+      } else if (reg.index() < fixed_register_count()) {
+        return true;
       } else {
-        return (reg.index() >= 0 && reg.index() < temporary_register_next_);
+        return TemporaryRegisterIsLive(reg);
       }
     }
   }
@@ -880,20 +960,43 @@ bool BytecodeArrayBuilder::FitsInIdx16Operand(int value) {
 
 
 TemporaryRegisterScope::TemporaryRegisterScope(BytecodeArrayBuilder* builder)
-    : builder_(builder), count_(0), last_register_index_(-1) {}
+    : builder_(builder),
+      allocated_(builder->zone()),
+      next_consecutive_register_(-1),
+      next_consecutive_count_(-1) {}
 
 
 TemporaryRegisterScope::~TemporaryRegisterScope() {
-  while (count_-- != 0) {
-    builder_->ReturnTemporaryRegister(last_register_index_--);
+  for (auto i = allocated_.rbegin(); i != allocated_.rend(); i++) {
+    builder_->ReturnTemporaryRegister(*i);
   }
+  allocated_.clear();
 }
 
 
 Register TemporaryRegisterScope::NewRegister() {
-  count_++;
-  last_register_index_ = builder_->BorrowTemporaryRegister();
-  return Register(last_register_index_);
+  int allocated = builder_->BorrowTemporaryRegister();
+  allocated_.push_back(allocated);
+  return Register(allocated);
+}
+
+
+void TemporaryRegisterScope::PrepareForConsecutiveAllocations(size_t count) {
+  if (static_cast<int>(count) > next_consecutive_count_) {
+    next_consecutive_register_ =
+        builder_->PrepareForConsecutiveTemporaryRegisters(count);
+    next_consecutive_count_ = static_cast<int>(count);
+  }
+}
+
+
+Register TemporaryRegisterScope::NextConsecutiveRegister() {
+  DCHECK_GE(next_consecutive_register_, 0);
+  DCHECK_GT(next_consecutive_count_, 0);
+  builder_->BorrowConsecutiveTemporaryRegister(next_consecutive_register_);
+  allocated_.push_back(next_consecutive_register_);
+  next_consecutive_count_--;
+  return Register(next_consecutive_register_++);
 }
 
 }  // namespace interpreter

src/interpreter/bytecode-array-builder.h

@@ -28,14 +28,14 @@ class BytecodeArrayBuilder {
   BytecodeArrayBuilder(Isolate* isolate, Zone* zone);
   Handle<BytecodeArray> ToBytecodeArray();
 
-  // Set number of parameters expected by function.
+  // Set the number of parameters expected by function.
   void set_parameter_count(int number_of_params);
   int parameter_count() const {
     DCHECK_GE(parameter_count_, 0);
     return parameter_count_;
   }
 
-  // Set number of locals required for bytecode array.
+  // Set the number of locals required for bytecode array.
   void set_locals_count(int number_of_locals);
   int locals_count() const {
     DCHECK_GE(local_register_count_, 0);
@@ -57,7 +57,14 @@ class BytecodeArrayBuilder {
 
   Register Parameter(int parameter_index) const;
 
-  // Constant loads to the accumulator.
+  // Return true if the register |reg| represents a parameter or a
+  // local.
+  bool RegisterIsParameterOrLocal(Register reg) const;
+
+  // Return true if the register |reg| represents a temporary register.
+  bool RegisterIsTemporary(Register reg) const;
+
+  // Constant loads to accumulator.
   BytecodeArrayBuilder& LoadLiteral(v8::internal::Smi* value);
   BytecodeArrayBuilder& LoadLiteral(Handle<Object> object);
   BytecodeArrayBuilder& LoadUndefined();
@@ -66,7 +73,7 @@ class BytecodeArrayBuilder {
   BytecodeArrayBuilder& LoadTrue();
   BytecodeArrayBuilder& LoadFalse();
 
-  // Global loads to accumulator and stores from the accumulator.
+  // Global loads to the accumulator and stores from the accumulator.
   BytecodeArrayBuilder& LoadGlobal(int slot_index);
   BytecodeArrayBuilder& StoreGlobal(int slot_index, LanguageMode language_mode);
 
@@ -140,7 +147,7 @@ class BytecodeArrayBuilder {
   BytecodeArrayBuilder& CompareOperation(Token::Value op, Register reg,
                                          Strength strength);
 
-  // Casts
+  // Casts.
   BytecodeArrayBuilder& CastAccumulatorToBoolean();
   BytecodeArrayBuilder& CastAccumulatorToName();
 
@@ -207,9 +214,14 @@ class BytecodeArrayBuilder {
 
   size_t GetConstantPoolEntry(Handle<Object> object);
 
-  // Scope helpers used by TemporaryRegisterScope
   int BorrowTemporaryRegister();
   void ReturnTemporaryRegister(int reg_index);
+  int PrepareForConsecutiveTemporaryRegisters(size_t count);
+  void BorrowConsecutiveTemporaryRegister(int reg_index);
+  bool TemporaryRegisterIsLive(Register reg) const;
+
+  Register first_temporary_register() const;
+  Register last_temporary_register() const;
 
   Isolate* isolate_;
   Zone* zone_;
@@ -226,10 +238,11 @@ class BytecodeArrayBuilder {
   int local_register_count_;
   int context_register_count_;
   int temporary_register_count_;
-  int temporary_register_next_;
+
+  ZoneSet<int> free_temporaries_;
 
   friend class TemporaryRegisterScope;
-  DISALLOW_IMPLICIT_CONSTRUCTORS(BytecodeArrayBuilder);
+  DISALLOW_COPY_AND_ASSIGN(BytecodeArrayBuilder);
 };
 
 
@@ -273,19 +286,26 @@ class BytecodeLabel final {
 
 // A stack-allocated class than allows the instantiator to allocate
 // temporary registers that are cleaned up when scope is closed.
+// TODO(oth): Deprecate TemporaryRegisterScope use. Code should be
+// using result scopes as far as possible.
 class TemporaryRegisterScope {
  public:
   explicit TemporaryRegisterScope(BytecodeArrayBuilder* builder);
   ~TemporaryRegisterScope();
   Register NewRegister();
 
+  void PrepareForConsecutiveAllocations(size_t count);
+  Register NextConsecutiveRegister();
+
  private:
   void* operator new(size_t size);
   void operator delete(void* p);
 
   BytecodeArrayBuilder* builder_;
-  int count_;
-  int last_register_index_;
+  const TemporaryRegisterScope* outer_;
+  ZoneVector<int> allocated_;
+  int next_consecutive_register_;
+  int next_consecutive_count_;
 
   DISALLOW_COPY_AND_ASSIGN(TemporaryRegisterScope);
 };

src/interpreter/bytecode-generator.h

@@ -31,14 +31,17 @@ class BytecodeGenerator : public AstVisitor {
   class ContextScope;
   class ControlScope;
   class ControlScopeForIteration;
+  class ExpressionResultScope;
+  class EffectResultScope;
+  class AccumulatorResultScope;
+  class RegisterResultScope;
 
   void MakeBytecodeBody();
   Register NextContextRegister() const;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
 
-  Register VisitArguments(ZoneList<Expression*>* arguments,
-                          TemporaryRegisterScope* caller_scope);
+  Register VisitArguments(ZoneList<Expression*>* arguments);
   void VisitArithmeticExpression(BinaryOperation* binop);
   void VisitCommaExpression(BinaryOperation* binop);
   void VisitLogicalOrExpression(BinaryOperation* binop);
@@ -61,6 +64,20 @@ class BytecodeGenerator : public AstVisitor {
   void VisitTypeOf(UnaryOperation* expr);
   void VisitNot(UnaryOperation* expr);
 
+  // Visitors for obtaining expression result in the accumulator, in a
+  // register, or just getting the effect.
+  void VisitForAccumulatorValue(Expression* expression);
+  MUST_USE_RESULT Register VisitForRegisterValue(Expression* expression);
+  void VisitForEffect(Expression* node);
+
+  // Methods marking the start and end of binary expressions.
+  void PrepareForBinaryExpression();
+  void CompleteBinaryExpression();
+
+  // Methods for tracking and remapping register.
+  void RecordStoreToRegister(Register reg);
+  Register LoadFromAliasedRegister(Register reg);
+
   inline BytecodeArrayBuilder* builder() { return &builder_; }
 
   inline Isolate* isolate() const { return isolate_; }
@@ -79,6 +96,10 @@ class BytecodeGenerator : public AstVisitor {
   inline void set_execution_context(ContextScope* context) {
     execution_context_ = context;
   }
+  inline void set_execution_result(ExpressionResultScope* execution_result) {
+    execution_result_ = execution_result;
+  }
+  ExpressionResultScope* execution_result() const { return execution_result_; }
 
   ZoneVector<Handle<Object>>* globals() { return &globals_; }
   inline LanguageMode language_mode() const;
@@ -93,6 +114,10 @@ class BytecodeGenerator : public AstVisitor {
   ZoneVector<Handle<Object>> globals_;
   ControlScope* execution_control_;
   ContextScope* execution_context_;
+  ExpressionResultScope* execution_result_;
+
+  int binary_expression_depth_;
+  ZoneSet<int> binary_expression_hazard_set_;
 };
 
 }  // namespace interpreter

src/interpreter/bytecodes.h

@@ -211,8 +211,18 @@ class Register {
                             Register reg4 = Register(),
                             Register reg5 = Register());
 
-  bool operator==(const Register& o) const { return o.index() == index(); }
-  bool operator!=(const Register& o) const { return o.index() != index(); }
+  bool operator==(const Register& other) const {
+    return index() == other.index();
+  }
+  bool operator!=(const Register& other) const {
+    return index() != other.index();
+  }
+  bool operator<(const Register& other) const {
+    return index() < other.index();
+  }
+  bool operator<=(const Register& other) const {
+    return index() <= other.index();
+  }
 
  private:
   static const int kIllegalIndex = kMaxInt;

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -178,12 +178,12 @@ TEST_F(BytecodeArrayBuilderTest, FrameSizesLookGood) {
         builder.set_parameter_count(0);
         builder.set_locals_count(locals);
         builder.set_context_count(contexts);
-        builder.Return();
 
         TemporaryRegisterScope temporaries(&builder);
         for (int i = 0; i < temps; i++) {
-          temporaries.NewRegister();
+          builder.StoreAccumulatorInRegister(temporaries.NewRegister());
         }
+        builder.Return();
 
         Handle<BytecodeArray> the_array = builder.ToBytecodeArray();
         int total_registers = locals + contexts + temps;
@@ -247,6 +247,32 @@ TEST_F(BytecodeArrayBuilderTest, Parameters) {
 }
 
 
+TEST_F(BytecodeArrayBuilderTest, RegisterType) {
+  BytecodeArrayBuilder builder(isolate(), zone());
+  builder.set_parameter_count(10);
+  builder.set_locals_count(3);
+  builder.set_context_count(0);
+
+  TemporaryRegisterScope temporary_register_scope(&builder);
+  Register temp0 = temporary_register_scope.NewRegister();
+  Register param0(builder.Parameter(0));
+  Register param9(builder.Parameter(9));
+  Register temp1 = temporary_register_scope.NewRegister();
+  Register reg0(0);
+  Register reg1(1);
+  Register reg2(2);
+  Register temp2 = temporary_register_scope.NewRegister();
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(temp0), false);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(temp1), false);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(temp2), false);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(param0), true);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(param9), true);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(reg0), true);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(reg1), true);
+  CHECK_EQ(builder.RegisterIsParameterOrLocal(reg2), true);
+}
+
+
 TEST_F(BytecodeArrayBuilderTest, Constants) {
   BytecodeArrayBuilder builder(isolate(), zone());
   builder.set_parameter_count(0);