[interpreter] Wide register support.

src/interpreter/bytecode-array-builder.cc

Index: src/interpreter/bytecode-array-builder.cc
diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc
index c513663a2f97e06d713ae2c418227d597dcf2218..ca84edd83975ba7644c1e1aa539ab3fe7a5bb0be 100644
--- a/src/interpreter/bytecode-array-builder.cc
+++ b/src/interpreter/bytecode-array-builder.cc
@@ -8,7 +8,7 @@ namespace v8 {
 namespace internal {
 namespace interpreter {
 
-class BytecodeArrayBuilder::PreviousBytecodeHelper {
+class BytecodeArrayBuilder::PreviousBytecodeHelper BASE_EMBEDDED {
  public:
   explicit PreviousBytecodeHelper(const BytecodeArrayBuilder& array_builder)
       : array_builder_(array_builder),
@@ -37,9 +37,9 @@ class BytecodeArrayBuilder::PreviousBytecodeHelper {
         Bytecodes::GetOperandOffset(bytecode, operand_index);
     OperandSize size = Bytecodes::GetOperandSize(bytecode, operand_index);
     switch (size) {
-      default:
       case OperandSize::kNone:
         UNREACHABLE();
+        break;
       case OperandSize::kByte:
         return static_cast<uint32_t>(
             array_builder_.bytecodes()->at(operand_offset));
@@ -49,6 +49,7 @@ class BytecodeArrayBuilder::PreviousBytecodeHelper {
             array_builder_.bytecodes()->at(operand_offset + 1);
         return static_cast<uint32_t>(operand);
     }
+    return 0;
   }
 
   Handle<Object> GetConstantForIndexOperand(int operand_index) const {
@@ -79,7 +80,8 @@ BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone)
       local_register_count_(-1),
       context_register_count_(-1),
       temporary_register_count_(0),
-      free_temporaries_(zone) {}
+      free_temporaries_(zone),
+      register_translator_(this) {}
 
 
 BytecodeArrayBuilder::~BytecodeArrayBuilder() { DCHECK_EQ(0, unbound_jumps_); }
@@ -148,7 +150,8 @@ Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() {
   EnsureReturn();
 
   int bytecode_size = static_cast<int>(bytecodes_.size());
-  int register_count = fixed_register_count() + temporary_register_count_;
+  int register_count =
+      fixed_and_temporary_register_count() + translation_register_count();
   int frame_size = register_count * kPointerSize;
   Handle<FixedArray> constant_pool = constant_array_builder()->ToFixedArray();
   Handle<FixedArray> handler_table = handler_table_builder()->ToHandlerTable();
@@ -166,14 +169,23 @@ void BytecodeArrayBuilder::Output(Bytecode bytecode, uint32_t(&operands)[N]) {
   // Don't output dead code.
   if (exit_seen_in_block_) return;
 
-  DCHECK_EQ(Bytecodes::NumberOfOperands(bytecode), static_cast<int>(N));
+  int operand_count = static_cast<int>(N);
+  DCHECK_EQ(Bytecodes::NumberOfOperands(bytecode), operand_count);
+
+  int register_operand_count = Bytecodes::NumberOfRegisterOperands(bytecode);
+  if (register_operand_count > 0) {
+    register_translator()->TranslateRegisters(bytecode, operands,
+                                              operand_count);
+  }
+
   last_bytecode_start_ = bytecodes()->size();
   bytecodes()->push_back(Bytecodes::ToByte(bytecode));
-  for (int i = 0; i < static_cast<int>(N); i++) {
+  for (int i = 0; i < operand_count; i++) {
     DCHECK(OperandIsValid(bytecode, i, operands[i]));
     switch (Bytecodes::GetOperandSize(bytecode, i)) {
       case OperandSize::kNone:
         UNREACHABLE();
+        break;
       case OperandSize::kByte:
         bytecodes()->push_back(static_cast<uint8_t>(operands[i]));
         break;
@@ -186,6 +198,10 @@ void BytecodeArrayBuilder::Output(Bytecode bytecode, uint32_t(&operands)[N]) {
       }
     }
   }
+
+  if (register_operand_count > 0) {
+    register_translator()->UntranslateRegisters();

[rmcilroy, 2016/01/25 12:12:05]

Not sure about the naming of UntranslateRegisters. How about TranslateRegisters
-> TranslateAndMoveRegisters()  and UntranslateRegisters ->
RestoreTranslatedRegisters() ?

[oth, 2016/01/25 16:51:56]

Done as discussed TranslateInputRegisters/TranslateOutputRegisters to match
follow-on CL.

+  }
 }
 
 
@@ -361,9 +377,14 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::StoreAccumulatorInRegister(
 BytecodeArrayBuilder& BytecodeArrayBuilder::MoveRegister(Register from,
                                                          Register to) {
   DCHECK(from != to);
-  if (FitsInReg8Operand(to) && FitsInReg8Operand(from)) {
+  // Move bytecodes modify the stack. Chacking validity is essential.

[rmcilroy, 2016/01/25 12:12:05]

I'm not sure what this comment means? I'm guessing this is explaining why
RegisterIsValid is here with CHECK instead of DCHECK. Star also modifies the
stack, should we add the check there too, or do it for all kReg8Output registers
when we add that operand. 

Either way, /s/Chacking/Checking 

[oth, 2016/01/25 16:51:56]

Have changed this to a DCHECK for the purpose of this CL. I tend to think all
register operands should be checked, but not in this CL. The particular one to
watch at this juncture is in MoveRegisterUntranslated which remains a CHECK.

+  if (FitsInReg8Operand(from) && FitsInReg8Operand(to)) {
+    CHECK(RegisterIsValid(from, OperandType::kReg8) &&
+          RegisterIsValid(to, OperandType::kReg8));
     Output(Bytecode::kMov, from.ToRawOperand(), to.ToRawOperand());
-  } else if (FitsInReg16Operand(to) && FitsInReg16Operand(from)) {
+  } else if (FitsInReg16Operand(from) && FitsInReg16Operand(to)) {
+    CHECK(RegisterIsValid(from, OperandType::kReg16) &&
+          RegisterIsValid(to, OperandType::kReg16));
     Output(Bytecode::kMovWide, from.ToRawOperand(), to.ToRawOperand());
   } else {
     UNIMPLEMENTED();
@@ -372,6 +393,22 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::MoveRegister(Register from,
 }
 
 
+void BytecodeArrayBuilder::MoveRegisterUntranslated(Register from,
+                                                    Register to) {
+  // Move bytecodes modify the stack. Chacking validity is essential.
+  if (FitsInReg8OperandUntranslated(from)) {
+    CHECK(RegisterIsValid(from, OperandType::kReg8) &&
+          RegisterIsValid(to, OperandType::kReg16));
+  } else if (FitsInReg8OperandUntranslated(to)) {
+    CHECK(RegisterIsValid(from, OperandType::kReg16) &&
+          RegisterIsValid(to, OperandType::kReg8));
+  } else {
+    UNIMPLEMENTED();
+  }
+  Output(Bytecode::kMovWide, from.ToRawOperand(), to.ToRawOperand());
+}
+
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::LoadGlobal(
     const Handle<String> name, int feedback_slot, LanguageMode language_mode,
     TypeofMode typeof_mode) {
@@ -1083,7 +1120,6 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::New(Register constructor,
     DCHECK_EQ(0u, arg_count);
     first_arg = Register(0);
   }
-
   if (FitsInReg8Operand(constructor) && FitsInReg8Operand(first_arg) &&
       FitsInIdx8Operand(arg_count)) {
     Output(Bytecode::kNew, constructor.ToRawOperand(), first_arg.ToRawOperand(),
@@ -1181,9 +1217,14 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
 }
 
 
+void BytecodeArrayBuilder::ForgeTemporaryRegister() {
+  temporary_register_count_++;
+}
+
+
 int BytecodeArrayBuilder::BorrowTemporaryRegister() {
   if (free_temporaries_.empty()) {
-    temporary_register_count_ += 1;
+    ForgeTemporaryRegister();
     return last_temporary_register().index();
   } else {
     auto pos = free_temporaries_.begin();
@@ -1202,7 +1243,7 @@ int BytecodeArrayBuilder::BorrowTemporaryRegisterNotInRange(int start_index,
     // greater than end_index.
     index = free_temporaries_.upper_bound(end_index);
     if (index == free_temporaries_.end()) {
-      temporary_register_count_ += 1;
+      ForgeTemporaryRegister();
       return last_temporary_register().index();
     }
   } else {
@@ -1234,11 +1275,33 @@ int BytecodeArrayBuilder::PrepareForConsecutiveTemporaryRegisters(
     return -1;
   }
 
+  // TODO(oth): replace use of set<> here for free_temporaries with a
+  // more efficient structure. And/or partition into two searches -
+  // one before the translation window and one after.
+
+  // A run will require at least |count| free temporaries.
+  while (free_temporaries_.size() < count) {
+    ForgeTemporaryRegister();
+    free_temporaries_.insert(last_temporary_register().index());
+  }
+
   // 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)) {
+    int expected = *start + static_cast<int>(run_length);
+    if (*run_end != expected) {
+      start = run_end;
+      run_length = 0;
+    }
+    Register reg_start(*start);
+    Register reg_expected(expected);
+    if (RegisterTranslator::DistanceToTranslationWindow(reg_start) > 0 &&
+        RegisterTranslator::DistanceToTranslationWindow(reg_expected) <= 0) {
+      // Run straddles the lower edge of the translation window. Registers
+      // after the start of this boundary are displaced by the register
+      // translator to provide a hole for translation. Runs either side
+      // of the boundary are fine.
       start = run_end;
       run_length = 0;
     }
@@ -1255,12 +1318,34 @@ int BytecodeArrayBuilder::PrepareForConsecutiveTemporaryRegisters(
     run_length = 0;
   }
 
+  // Pad temporaries if extended run would cross translation boundary.
+  Register reg_first(*start);
+  Register reg_last(*start + static_cast<int>(count) - 1);
+  DCHECK_GT(RegisterTranslator::DistanceToTranslationWindow(reg_first),
+            RegisterTranslator::DistanceToTranslationWindow(reg_last));
+  while (RegisterTranslator::DistanceToTranslationWindow(reg_first) > 0 &&
+         RegisterTranslator::DistanceToTranslationWindow(reg_last) <= 0) {
+    ForgeTemporaryRegister();
+    free_temporaries_.insert(last_temporary_register().index());
+    start = --free_temporaries_.end();
+    reg_first = Register(*start);
+    reg_last = Register(*start + static_cast<int>(count) - 1);
+    run_length = 0;
+  }
+
   // Ensure enough registers for run.
   while (run_length++ < count) {
-    temporary_register_count_++;
+    ForgeTemporaryRegister();
     free_temporaries_.insert(last_temporary_register().index());
   }
-  return last_temporary_register().index() - static_cast<int>(count) + 1;
+
+  int run_start =
+      last_temporary_register().index() - static_cast<int>(count) + 1;
+  DCHECK(RegisterTranslator::DistanceToTranslationWindow(Register(run_start)) <=
+             0 ||
+         RegisterTranslator::DistanceToTranslationWindow(
+             Register(run_start + static_cast<int>(count) - 1)) > 0);
+  return run_start;
 }
 
 
@@ -1331,10 +1416,14 @@ bool BytecodeArrayBuilder::RegisterIsValid(Register reg,
                                            OperandType reg_type) const {
   switch (Bytecodes::SizeOfOperand(reg_type)) {
     case OperandSize::kByte:
-      if (!FitsInReg8Operand(reg)) { return false; }
+      if (!FitsInReg8OperandUntranslated(reg)) {
+        return false;
+      }
       break;
     case OperandSize::kShort:
-      if (!FitsInReg16Operand(reg)) { return false; }
+      if (!FitsInReg16OperandUntranslated(reg)) {
+        return false;
+      }
       break;
     case OperandSize::kNone:
       UNREACHABLE();
@@ -1345,12 +1434,12 @@ bool BytecodeArrayBuilder::RegisterIsValid(Register reg,
       reg.is_new_target()) {
     return true;
   } else if (reg.is_parameter()) {
-    int parameter_index = reg.ToParameterIndex(parameter_count_);
-    return parameter_index >= 0 && parameter_index < parameter_count_;
-  } else if (reg.index() < fixed_register_count()) {
-    return true;
+    int parameter_index = reg.ToParameterIndex(parameter_count());
+    return parameter_index >= 0 && parameter_index < parameter_count();
   } else {
-    return TemporaryRegisterIsLive(reg);
+    int register_limit =
+        (fixed_and_temporary_register_count() + translation_register_count());
+    return reg.index() < register_limit;

[rmcilroy, 2016/01/25 12:12:05]

Same comment as last time on this line - looks like we have less checking here
since we are just checking the register is in the valid range and not whether it
is live. Is this intended?

[oth, 2016/01/25 16:51:56]

No, brain melt. Fixed.

   }
 }
 
@@ -1365,9 +1454,10 @@ bool BytecodeArrayBuilder::IsRegisterInAccumulator(Register reg) {
   if (LastBytecodeInSameBlock()) {
     PreviousBytecodeHelper previous_bytecode(*this);
     Bytecode bytecode = previous_bytecode.GetBytecode();
-    if ((bytecode == Bytecode::kLdar || bytecode == Bytecode::kStar) &&
-        (reg == Register::FromOperand(previous_bytecode.GetOperand(0)))) {
-      return true;
+    if (bytecode == Bytecode::kLdar || bytecode == Bytecode::kStar) {
+      Register previous_reg =
+          Register::FromOperand(previous_bytecode.GetOperand(0));
+      return previous_reg == reg;
     }
   }
   return false;
@@ -1680,13 +1770,25 @@ bool BytecodeArrayBuilder::FitsInIdx16Operand(size_t value) {
 
 // static
 bool BytecodeArrayBuilder::FitsInReg8Operand(Register value) {
-  return kMinInt8 <= value.index() && value.index() <= kMaxInt8;
+  return RegisterTranslator::FitsInReg8Operand(value);
+}
+
+
+// static
+bool BytecodeArrayBuilder::FitsInReg8OperandUntranslated(Register value) {
+  return value.is_byte_operand();
 }
 
 
 // static
 bool BytecodeArrayBuilder::FitsInReg16Operand(Register value) {
-  return kMinInt16 <= value.index() && value.index() <= kMaxInt16;
+  return RegisterTranslator::FitsInReg16Operand(value);
+}
+
+
+// static
+bool BytecodeArrayBuilder::FitsInReg16OperandUntranslated(Register value) {
+  return value.is_short_operand();
 }
 
 }  // namespace interpreter