[interpreter] Split function into Receiver() and Parameter(i).

The parameter indices are shifted by 1 in BytecodeArrayBuilder
because the receiver is variable at index 0 and not -1.

Split BytecodeArrayBuilder::Parameter(index) method into
Receiver() (same as Parameter(-1)) and
Parameter(index).

This way we avoid confusing (index+1) counting in BytecodeGenerator().

BUG=

Change-Id: Id87ec7c708cecfc3108011994f3177f483772bcc
Reviewed-on: https://chromium-review.googlesource.com/461904Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44262}

src/ast/variables.h

@@ -100,6 +100,12 @@ class Variable final : public ZoneObject {
 
   int index() const { return index_; }
 
+  bool IsReceiver() const {
+    DCHECK(IsParameter());
+
+    return index_ == -1;
+  }
+
   bool IsExport() const {
     DCHECK_EQ(location(), VariableLocation::MODULE);
     DCHECK_NE(index(), 0);

src/interpreter/bytecode-array-builder.cc

@@ -68,7 +68,13 @@ Register BytecodeArrayBuilder::last_context_register() const {
 
 Register BytecodeArrayBuilder::Parameter(int parameter_index) const {
   DCHECK_GE(parameter_index, 0);
-  return Register::FromParameterIndex(parameter_index, parameter_count());
+  // The parameter indices are shifted by 1 (receiver is the
+  // first entry).
+  return Register::FromParameterIndex(parameter_index + 1, parameter_count());
+}
+
+Register BytecodeArrayBuilder::Receiver() const {
+  return Register::FromParameterIndex(0, parameter_count());
 }
 
 Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray(Isolate* isolate) {

src/interpreter/bytecode-array-builder.h

@@ -73,6 +73,7 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final
   }
 
   Register Parameter(int parameter_index) const;
+  Register Receiver() const;
 
   // Constant loads to accumulator.
   BytecodeArrayBuilder& LoadConstantPoolEntry(size_t entry);

src/interpreter/bytecode-generator.cc

@@ -895,9 +895,7 @@ void BytecodeGenerator::VisitVariableDeclaration(VariableDeclaration* decl) {
       break;
     case VariableLocation::PARAMETER:
       if (variable->binding_needs_init()) {
-        // The parameter indices are shifted by 1 (receiver is variable
-        // index -1 but is parameter index 0 in BytecodeArrayBuilder).
-        Register destination(builder()->Parameter(variable->index() + 1));
+        Register destination(builder()->Parameter(variable->index()));
         builder()->LoadTheHole().StoreAccumulatorInRegister(destination);
       }
       break;
@@ -1925,9 +1923,12 @@ void BytecodeGenerator::BuildVariableLoad(Variable* variable, FeedbackSlot slot,
       break;
     }
     case VariableLocation::PARAMETER: {
-      // The parameter indices are shifted by 1 (receiver is variable
-      // index -1 but is parameter index 0 in BytecodeArrayBuilder).
-      Register source = builder()->Parameter(variable->index() + 1);
+      Register source;
+      if (variable->IsReceiver()) {
+        source = builder()->Receiver();
+      } else {
+        source = builder()->Parameter(variable->index());
+      }
       // We need to load the variable into the accumulator, even when in a
       // VisitForRegisterScope, in order to avoid register aliasing if
       // subsequent expressions assign to the same variable.
@@ -2138,7 +2139,11 @@ void BytecodeGenerator::BuildVariableAssignment(Variable* variable,
     case VariableLocation::LOCAL: {
       Register destination;
       if (VariableLocation::PARAMETER == variable->location()) {
-        destination = Register(builder()->Parameter(variable->index() + 1));
+        if (variable->IsReceiver()) {
+          destination = Register(builder()->Receiver());
+        } else {
+          destination = Register(builder()->Parameter(variable->index()));
+        }
       } else {
         destination = Register(variable->index());
       }
@@ -3253,7 +3258,7 @@ void BytecodeGenerator::BuildNewLocalActivationContext() {
     // its sole argument, which we pass on to PushModuleContext.
     RegisterList args = register_allocator()->NewRegisterList(3);
     builder()
-        ->MoveRegister(builder()->Parameter(1), args[0])
+        ->MoveRegister(builder()->Parameter(0), args[0])
         .LoadAccumulatorWithRegister(Register::function_closure())
         .StoreAccumulatorInRegister(args[1])
         .LoadLiteral(scope)
@@ -3289,7 +3294,7 @@ void BytecodeGenerator::BuildLocalActivationContextInitialization() {
 
   if (scope->has_this_declaration() && scope->receiver()->IsContextSlot()) {
     Variable* variable = scope->receiver();
-    Register receiver(builder()->Parameter(0));
+    Register receiver(builder()->Receiver());
     // Context variable (at bottom of the context chain).
     DCHECK_EQ(0, scope->ContextChainLength(variable->scope()));
     builder()->LoadAccumulatorWithRegister(receiver).StoreContextSlot(
@@ -3302,9 +3307,7 @@ void BytecodeGenerator::BuildLocalActivationContextInitialization() {
     Variable* variable = scope->parameter(i);
     if (!variable->IsContextSlot()) continue;
 
-    // The parameter indices are shifted by 1 (receiver is variable
-    // index -1 but is parameter index 0 in BytecodeArrayBuilder).
-    Register parameter(builder()->Parameter(i + 1));
+    Register parameter(builder()->Parameter(i));
     // Context variable (at bottom of the context chain).
     DCHECK_EQ(0, scope->ContextChainLength(variable->scope()));
     builder()->LoadAccumulatorWithRegister(parameter).StoreContextSlot(

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

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

test/cctest/interpreter/test-interpreter.cc

@@ -479,7 +479,7 @@ TEST(InterpreterParameter1) {
   Zone* zone = handles.main_zone();
   BytecodeArrayBuilder builder(isolate, zone, 1, 0, 0);
 
-  builder.LoadAccumulatorWithRegister(builder.Parameter(0)).Return();
+  builder.LoadAccumulatorWithRegister(builder.Receiver()).Return();
   Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
 
   InterpreterTester tester(isolate, bytecode_array);
@@ -517,14 +517,14 @@ TEST(InterpreterParameter8) {
   Handle<i::FeedbackMetadata> metadata =
       NewFeedbackMetadata(isolate, &feedback_spec);
 
-  builder.LoadAccumulatorWithRegister(builder.Parameter(0))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(1), GetIndex(slot))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(2), GetIndex(slot1))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(3), GetIndex(slot2))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(4), GetIndex(slot3))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(5), GetIndex(slot4))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(6), GetIndex(slot5))
-      .BinaryOperation(Token::Value::ADD, builder.Parameter(7), GetIndex(slot6))
+  builder.LoadAccumulatorWithRegister(builder.Receiver())
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(0), GetIndex(slot))
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(1), GetIndex(slot1))
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(2), GetIndex(slot2))
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(3), GetIndex(slot3))
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(4), GetIndex(slot4))
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(5), GetIndex(slot5))
+      .BinaryOperation(Token::Value::ADD, builder.Parameter(6), GetIndex(slot6))
       .Return();
   ast_factory.Internalize(isolate);
   Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
@@ -837,13 +837,13 @@ TEST(InterpreterUnaryOpFeedback) {
     Handle<i::FeedbackMetadata> metadata =
         i::NewFeedbackMetadata(isolate, &feedback_spec);
 
-    builder.LoadAccumulatorWithRegister(builder.Parameter(0))
+    builder.LoadAccumulatorWithRegister(builder.Receiver())
         .CountOperation(test_case.op, GetIndex(slot0))
-        .LoadAccumulatorWithRegister(builder.Parameter(1))
+        .LoadAccumulatorWithRegister(builder.Parameter(0))
         .CountOperation(test_case.op, GetIndex(slot1))
-        .LoadAccumulatorWithRegister(builder.Parameter(2))
+        .LoadAccumulatorWithRegister(builder.Parameter(1))
         .CountOperation(test_case.op, GetIndex(slot2))
-        .LoadAccumulatorWithRegister(builder.Parameter(3))
+        .LoadAccumulatorWithRegister(builder.Parameter(2))
         .CountOperation(test_case.op, GetIndex(slot3))
         .Return();
 
@@ -900,10 +900,10 @@ TEST(InterpreterBitwiseTypeFeedback) {
     Handle<i::FeedbackMetadata> metadata =
         i::NewFeedbackMetadata(isolate, &feedback_spec);
 
-    builder.LoadAccumulatorWithRegister(builder.Parameter(0))
-        .BinaryOperation(op, builder.Parameter(1), GetIndex(slot0))
-        .BinaryOperation(op, builder.Parameter(2), GetIndex(slot1))
-        .BinaryOperation(op, builder.Parameter(3), GetIndex(slot2))
+    builder.LoadAccumulatorWithRegister(builder.Receiver())
+        .BinaryOperation(op, builder.Parameter(0), GetIndex(slot0))
+        .BinaryOperation(op, builder.Parameter(1), GetIndex(slot1))
+        .BinaryOperation(op, builder.Parameter(2), GetIndex(slot2))
         .Return();
 
     Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
@@ -944,8 +944,8 @@ TEST(InterpreterParameter1Assign) {
   BytecodeArrayBuilder builder(isolate, zone, 1, 0, 0);
 
   builder.LoadLiteral(Smi::FromInt(5))
-      .StoreAccumulatorInRegister(builder.Parameter(0))
-      .LoadAccumulatorWithRegister(builder.Parameter(0))
+      .StoreAccumulatorInRegister(builder.Receiver())
+      .LoadAccumulatorWithRegister(builder.Receiver())
       .Return();
   Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
 
@@ -1074,8 +1074,7 @@ TEST(InterpreterLoadNamedProperty) {
 
   BytecodeArrayBuilder builder(isolate, zone, 1, 0, 0);
 
-  builder.LoadNamedProperty(builder.Parameter(0), name, GetIndex(slot))
-      .Return();
+  builder.LoadNamedProperty(builder.Receiver(), name, GetIndex(slot)).Return();
   ast_factory.Internalize(isolate);
   Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
 
@@ -1129,7 +1128,7 @@ TEST(InterpreterLoadKeyedProperty) {
   BytecodeArrayBuilder builder(isolate, zone, 1, 0, 1);
 
   builder.LoadLiteral(key)
-      .LoadKeyedProperty(builder.Parameter(0), GetIndex(slot))
+      .LoadKeyedProperty(builder.Receiver(), GetIndex(slot))
       .Return();
   ast_factory.Internalize(isolate);
   Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
@@ -1172,7 +1171,7 @@ TEST(InterpreterStoreNamedProperty) {
   BytecodeArrayBuilder builder(isolate, zone, 1, 0, 0);
 
   builder.LoadLiteral(Smi::FromInt(999))
-      .StoreNamedProperty(builder.Parameter(0), name, GetIndex(slot), STRICT)
+      .StoreNamedProperty(builder.Receiver(), name, GetIndex(slot), STRICT)
       .Return();
   ast_factory.Internalize(isolate);
   Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
@@ -1237,7 +1236,7 @@ TEST(InterpreterStoreKeyedProperty) {
   builder.LoadLiteral(name)
       .StoreAccumulatorInRegister(Register(0))
       .LoadLiteral(Smi::FromInt(999))
-      .StoreKeyedProperty(builder.Parameter(0), Register(0), GetIndex(slot),
+      .StoreKeyedProperty(builder.Receiver(), Register(0), GetIndex(slot),
                           i::SLOPPY)
       .Return();
   ast_factory.Internalize(isolate);
@@ -1293,9 +1292,9 @@ static void TestInterpreterCall(TailCallMode tail_call_mode) {
     BytecodeArrayBuilder builder(isolate, zone, 1, 0, 1);
     Register reg = builder.register_allocator()->NewRegister();
     RegisterList args = builder.register_allocator()->NewRegisterList(1);
-    builder.LoadNamedProperty(builder.Parameter(0), name, slot_index)
+    builder.LoadNamedProperty(builder.Receiver(), name, slot_index)
         .StoreAccumulatorInRegister(reg)
-        .MoveRegister(builder.Parameter(0), args[0]);
+        .MoveRegister(builder.Receiver(), args[0]);
 
     builder.Call(reg, args, call_slot_index, Call::GLOBAL_CALL, tail_call_mode);
 
@@ -1317,9 +1316,9 @@ static void TestInterpreterCall(TailCallMode tail_call_mode) {
     BytecodeArrayBuilder builder(isolate, zone, 1, 0, 1);
     Register reg = builder.register_allocator()->NewRegister();
     RegisterList args = builder.register_allocator()->NewRegisterList(1);
-    builder.LoadNamedProperty(builder.Parameter(0), name, slot_index)
+    builder.LoadNamedProperty(builder.Receiver(), name, slot_index)
         .StoreAccumulatorInRegister(reg)
-        .MoveRegister(builder.Parameter(0), args[0]);
+        .MoveRegister(builder.Receiver(), args[0]);
     builder.Call(reg, args, call_slot_index, Call::GLOBAL_CALL, tail_call_mode);
     builder.Return();
     ast_factory.Internalize(isolate);
@@ -1343,9 +1342,9 @@ static void TestInterpreterCall(TailCallMode tail_call_mode) {
     Register reg = builder.register_allocator()->NewRegister();
     RegisterList args = builder.register_allocator()->NewRegisterList(3);
 
-    builder.LoadNamedProperty(builder.Parameter(0), name, slot_index)
+    builder.LoadNamedProperty(builder.Receiver(), name, slot_index)
         .StoreAccumulatorInRegister(reg)
-        .LoadAccumulatorWithRegister(builder.Parameter(0))
+        .LoadAccumulatorWithRegister(builder.Receiver())
         .StoreAccumulatorInRegister(args[0])
         .LoadLiteral(Smi::FromInt(51))
         .StoreAccumulatorInRegister(args[1])
@@ -1376,9 +1375,9 @@ static void TestInterpreterCall(TailCallMode tail_call_mode) {
     Register reg = builder.register_allocator()->NewRegister();
     RegisterList args = builder.register_allocator()->NewRegisterList(11);
 
-    builder.LoadNamedProperty(builder.Parameter(0), name, slot_index)
+    builder.LoadNamedProperty(builder.Receiver(), name, slot_index)
         .StoreAccumulatorInRegister(reg)
-        .LoadAccumulatorWithRegister(builder.Parameter(0))
+        .LoadAccumulatorWithRegister(builder.Receiver())
         .StoreAccumulatorInRegister(args[0])
         .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("a")))
         .StoreAccumulatorInRegister(args[1])
@@ -2106,7 +2105,7 @@ TEST(InterpreterCompareTypeOf) {
     if (literal_flag == LiteralFlag::kOther) continue;
 
     BytecodeArrayBuilder builder(isolate, zone, 1, 0, 0);
-    builder.LoadAccumulatorWithRegister(builder.Parameter(0))
+    builder.LoadAccumulatorWithRegister(builder.Receiver())
         .CompareTypeOf(kLiterals[l])
         .Return();
     Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);

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

@@ -514,9 +514,9 @@ TEST_F(BytecodeArrayBuilderTest, Parameters) {
   CanonicalHandleScope canonical(isolate());
   BytecodeArrayBuilder builder(isolate(), zone(), 10, 0, 0);
 
-  Register param0(builder.Parameter(0));
-  Register param9(builder.Parameter(9));
-  CHECK_EQ(param9.index() - param0.index(), 9);
+  Register receiver(builder.Receiver());
+  Register param8(builder.Parameter(8));
+  CHECK_EQ(param8.index() - receiver.index(), 9);
 }