[Interpreter] Make RegisterList constructor private to avoid missue.

RegisterLists should only be allocated via the register allocator. To ensure
this, make the RegisterList constructor private and only expose it to tests
and the BytecodeRegisterAllocator.

Change-Id: I09ebfc5c0f1baecfb1333fd672b96d462fd26fcf
Reviewed-on: https://chromium-review.googlesource.com/822196
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50073}

src/interpreter/bytecode-array-builder.cc

@@ -1389,7 +1389,7 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CallRuntime(
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CallRuntime(
     Runtime::FunctionId function_id, Register arg) {
-  return CallRuntime(function_id, RegisterList(arg.index(), 1));
+  return CallRuntime(function_id, RegisterList(arg));
 }
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CallRuntime(
@@ -1411,8 +1411,7 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CallRuntimeForPair(
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CallRuntimeForPair(
     Runtime::FunctionId function_id, Register arg, RegisterList return_pair) {
-  return CallRuntimeForPair(function_id, RegisterList(arg.index(), 1),
-                            return_pair);
+  return CallRuntimeForPair(function_id, RegisterList(arg), return_pair);
 }
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CallJSRuntime(int context_index,

src/interpreter/bytecode-generator.cc

@@ -2787,7 +2787,7 @@ void BytecodeGenerator::VisitCompoundAssignment(CompoundAssignment* expr) {
 // accumulator. When the generator is resumed, the sent value is loaded in the
 // accumulator.
 void BytecodeGenerator::BuildSuspendPoint(int suspend_id) {
-  RegisterList registers(0, register_allocator()->next_register_index());
+  RegisterList registers = register_allocator()->AllLiveRegisters();
 
   // Save context, registers, and state. Then return.
   builder()->SuspendGenerator(generator_object(), registers, suspend_id);
@@ -3421,16 +3421,15 @@ void BytecodeGenerator::VisitCall(Call* expr) {
         Register name = register_allocator()->NewRegister();
 
         // Call %LoadLookupSlotForCall to get the callee and receiver.
-        DCHECK(Register::AreContiguous(callee, receiver));
-        RegisterList result_pair(callee.index(), 2);
-        USE(receiver);
-
+        RegisterList result_pair = register_allocator()->NewRegisterList(2);
         Variable* variable = callee_expr->AsVariableProxy()->var();
         builder()
             ->LoadLiteral(variable->raw_name())
             .StoreAccumulatorInRegister(name)
             .CallRuntimeForPair(Runtime::kLoadLookupSlotForCall, name,
-                                result_pair);
+                                result_pair)
+            .MoveRegister(result_pair[0], callee)
+            .MoveRegister(result_pair[1], receiver);
       }
       break;
     }

src/interpreter/bytecode-register-allocator.h

@@ -87,6 +87,11 @@ class BytecodeRegisterAllocator final {
     return reg.index() < next_register_index_;
   }
 
+  // Returns a register list for all currently live registers.
+  RegisterList AllLiveRegisters() const {
+    return RegisterList(0, next_register_index());
+  }
+
   void set_observer(Observer* observer) { observer_ = observer; }
 
   int next_register_index() const { return next_register_index_; }

src/interpreter/bytecode-register-optimizer.cc

@@ -448,7 +448,7 @@ RegisterList BytecodeRegisterOptimizer::GetInputRegisterList(
   if (reg_list.register_count() == 1) {
     // If there is only a single register, treat it as a normal input register.
     Register reg(GetInputRegister(reg_list.first_register()));
-    return RegisterList(reg.index(), 1);
+    return RegisterList(reg);
   } else {
     int start_index = reg_list.first_register().index();
     for (int i = 0; i < reg_list.register_count(); ++i) {

src/interpreter/bytecode-register.h

@@ -61,9 +61,9 @@ class V8_EXPORT_PRIVATE Register final {
   }
 
   static bool AreContiguous(Register reg1, Register reg2,
-                            Register reg3 = Register(),
-                            Register reg4 = Register(),
-                            Register reg5 = Register());
+                            Register reg3 = invalid_value(),
+                            Register reg4 = invalid_value(),
+                            Register reg5 = invalid_value());
 
   std::string ToString(int parameter_count) const;
 
@@ -98,14 +98,11 @@ class V8_EXPORT_PRIVATE Register final {
 
 class RegisterList {
  public:
-  RegisterList() : first_reg_index_(Register().index()), register_count_(0) {}
-  RegisterList(int first_reg_index, int register_count)
-      : first_reg_index_(first_reg_index), register_count_(register_count) {}
+  RegisterList()
+      : first_reg_index_(Register::invalid_value().index()),
+        register_count_(0) {}
   explicit RegisterList(Register r) : RegisterList(r.index(), 1) {}
 
-  // Increases the size of the register list by one.
-  void IncrementRegisterCount() { register_count_++; }
-
   // Returns a new RegisterList which is a truncated version of this list, with
   // |count| registers.
   const RegisterList Truncate(int new_count) {
@@ -130,6 +127,17 @@ class RegisterList {
   int register_count() const { return register_count_; }
 
  private:
+  friend class BytecodeRegisterAllocator;
+  friend class BytecodeDecoder;
+  friend class InterpreterTester;
+  friend class BytecodeUtils;
+
+  RegisterList(int first_reg_index, int register_count)
+      : first_reg_index_(first_reg_index), register_count_(register_count) {}
+
+  // Increases the size of the register list by one.
+  void IncrementRegisterCount() { register_count_++; }
+
   int first_reg_index_;
   int register_count_;
 };

src/interpreter/handler-table-builder.cc

@@ -34,7 +34,7 @@ Handle<HandlerTable> HandlerTableBuilder::ToHandlerTable(Isolate* isolate) {
 
 int HandlerTableBuilder::NewHandlerEntry() {
   int handler_id = static_cast<int>(entries_.size());
-  Entry entry = {0, 0, 0, Register(), HandlerTable::UNCAUGHT};
+  Entry entry = {0, 0, 0, Register::invalid_value(), HandlerTable::UNCAUGHT};
   entries_.push_back(entry);
   return handler_id;
 }

test/cctest/interpreter/interpreter-tester.h

@@ -83,6 +83,11 @@ class InterpreterTester {
 
   static const char kFunctionName[];
 
+  // Expose raw RegisterList construction to tests.
+  static RegisterList NewRegisterList(int first_reg_index, int register_count) {
+    return RegisterList(first_reg_index, register_count);
+  }
+
  private:
   Isolate* isolate_;
   const char* source_;

test/cctest/interpreter/test-interpreter-intrinsics.cc

@@ -27,7 +27,8 @@ class InvokeIntrinsicHelper {
   Handle<Object> Invoke(A... args) {
     CHECK(IntrinsicsHelper::IsSupported(function_id_));
     BytecodeArrayBuilder builder(zone_, sizeof...(args), 0, 0);
-    RegisterList reg_list(builder.Receiver().index(), sizeof...(args));
+    RegisterList reg_list = InterpreterTester::NewRegisterList(
+        builder.Receiver().index(), sizeof...(args));
     builder.CallRuntime(function_id_, reg_list).Return();
     InterpreterTester tester(isolate_, builder.ToBytecodeArray(isolate_));
     auto callable = tester.GetCallable<A...>();

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

@@ -13,6 +13,7 @@
 #include "src/interpreter/bytecode-label.h"
 #include "src/interpreter/bytecode-register-allocator.h"
 #include "src/objects-inl.h"
+#include "test/unittests/interpreter/bytecode-utils.h"
 #include "test/unittests/test-utils.h"
 
 namespace v8 {
@@ -41,8 +42,11 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
   Register reg(0);
   Register other(reg.index() + 1);
   Register wide(128);
-  RegisterList reg_list(0, 10);
-  RegisterList empty, single(0, 1), pair(0, 2), triple(0, 3);
+  RegisterList empty;
+  RegisterList single = BytecodeUtils::NewRegisterList(0, 1);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
+  RegisterList reg_list = BytecodeUtils::NewRegisterList(0, 10);
 
   // Emit argument creation operations.
   builder.CreateArguments(CreateArgumentsType::kMappedArguments)

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

@@ -7,6 +7,7 @@
 #include "src/interpreter/bytecode-array-builder.h"
 #include "src/interpreter/bytecode-array-iterator.h"
 #include "src/objects-inl.h"
+#include "test/unittests/interpreter/bytecode-utils.h"
 #include "test/unittests/test-utils.h"
 
 namespace v8 {
@@ -33,8 +34,8 @@ TEST_F(BytecodeArrayIteratorTest, IteratesBytecodeArray) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t name_index = 2;

test/unittests/interpreter/bytecode-array-random-iterator-unittest.cc

@@ -7,6 +7,7 @@
 #include "src/interpreter/bytecode-array-builder.h"
 #include "src/interpreter/bytecode-array-random-iterator.h"
 #include "src/objects-inl.h"
+#include "test/unittests/interpreter/bytecode-utils.h"
 #include "test/unittests/test-utils.h"
 
 namespace v8 {
@@ -33,8 +34,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, InvalidBeforeStart) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t feedback_slot = feedback_spec.AddLoadICSlot().ToInt();
@@ -87,8 +88,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, InvalidAfterEnd) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t feedback_slot = feedback_spec.AddLoadICSlot().ToInt();
@@ -141,8 +142,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, AccessesFirst) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t feedback_slot = feedback_spec.AddLoadICSlot().ToInt();
@@ -199,8 +200,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, AccessesLast) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t feedback_slot = feedback_spec.AddLoadICSlot().ToInt();
@@ -258,8 +259,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, RandomAccessValid) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t name_index = 2;
@@ -443,8 +444,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, IteratesBytecodeArray) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t name_index = 2;
@@ -722,8 +723,8 @@ TEST_F(BytecodeArrayRandomIteratorTest, IteratesBytecodeArrayBackwards) {
   Smi* smi_1 = Smi::FromInt(-65536);
   Register reg_0(0);
   Register reg_1(1);
-  RegisterList pair(0, 2);
-  RegisterList triple(0, 3);
+  RegisterList pair = BytecodeUtils::NewRegisterList(0, 2);
+  RegisterList triple = BytecodeUtils::NewRegisterList(0, 3);
   Register param = Register::FromParameterIndex(2, builder.parameter_count());
   const AstRawString* name = ast_factory.GetOneByteString("abc");
   uint32_t name_index = 2;

test/unittests/interpreter/bytecode-register-optimizer-unittest.cc

@@ -6,6 +6,7 @@
 
 #include "src/interpreter/bytecode-label.h"
 #include "src/interpreter/bytecode-register-optimizer.h"
+#include "test/unittests/interpreter/bytecode-utils.h"
 #include "test/unittests/test-utils.h"
 
 namespace v8 {
@@ -169,8 +170,8 @@ TEST_F(BytecodeRegisterOptimizerTest, SingleTemporaryNotMaterializedForInput) {
   CHECK_EQ(write_count(), 0u);
 
   Register reg = optimizer()->GetInputRegister(temp0);
-  RegisterList reg_list =
-      optimizer()->GetInputRegisterList(RegisterList(temp0.index(), 1));
+  RegisterList reg_list = optimizer()->GetInputRegisterList(
+      BytecodeUtils::NewRegisterList(temp0.index(), 1));
   CHECK_EQ(write_count(), 0u);
   CHECK_EQ(parameter.index(), reg.index());
   CHECK_EQ(parameter.index(), reg_list.first_register().index());
@@ -189,8 +190,8 @@ TEST_F(BytecodeRegisterOptimizerTest, RangeOfTemporariesMaterializedForInput) {
 
   optimizer()
       ->PrepareForBytecode<Bytecode::kCallJSRuntime, AccumulatorUse::kWrite>();
-  RegisterList reg_list =
-      optimizer()->GetInputRegisterList(RegisterList(temp0.index(), 2));
+  RegisterList reg_list = optimizer()->GetInputRegisterList(
+      BytecodeUtils::NewRegisterList(temp0.index(), 2));
   CHECK_EQ(temp0.index(), reg_list.first_register().index());
   CHECK_EQ(2, reg_list.register_count());
   CHECK_EQ(write_count(), 2u);

test/unittests/interpreter/bytecode-utils.h

@@ -6,6 +6,11 @@
 #define V8_UNITTESTS_INTERPRETER_BYTECODE_UTILS_H_
 
 #include "src/frames.h"
+#include "src/interpreter/bytecode-register.h"
+
+namespace v8 {
+namespace internal {
+namespace interpreter {
 
 #if V8_TARGET_LITTLE_ENDIAN
 
@@ -33,4 +38,19 @@
 #define R16(i) U16(REG_OPERAND(i))
 #define R32(i) U32(REG_OPERAND(i))
 
+class BytecodeUtils {
+ public:
+  // Expose raw RegisterList construction to tests.
+  static RegisterList NewRegisterList(int first_reg_index, int register_count) {
+    return RegisterList(first_reg_index, register_count);
+  }
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(BytecodeUtils);
+};
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8
+
 #endif  // V8_UNITTESTS_INTERPRETER_BYTECODE_UTILS_H_