[compiler] Add convenience accessors to state value iteration

The `parameters` attached to FrameState nodes are often iterated s.t.
the receiver (implicitly at index 0), and potentially some leading
parameters, are skipped. The new convenience functions
`begin_without_receiver` and `begin_without_receiver_and_skip` make this
pattern more convenient.

Bug: chromium:1166136
Change-Id: Ic2bc7319edf9b8567346788dfaebd8852672a703
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2637221
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72211}

src/compiler/js-call-reducer.cc

@@ -3881,7 +3881,6 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
   CreateArgumentsType const type = CreateArgumentsTypeOf(arguments_list->op());
   FrameState frame_state =
       FrameState{NodeProperties::GetFrameStateInput(arguments_list)};
-  int start_index = 0;
 
   int formal_parameter_count;
   {
@@ -3904,8 +3903,6 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
         return NoChange();
       }
     }
-  } else if (type == CreateArgumentsType::kRestParameter) {
-    start_index = formal_parameter_count;
   }
 
   // TODO(jgruber,v8:8888): Attempt to remove this restriction. The reason it
@@ -3922,6 +3919,13 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
   // Remove the {arguments_list} input from the {node}.
   node->RemoveInput(arraylike_or_spread_index);
 
+  // The index of the first relevant parameter. Only non-zero when looking at
+  // rest parameters, in which case it is set to the index of the first rest
+  // parameter.
+  const int start_index = (type == CreateArgumentsType::kRestParameter)
+                              ? formal_parameter_count
+                              : 0;
+
   // After removing the arraylike or spread object, the argument count is:
   int argc =
       arraylike_or_spread_index - JSCallOrConstructNode::FirstArgumentIndex();
@@ -3952,29 +3956,12 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
     frame_state = outer_state;
   }
   // Add the actual parameters to the {node}, skipping the receiver.
-  const int argument_count =
-      frame_state.frame_state_info().parameter_count() - 1;  // Minus receiver.
-  if (start_index < argument_count) {
-    StateValuesAccess parameters_access(frame_state.parameters());
-    auto parameters_it = ++parameters_access.begin();  // Skip the receiver.
-    for (int i = 0; i < start_index; i++) {
-      // A non-zero start_index implies that there are rest arguments. Skip
-      // them.
-      ++parameters_it;
-    }
-    for (int i = start_index; i < argument_count; ++i, ++parameters_it) {
-      Node* parameter_node = parameters_it.node();
-      DCHECK_NOT_NULL(parameter_node);
-      node->InsertInput(graph()->zone(),
-                        JSCallOrConstructNode::ArgumentIndex(argc++),
-                        parameter_node);
-    }
-    // TODO(jgruber): Currently, each use-site does the awkward dance above,
-    // iterating based on the FrameStateInfo's parameter count minus one, and
-    // manually advancing the iterator past the receiver. Consider wrapping all
-    // this in an understandable iterator s.t. one only needs to iterate from
-    // the beginning until done().
-    DCHECK(parameters_it.done());
+  StateValuesAccess parameters_access(frame_state.parameters());
+  for (auto it = parameters_access.begin_without_receiver_and_skip(start_index);
+       !it.done(); ++it) {
+    DCHECK_NOT_NULL(it.node());
+    node->InsertInput(graph()->zone(),
+                      JSCallOrConstructNode::ArgumentIndex(argc++), it.node());
   }
 
   if (IsCallWithArrayLikeOrSpread(node)) {

src/compiler/js-create-lowering.cc

@@ -1432,7 +1432,7 @@ Node* JSCreateLowering::AllocateArguments(Node* effect, Node* control,
   // Prepare an iterator over argument values recorded in the frame state.
   Node* const parameters = frame_state.parameters();
   StateValuesAccess parameters_access(parameters);
-  auto parameters_it = ++parameters_access.begin();
+  auto parameters_it = parameters_access.begin_without_receiver();
 
   // Actually allocate the backing store.
   AllocationBuilder a(jsgraph(), effect, control);
@@ -1459,12 +1459,8 @@ Node* JSCreateLowering::AllocateRestArguments(Node* effect, Node* control,
   // Prepare an iterator over argument values recorded in the frame state.
   Node* const parameters = frame_state.parameters();
   StateValuesAccess parameters_access(parameters);
-  auto parameters_it = ++parameters_access.begin();
-
-  // Skip unused arguments.
-  for (int i = 0; i < start_index; i++) {
-    ++parameters_it;
-  }
+  auto parameters_it =
+      parameters_access.begin_without_receiver_and_skip(start_index);
 
   // Actually allocate the backing store.
   AllocationBuilder a(jsgraph(), effect, control);
@@ -1501,7 +1497,8 @@ Node* JSCreateLowering::AllocateAliasedArguments(
   // Prepare an iterator over argument values recorded in the frame state.
   Node* const parameters = frame_state.parameters();
   StateValuesAccess parameters_access(parameters);
-  auto parameters_it = ++parameters_access.begin();
+  auto parameters_it =
+      parameters_access.begin_without_receiver_and_skip(mapped_count);
 
   // The unmapped argument values recorded in the frame state are stored yet
   // another indirection away and then linked into the parameter map below,
@@ -1509,7 +1506,7 @@ Node* JSCreateLowering::AllocateAliasedArguments(
   AllocationBuilder aa(jsgraph(), effect, control);
   aa.AllocateArray(argument_count,
                    MapRef(broker(), factory()->fixed_array_map()));
-  for (int i = 0; i < mapped_count; ++i, ++parameters_it) {
+  for (int i = 0; i < mapped_count; ++i) {
     aa.Store(AccessBuilder::ForFixedArrayElement(), jsgraph()->Constant(i),
              jsgraph()->TheHoleConstant());
   }

src/compiler/state-values-utils.h

@@ -126,6 +126,17 @@ class V8_EXPORT_PRIVATE StateValuesAccess {
 
   size_t size() const;
   iterator begin() const { return iterator(node_); }
+  iterator begin_without_receiver() const {
+    return ++begin();  // Skip the receiver.
+  }
+  iterator begin_without_receiver_and_skip(int n_skips) {
+    iterator it = begin_without_receiver();
+    while (n_skips > 0 && !it.done()) {
+      ++it;
+      --n_skips;
+    }
+    return it;
+  }
   iterator end() const { return iterator(); }
 
  private: