diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index 1c3b626b90..be3ee97b17 100644 --- a/src/compiler/js-call-reducer.cc +++ b/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)) { diff --git a/src/compiler/js-create-lowering.cc b/src/compiler/js-create-lowering.cc index 46004c3f96..c212f5388d 100644 --- a/src/compiler/js-create-lowering.cc +++ b/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()); } diff --git a/src/compiler/state-values-utils.h b/src/compiler/state-values-utils.h index ff66c3df71..78d57a92b9 100644 --- a/src/compiler/state-values-utils.h +++ b/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: