[interpreter] Streamline bytecode array writing.

Simplify bytecode array writing and remove some now unused bytecode traits
definitions.

BUG=v8:4280
LOG=N

Review-Url: https://codereview.chromium.org/2100793003
Cr-Commit-Position: refs/heads/master@{#37321}

src/interpreter/bytecode-array-writer.cc

@@ -13,6 +13,9 @@ namespace v8 {
 namespace internal {
 namespace interpreter {
 
+STATIC_CONST_MEMBER_DEFINITION const size_t
+    BytecodeArrayWriter::kMaxSizeOfPackedBytecode;
+
 BytecodeArrayWriter::BytecodeArrayWriter(
     Isolate* isolate, Zone* zone, ConstantArrayBuilder* constant_array_builder)
     : isolate_(isolate),
@@ -132,7 +135,8 @@ OperandScale GetOperandScale(const BytecodeNode* const node) {
   const OperandTypeInfo* operand_type_infos =
       Bytecodes::GetOperandTypeInfos(node->bytecode());
   OperandScale operand_scale = OperandScale::kSingle;
-  for (int i = 0; i < node->operand_count(); ++i) {
+  int operand_count = node->operand_count();
+  for (int i = 0; i < operand_count; ++i) {
     switch (operand_type_infos[i]) {
       case OperandTypeInfo::kScalableSignedByte: {
         uint32_t operand = node->operand(i);
@@ -162,56 +166,58 @@ OperandScale GetOperandScale(const BytecodeNode* const node) {
 void BytecodeArrayWriter::EmitBytecode(const BytecodeNode* const node) {
   DCHECK_NE(node->bytecode(), Bytecode::kIllegal);
 
+  uint8_t buffer[kMaxSizeOfPackedBytecode];
+  uint8_t* buffer_limit = buffer;
+
   OperandScale operand_scale = GetOperandScale(node);
   if (operand_scale != OperandScale::kSingle) {
     Bytecode prefix = Bytecodes::OperandScaleToPrefixBytecode(operand_scale);
-    bytecodes()->push_back(Bytecodes::ToByte(prefix));
+    *buffer_limit++ = Bytecodes::ToByte(prefix);
   }
 
   Bytecode bytecode = node->bytecode();
-  bytecodes()->push_back(Bytecodes::ToByte(bytecode));
+  *buffer_limit++ = Bytecodes::ToByte(bytecode);
 
-  int register_operand_bitmap = Bytecodes::GetRegisterOperandBitmap(bytecode);
   const uint32_t* const operands = node->operands();
-  const OperandSize* operand_sizes =
-      Bytecodes::GetOperandSizes(bytecode, operand_scale);
   const OperandType* operand_types = Bytecodes::GetOperandTypes(bytecode);
-  for (int i = 0; operand_types[i] != OperandType::kNone; ++i) {
-    OperandType operand_type = operand_types[i];
-    switch (operand_sizes[i]) {
+  const int operand_count = Bytecodes::NumberOfOperands(bytecode);
+  for (int i = 0; i < operand_count; ++i) {
+    OperandSize operand_size =
+        Bytecodes::SizeOfOperand(operand_types[i], operand_scale);
+    switch (operand_size) {
       case OperandSize::kNone:
         UNREACHABLE();
         break;
       case OperandSize::kByte:
-        bytecodes()->push_back(static_cast<uint8_t>(operands[i]));
+        *buffer_limit++ = static_cast<uint8_t>(operands[i]);
         break;
       case OperandSize::kShort: {
-        uint8_t operand_bytes[2];
-        WriteUnalignedUInt16(operand_bytes, operands[i]);
-        bytecodes()->insert(bytecodes()->end(), operand_bytes,
-                            operand_bytes + 2);
+        WriteUnalignedUInt16(buffer_limit, operands[i]);
+        buffer_limit += 2;
         break;
       }
       case OperandSize::kQuad: {
-        uint8_t operand_bytes[4];
-        WriteUnalignedUInt32(operand_bytes, operands[i]);
-        bytecodes()->insert(bytecodes()->end(), operand_bytes,
-                            operand_bytes + 4);
+        WriteUnalignedUInt32(buffer_limit, operands[i]);
+        buffer_limit += 4;
         break;
       }
     }
 
-    if ((register_operand_bitmap >> i) & 1) {
-      int count;
-      if (operand_types[i + 1] == OperandType::kRegCount) {
-        count = static_cast<int>(operands[i + 1]);
-      } else {
-        count = Bytecodes::GetNumberOfRegistersRepresentedBy(operand_type);
-      }
-      Register reg = Register::FromOperand(static_cast<int32_t>(operands[i]));
-      max_register_count_ = std::max(max_register_count_, reg.index() + count);
+    int count = Bytecodes::GetNumberOfRegistersRepresentedBy(operand_types[i]);
+    if (count == 0) {
+      continue;
     }
+    // NB operand_types is terminated by OperandType::kNone so
+    // operand_types[i + 1] is valid whilst i < operand_count.
+    if (operand_types[i + 1] == OperandType::kRegCount) {
+      count = static_cast<int>(operands[i]);
+    }
+    Register reg = Register::FromOperand(static_cast<int32_t>(operands[i]));
+    max_register_count_ = std::max(max_register_count_, reg.index() + count);
   }
+
+  DCHECK_LE(buffer_limit, buffer + sizeof(buffer));
+  bytecodes()->insert(bytecodes()->end(), buffer, buffer_limit);
 }
 
 // static

src/interpreter/bytecode-array-writer.h

@@ -36,6 +36,13 @@ class BytecodeArrayWriter final : public BytecodePipelineStage {
       Handle<FixedArray> handler_table) override;
 
  private:
+  // Maximum sized packed bytecode is comprised of a prefix bytecode,
+  // plus the actual bytecode, plus the maximum number of operands times
+  // the maximum operand size.
+  static const size_t kMaxSizeOfPackedBytecode =
+      2 * sizeof(Bytecode) +
+      Bytecodes::kMaxOperands * static_cast<size_t>(OperandSize::kLast);
+
   // Constants that act as placeholders for jump operands to be
   // patched. These have operand sizes that match the sizes of
   // reserved constant pool entries.

src/interpreter/bytecode-pipeline.h

@@ -183,10 +183,9 @@ class BytecodeNode final : ZoneObject {
 
  private:
   static const int kInvalidPosition = kMinInt;
-  static const size_t kMaxOperands = 4;
 
   Bytecode bytecode_;
-  uint32_t operands_[kMaxOperands];
+  uint32_t operands_[Bytecodes::kMaxOperands];
   BytecodeSourceInfo source_info_;
 };
 

src/interpreter/bytecode-register-optimizer.cc

@@ -504,35 +504,32 @@ void BytecodeRegisterOptimizer::PrepareRegisterOperands(
   // For each output register about to be clobbered, materialize an
   // equivalent if it exists. Put each register in it's own equivalence set.
   //
-  int register_operand_bitmap =
-      Bytecodes::GetRegisterOperandBitmap(node->bytecode());
+  const uint32_t* operands = node->operands();
+  int operand_count = node->operand_count();
   const OperandType* operand_types =
       Bytecodes::GetOperandTypes(node->bytecode());
-  uint32_t* operands = node->operands();
-  for (int i = 0; register_operand_bitmap != 0;
-       ++i, register_operand_bitmap >>= 1) {
-    if ((register_operand_bitmap & 1) == 0) {
-      continue;
-    }
-    OperandType operand_type = operand_types[i];
-    int count = 0;
+  for (int i = 0; i < operand_count; ++i) {
+    int count;
+    // operand_types is terminated by OperandType::kNone so this does not
+    // go out of bounds.
     if (operand_types[i + 1] == OperandType::kRegCount) {
       count = static_cast<int>(operands[i + 1]);
-      if (count == 0) {
-        continue;
-      }
     } else {
-      count = Bytecodes::GetNumberOfRegistersRepresentedBy(operand_type);
+      count = Bytecodes::GetNumberOfRegistersRepresentedBy(operand_types[i]);
+    }
+
+    if (count == 0) {
+      continue;
     }
 
     Register reg = Register::FromOperand(static_cast<int32_t>(operands[i]));
-    if (Bytecodes::IsRegisterInputOperandType(operand_type)) {
+    if (Bytecodes::IsRegisterInputOperandType(operand_types[i])) {
       if (count == 1) {
         PrepareRegisterInputOperand(node, reg, i);
       } else if (count > 1) {
         PrepareRegisterRangeInputOperand(reg, count);
       }
-    } else if (Bytecodes::IsRegisterOutputOperandType(operand_type)) {
+    } else if (Bytecodes::IsRegisterOutputOperandType(operand_types[i])) {
       PrepareRegisterRangeOutputOperand(reg, count);
     }
   }

src/interpreter/bytecode-traits.h

@@ -100,25 +100,6 @@ struct BytecodeTraits<accumulator_use, operand_0, operand_1, operand_2,
     return operand_type_infos;
   }
 
-  static const OperandSize* GetOperandSizes(OperandScale operand_scale) {
-    switch (operand_scale) {
-#define CASE(Name, _)                                                  \
-  case OperandScale::k##Name: {                                        \
-    static const OperandSize kOperandSizes[] = {                       \
-        OperandScaler<operand_0, OperandScale::k##Name>::kOperandSize, \
-        OperandScaler<operand_1, OperandScale::k##Name>::kOperandSize, \
-        OperandScaler<operand_2, OperandScale::k##Name>::kOperandSize, \
-        OperandScaler<operand_3, OperandScale::k##Name>::kOperandSize, \
-    };                                                                 \
-    return kOperandSizes;                                              \
-  }
-      OPERAND_SCALE_LIST(CASE)
-#undef CASE
-    }
-    UNREACHABLE();
-    return nullptr;
-  }
-
   template <OperandType ot>
   static inline bool HasAnyOperandsOfType() {
     return operand_0 == ot || operand_1 == ot || operand_2 == ot ||
@@ -139,11 +120,6 @@ struct BytecodeTraits<accumulator_use, operand_0, operand_1, operand_2,
       RegisterOperandTraits<operand_1>::kIsRegisterOperand +
       RegisterOperandTraits<operand_2>::kIsRegisterOperand +
       RegisterOperandTraits<operand_3>::kIsRegisterOperand;
-  static const int kRegisterOperandBitmap =
-      RegisterOperandTraits<operand_0>::kIsRegisterOperand +
-      (RegisterOperandTraits<operand_1>::kIsRegisterOperand << 1) +
-      (RegisterOperandTraits<operand_2>::kIsRegisterOperand << 2) +
-      (RegisterOperandTraits<operand_3>::kIsRegisterOperand << 3);
 };
 
 template <AccumulatorUse accumulator_use, OperandType operand_0,
@@ -163,24 +139,6 @@ struct BytecodeTraits<accumulator_use, operand_0, operand_1, operand_2> {
     return operand_type_infos;
   }
 
-  static const OperandSize* GetOperandSizes(OperandScale operand_scale) {
-    switch (operand_scale) {
-#define CASE(Name, _)                                                  \
-  case OperandScale::k##Name: {                                        \
-    static const OperandSize kOperandSizes[] = {                       \
-        OperandScaler<operand_0, OperandScale::k##Name>::kOperandSize, \
-        OperandScaler<operand_1, OperandScale::k##Name>::kOperandSize, \
-        OperandScaler<operand_2, OperandScale::k##Name>::kOperandSize, \
-    };                                                                 \
-    return kOperandSizes;                                              \
-  }
-      OPERAND_SCALE_LIST(CASE)
-#undef CASE
-    }
-    UNREACHABLE();
-    return nullptr;
-  }
-
   template <OperandType ot>
   static inline bool HasAnyOperandsOfType() {
     return operand_0 == ot || operand_1 == ot || operand_2 == ot;
@@ -198,10 +156,6 @@ struct BytecodeTraits<accumulator_use, operand_0, operand_1, operand_2> {
       RegisterOperandTraits<operand_0>::kIsRegisterOperand +
       RegisterOperandTraits<operand_1>::kIsRegisterOperand +
       RegisterOperandTraits<operand_2>::kIsRegisterOperand;
-  static const int kRegisterOperandBitmap =
-      RegisterOperandTraits<operand_0>::kIsRegisterOperand +
-      (RegisterOperandTraits<operand_1>::kIsRegisterOperand << 1) +
-      (RegisterOperandTraits<operand_2>::kIsRegisterOperand << 2);
 };
 
 template <AccumulatorUse accumulator_use, OperandType operand_0,
@@ -220,23 +174,6 @@ struct BytecodeTraits<accumulator_use, operand_0, operand_1> {
     return operand_type_infos;
   }
 
-  static const OperandSize* GetOperandSizes(OperandScale operand_scale) {
-    switch (operand_scale) {
-#define CASE(Name, _)                                                  \
-  case OperandScale::k##Name: {                                        \
-    static const OperandSize kOperandSizes[] = {                       \
-        OperandScaler<operand_0, OperandScale::k##Name>::kOperandSize, \
-        OperandScaler<operand_1, OperandScale::k##Name>::kOperandSize, \
-    };                                                                 \
-    return kOperandSizes;                                              \
-  }
-      OPERAND_SCALE_LIST(CASE)
-#undef CASE
-    }
-    UNREACHABLE();
-    return nullptr;
-  }
-
   template <OperandType ot>
   static inline bool HasAnyOperandsOfType() {
     return operand_0 == ot || operand_1 == ot;
@@ -252,9 +189,6 @@ struct BytecodeTraits<accumulator_use, operand_0, operand_1> {
   static const int kRegisterOperandCount =
       RegisterOperandTraits<operand_0>::kIsRegisterOperand +
       RegisterOperandTraits<operand_1>::kIsRegisterOperand;
-  static const int kRegisterOperandBitmap =
-      RegisterOperandTraits<operand_0>::kIsRegisterOperand +
-      (RegisterOperandTraits<operand_1>::kIsRegisterOperand << 1);
 };
 
 template <AccumulatorUse accumulator_use, OperandType operand_0>
@@ -270,22 +204,6 @@ struct BytecodeTraits<accumulator_use, operand_0> {
     return operand_type_infos;
   }
 
-  static const OperandSize* GetOperandSizes(OperandScale operand_scale) {
-    switch (operand_scale) {
-#define CASE(Name, _)                                                  \
-  case OperandScale::k##Name: {                                        \
-    static const OperandSize kOperandSizes[] = {                       \
-        OperandScaler<operand_0, OperandScale::k##Name>::kOperandSize, \
-    };                                                                 \
-    return kOperandSizes;                                              \
-  }
-      OPERAND_SCALE_LIST(CASE)
-#undef CASE
-    }
-    UNREACHABLE();
-    return nullptr;
-  }
-
   template <OperandType ot>
   static inline bool HasAnyOperandsOfType() {
     return operand_0 == ot;
@@ -299,8 +217,6 @@ struct BytecodeTraits<accumulator_use, operand_0> {
   static const int kOperandCount = 1;
   static const int kRegisterOperandCount =
       RegisterOperandTraits<operand_0>::kIsRegisterOperand;
-  static const int kRegisterOperandBitmap =
-      RegisterOperandTraits<operand_0>::kIsRegisterOperand;
 };
 
 template <AccumulatorUse accumulator_use>
@@ -316,10 +232,6 @@ struct BytecodeTraits<accumulator_use> {
     return operand_type_infos;
   }
 
-  static const OperandSize* GetOperandSizes(OperandScale operand_scale) {
-    return nullptr;
-  }
-
   template <OperandType ot>
   static inline bool HasAnyOperandsOfType() {
     return false;
@@ -330,7 +242,6 @@ struct BytecodeTraits<accumulator_use> {
   static const AccumulatorUse kAccumulatorUse = accumulator_use;
   static const int kOperandCount = 0;
   static const int kRegisterOperandCount = 0;
-  static const int kRegisterOperandBitmap = 0;
 };
 
 static OperandSize ScaledOperandSize(OperandType operand_type,

src/interpreter/bytecodes.cc

@@ -15,6 +15,7 @@ namespace v8 {
 namespace internal {
 namespace interpreter {
 
+STATIC_CONST_MEMBER_DEFINITION const int Bytecodes::kMaxOperands;
 
 // static
 const char* Bytecodes::ToString(Bytecode bytecode) {
@@ -342,37 +343,8 @@ const OperandTypeInfo* Bytecodes::GetOperandTypeInfos(Bytecode bytecode) {
 OperandSize Bytecodes::GetOperandSize(Bytecode bytecode, int i,
                                       OperandScale operand_scale) {
   DCHECK_LT(i, NumberOfOperands(bytecode));
-  return GetOperandSizes(bytecode, operand_scale)[i];
-}
-
-// static
-const OperandSize* Bytecodes::GetOperandSizes(Bytecode bytecode,
-                                              OperandScale operand_scale) {
-  DCHECK(bytecode <= Bytecode::kLast);
-  switch (bytecode) {
-#define CASE(Name, ...)   \
-  case Bytecode::k##Name: \
-    return BytecodeTraits<__VA_ARGS__>::GetOperandSizes(operand_scale);
-    BYTECODE_LIST(CASE)
-#undef CASE
-  }
-  UNREACHABLE();
-  return nullptr;
-}
-
-// static
-int Bytecodes::GetRegisterOperandBitmap(Bytecode bytecode) {
-  DCHECK(bytecode <= Bytecode::kLast);
-  switch (bytecode) {
-#define CASE(Name, ...)                              \
-  case Bytecode::k##Name:                            \
-    typedef BytecodeTraits<__VA_ARGS__> Name##Trait; \
-    return Name##Trait::kRegisterOperandBitmap;
-    BYTECODE_LIST(CASE)
-#undef CASE
-  }
-  UNREACHABLE();
-  return false;
+  OperandType operand_type = GetOperandType(bytecode, i);
+  return SizeOfOperand(operand_type, operand_scale);
 }
 
 // static
@@ -603,7 +575,7 @@ int Bytecodes::GetNumberOfRegistersRepresentedBy(OperandType operand_type) {
     case OperandType::kRegOutTriple:
       return 3;
     default:
-      UNREACHABLE();
+      return 0;
   }
   return 0;
 }

src/interpreter/bytecodes.h

@@ -438,6 +438,9 @@ class Register final {
 
 class Bytecodes {
  public:
+  //  The maximum number of operands a bytecode may have.
+  static const int kMaxOperands = 4;
+
   // Returns string representation of |bytecode|.
   static const char* ToString(Bytecode bytecode);
 
@@ -525,19 +528,11 @@ class Bytecodes {
   static OperandSize GetOperandSize(Bytecode bytecode, int i,
                                     OperandScale operand_scale);
 
-  // Returns a pointer to an array of the operand sizes for |bytecode|.
-  static const OperandSize* GetOperandSizes(Bytecode bytecode,
-                                            OperandScale operand_scale);
-
   // Returns the offset of the i-th operand of |bytecode| relative to the start
   // of the bytecode.
   static int GetOperandOffset(Bytecode bytecode, int i,
                               OperandScale operand_scale);
 
-  // Returns a zero-based bitmap of the register operand positions of
-  // |bytecode|.
-  static int GetRegisterOperandBitmap(Bytecode bytecode);
-
   // Returns a debug break bytecode to replace |bytecode|.
   static Bytecode GetDebugBreak(Bytecode bytecode);
 

test/unittests/interpreter/bytecodes-unittest.cc

@@ -93,33 +93,6 @@ TEST(OperandScaling, ScalableAndNonScalable) {
   }
 }
 
-TEST(Bytecodes, HasAnyRegisterOperands) {
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kAdd), 1);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kCall), 2);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kCallRuntime), 1);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kCallRuntimeForPair),
-           2);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kDeletePropertyStrict),
-           1);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kForInPrepare), 1);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kInc), 0);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kJumpIfTrue), 0);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kNew), 2);
-  CHECK_EQ(Bytecodes::NumberOfRegisterOperands(Bytecode::kToName), 0);
-}
-
-TEST(Bytecodes, RegisterOperandBitmaps) {
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kAdd), 1);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kCallRuntimeForPair),
-           10);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kStar), 1);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kMov), 3);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kTestIn), 1);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kForInPrepare), 1);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kForInDone), 3);
-  CHECK_EQ(Bytecodes::GetRegisterOperandBitmap(Bytecode::kForInNext), 7);
-}
-
 TEST(Bytecodes, RegisterOperands) {
   CHECK(Bytecodes::IsRegisterOperandType(OperandType::kReg));
   CHECK(Bytecodes::IsRegisterInputOperandType(OperandType::kReg));