[interpreter]: Update BytecodeArrayBuilder register handling.

Modifies the BytecodeArrayBuilder to create register operands which are
negative. This reduces the number of instructions to access registers
by the interpreter and allows us to use positive register operands to
access parameter values.

Adds a Register class to keep register usage typesafe and simplify the
convertion to bytecode operand values.

BUG=v8:4280
LOG=N

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

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

src/interpreter/bytecode-array-builder.cc

@@ -39,8 +39,8 @@ Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() {
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::BinaryOperation(Token::Value binop,
-                                                            int reg) {
-  Output(BytecodeForBinaryOperation(binop), reg);
+                                                            Register reg) {
+  Output(BytecodeForBinaryOperation(binop), reg.ToOperand());
   return *this;
 }
 
@@ -91,15 +91,15 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::LoadFalse() {
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::LoadAccumulatorWithRegister(
-    int reg) {
-  Output(Bytecode::kLdar, reg);
+    Register reg) {
+  Output(Bytecode::kLdar, reg.ToOperand());
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::StoreAccumulatorInRegister(
-    int reg) {
-  Output(Bytecode::kStar, reg);
+    Register reg) {
+  Output(Bytecode::kStar, reg.ToOperand());
   return *this;
 }
 
@@ -112,18 +112,18 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::Return() {
 
 int BytecodeArrayBuilder::BorrowTemporaryRegister() {
   DCHECK_GE(local_register_count_, 0);
-  int temporary_register = temporary_register_next_++;
+  int temporary_reg_index = temporary_register_next_++;
   int count = temporary_register_next_ - local_register_count_;
   if (count > temporary_register_count_) {
     temporary_register_count_ = count;
   }
-  return temporary_register;
+  return temporary_reg_index;
 }
 
 
-void BytecodeArrayBuilder::ReturnTemporaryRegister(int reg) {
-  DCHECK_EQ(reg, temporary_register_next_ - 1);
-  temporary_register_next_ = reg;
+void BytecodeArrayBuilder::ReturnTemporaryRegister(int reg_index) {
+  DCHECK_EQ(reg_index, temporary_register_next_ - 1);
+  temporary_register_next_ = reg_index;
 }
 
 
@@ -136,7 +136,8 @@ bool BytecodeArrayBuilder::OperandIsValid(Bytecode bytecode, int operand_index,
     case OperandType::kImm8:
       return true;
     case OperandType::kReg:
-      return operand_value < temporary_register_next_;
+      return Register::FromOperand(operand_value).index() <
+             temporary_register_next_;
   }
   UNREACHABLE();
   return false;
@@ -200,20 +201,20 @@ Bytecode BytecodeArrayBuilder::BytecodeForBinaryOperation(Token::Value op) {
 
 
 TemporaryRegisterScope::TemporaryRegisterScope(BytecodeArrayBuilder* builder)
-    : builder_(builder), count_(0), register_(-1) {}
+    : builder_(builder), count_(0), last_register_index_(-1) {}
 
 
 TemporaryRegisterScope::~TemporaryRegisterScope() {
   while (count_-- != 0) {
-    builder_->ReturnTemporaryRegister(register_--);
+    builder_->ReturnTemporaryRegister(last_register_index_--);
   }
 }
 
 
-int TemporaryRegisterScope::NewRegister() {
+Register TemporaryRegisterScope::NewRegister() {
   count_++;
-  register_ = builder_->BorrowTemporaryRegister();
-  return register_;
+  last_register_index_ = builder_->BorrowTemporaryRegister();
+  return Register(last_register_index_);
 }
 
 }  // namespace interpreter

src/interpreter/bytecode-array-builder.h

@@ -17,6 +17,8 @@ class Isolate;
 
 namespace interpreter {
 
+class Register;
+
 class BytecodeArrayBuilder {
  public:
   explicit BytecodeArrayBuilder(Isolate* isolate);
@@ -26,7 +28,7 @@ class BytecodeArrayBuilder {
   void set_locals_count(int number_of_locals);
   int locals_count() const;
 
-  // Constant loads to accumulator
+  // Constant loads to accumulator.
   BytecodeArrayBuilder& LoadLiteral(v8::internal::Smi* value);
   BytecodeArrayBuilder& LoadUndefined();
   BytecodeArrayBuilder& LoadNull();
@@ -34,14 +36,14 @@ class BytecodeArrayBuilder {
   BytecodeArrayBuilder& LoadTrue();
   BytecodeArrayBuilder& LoadFalse();
 
-  // Register-accumulator transfers
-  BytecodeArrayBuilder& LoadAccumulatorWithRegister(int reg);
-  BytecodeArrayBuilder& StoreAccumulatorInRegister(int reg);
+  // Register-accumulator transfers.
+  BytecodeArrayBuilder& LoadAccumulatorWithRegister(Register reg);
+  BytecodeArrayBuilder& StoreAccumulatorInRegister(Register reg);
 
-  // Operators
-  BytecodeArrayBuilder& BinaryOperation(Token::Value binop, int reg);
+  // Operators.
+  BytecodeArrayBuilder& BinaryOperation(Token::Value binop, Register reg);
 
-  // Flow Control
+  // Flow Control.
   BytecodeArrayBuilder& Return();
 
  private:
@@ -56,7 +58,7 @@ class BytecodeArrayBuilder {
                       uint8_t operand_value) const;
 
   int BorrowTemporaryRegister();
-  void ReturnTemporaryRegister(int reg);
+  void ReturnTemporaryRegister(int reg_index);
 
   Isolate* isolate_;
   std::vector<uint8_t> bytecodes_;
@@ -70,13 +72,34 @@ class BytecodeArrayBuilder {
   DISALLOW_IMPLICIT_CONSTRUCTORS(BytecodeArrayBuilder);
 };
 
+// An interpreter register which is located in the function's regsiter file
+// in its stack-frame.
+class Register {
+ public:
+  static const int kMaxRegisterIndex = 128;
+
+  explicit Register(int index) : index_(index) {
+    DCHECK_LE(index_, kMaxRegisterIndex);
+  }
+
+  int index() { return index_; }
+  uint8_t ToOperand() { return static_cast<uint8_t>(-index_); }
+  static Register FromOperand(uint8_t operand) { return Register(-operand); }
+
+ private:
+  void* operator new(size_t size);
+  void operator delete(void* p);
+
+  int index_;
+};
+
 // A stack-allocated class than allows the instantiator to allocate
 // temporary registers that are cleaned up when scope is closed.
 class TemporaryRegisterScope {
  public:
   explicit TemporaryRegisterScope(BytecodeArrayBuilder* builder);
   ~TemporaryRegisterScope();
-  int NewRegister();
+  Register NewRegister();
 
  private:
   void* operator new(size_t size);
@@ -84,7 +107,7 @@ class TemporaryRegisterScope {
 
   BytecodeArrayBuilder* builder_;
   int count_;
-  int register_;
+  int last_register_index_;
 
   DISALLOW_COPY_AND_ASSIGN(TemporaryRegisterScope);
 };

src/interpreter/bytecodes.cc

@@ -4,6 +4,8 @@
 
 #include "src/interpreter/bytecodes.h"
 
+#include "src/interpreter/bytecode-array-builder.h"
+
 namespace v8 {
 namespace internal {
 namespace interpreter {
@@ -37,6 +39,20 @@ const char* Bytecodes::ToString(Bytecode bytecode) {
 }
 
 
+// static
+const char* Bytecodes::OperandTypeToString(OperandType operand_type) {
+  switch (operand_type) {
+#define CASE(Name)           \
+  case OperandType::k##Name: \
+    return #Name;
+    OPERAND_TYPE_LIST(CASE)
+#undef CASE
+  }
+  UNREACHABLE();
+  return "";
+}
+
+
 // static
 uint8_t Bytecodes::ToByte(Bytecode bytecode) {
   return static_cast<uint8_t>(bytecode);
@@ -114,7 +130,7 @@ std::ostream& Bytecodes::Decode(std::ostream& os,
         os << "#" << static_cast<int>(operand);
         break;
       case interpreter::OperandType::kReg:
-        os << "r" << static_cast<int>(operand);
+        os << "r" << Register::FromOperand(operand).index();
         break;
       case interpreter::OperandType::kNone:
         UNREACHABLE();
@@ -132,6 +148,11 @@ std::ostream& operator<<(std::ostream& os, const Bytecode& bytecode) {
   return os << Bytecodes::ToString(bytecode);
 }
 
+
+std::ostream& operator<<(std::ostream& os, const OperandType& operand_type) {
+  return os << Bytecodes::OperandTypeToString(operand_type);
+}
+
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8

src/interpreter/bytecodes.h

@@ -78,6 +78,9 @@ class Bytecodes {
   // Returns string representation of |bytecode|.
   static const char* ToString(Bytecode bytecode);
 
+  // Returns string representation of |operand_type|.
+  static const char* OperandTypeToString(OperandType operand_type);
+
   // Returns byte value of bytecode.
   static uint8_t ToByte(Bytecode bytecode);
 
@@ -107,6 +110,7 @@ class Bytecodes {
 };
 
 std::ostream& operator<<(std::ostream& os, const Bytecode& bytecode);
+std::ostream& operator<<(std::ostream& os, const OperandType& operand_type);
 
 }  // namespace interpreter
 }  // namespace internal

test/cctest/cctest.gyp

@@ -87,7 +87,6 @@
         'gay-fixed.cc',
         'gay-precision.cc',
         'gay-shortest.cc',
-        'interpreter/test-bytecode-array-builder.cc',
         'print-extension.cc',
         'profiler-extension.cc',
         'test-accessors.cc',

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

@@ -5,14 +5,21 @@
 #include "src/v8.h"
 
 #include "src/interpreter/bytecode-array-builder.h"
-#include "test/cctest/cctest.h"
+#include "test/unittests/test-utils.h"
 
-using namespace v8::internal;
-using namespace v8::internal::interpreter;
+namespace v8 {
+namespace internal {
+namespace interpreter {
 
-TEST(AllBytecodesGenerated) {
-  InitializedHandleScope handle_scope;
-  BytecodeArrayBuilder builder(handle_scope.main_isolate());
+class BytecodeArrayBuilderTest : public TestWithIsolate {
+ public:
+  BytecodeArrayBuilderTest() {}
+  ~BytecodeArrayBuilderTest() override {}
+};
+
+
+TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
+  BytecodeArrayBuilder builder(isolate());
 
   builder.set_locals_count(1);
   CHECK_EQ(builder.locals_count(), 1);
@@ -27,13 +34,14 @@ TEST(AllBytecodesGenerated) {
       .LoadFalse();
 
   // Emit accumulator transfers.
-  builder.LoadAccumulatorWithRegister(0).StoreAccumulatorInRegister(0);
+  Register reg(0);
+  builder.LoadAccumulatorWithRegister(reg).StoreAccumulatorInRegister(reg);
 
   // Emit binary operators invocations.
-  builder.BinaryOperation(Token::Value::ADD, 0)
-      .BinaryOperation(Token::Value::SUB, 0)
-      .BinaryOperation(Token::Value::MUL, 0)
-      .BinaryOperation(Token::Value::DIV, 0);
+  builder.BinaryOperation(Token::Value::ADD, reg)
+      .BinaryOperation(Token::Value::SUB, reg)
+      .BinaryOperation(Token::Value::MUL, reg)
+      .BinaryOperation(Token::Value::DIV, reg);
 
   // Emit control flow. Return must be the last instruction.
   builder.Return();
@@ -66,11 +74,10 @@ TEST(AllBytecodesGenerated) {
 }
 
 
-TEST(FrameSizesLookGood) {
+TEST_F(BytecodeArrayBuilderTest, FrameSizesLookGood) {
   for (int locals = 0; locals < 5; locals++) {
     for (int temps = 0; temps < 3; temps++) {
-      InitializedHandleScope handle_scope;
-      BytecodeArrayBuilder builder(handle_scope.main_isolate());
+      BytecodeArrayBuilder builder(isolate());
       builder.set_locals_count(locals);
       builder.Return();
 
@@ -87,16 +94,15 @@ TEST(FrameSizesLookGood) {
 }
 
 
-TEST(TemporariesRecycled) {
-  InitializedHandleScope handle_scope;
-  BytecodeArrayBuilder builder(handle_scope.main_isolate());
+TEST_F(BytecodeArrayBuilderTest, TemporariesRecycled) {
+  BytecodeArrayBuilder builder(isolate());
   builder.set_locals_count(0);
   builder.Return();
 
   int first;
   {
     TemporaryRegisterScope temporaries(&builder);
-    first = temporaries.NewRegister();
+    first = temporaries.NewRegister().index();
     temporaries.NewRegister();
     temporaries.NewRegister();
     temporaries.NewRegister();
@@ -105,8 +111,12 @@ TEST(TemporariesRecycled) {
   int second;
   {
     TemporaryRegisterScope temporaries(&builder);
-    second = temporaries.NewRegister();
+    second = temporaries.NewRegister().index();
   }
 
   CHECK_EQ(first, second);
 }
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8

test/unittests/unittests.gyp

@@ -94,6 +94,7 @@
         'compiler/value-numbering-reducer-unittest.cc',
         'compiler/zone-pool-unittest.cc',
         'counters-unittest.cc',
+        'interpreter/bytecode-array-builder-unittest.cc',
         'libplatform/default-platform-unittest.cc',
         'libplatform/task-queue-unittest.cc',
         'libplatform/worker-thread-unittest.cc',