[parsing,interpreter] Use StoreInArrayLiteral IC for array rest pattern.

This removes the last use of %AppendElement (and the function itself),
which was in the pattern rewriter's code for destructuring assignment
with an array rest pattern. In its place, it introduces a
StoreInArrayLiteral AST node that corresponds to the StoreInArrayLiteral
bytecode (which in turn corresponds to the StoreInArrayLiteral IC).

Change-Id: I1d212407b025cf0919263d119f6f47c88bd9a71e
Reviewed-on: https://chromium-review.googlesource.com/955307
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51898}

src/ast/ast-traversal-visitor.h

@@ -517,6 +517,15 @@ void AstTraversalVisitor<Subclass>::VisitSpread(Spread* expr) {
   RECURSE_EXPRESSION(Visit(expr->expression()));
 }
 
+template <class Subclass>
+void AstTraversalVisitor<Subclass>::VisitStoreInArrayLiteral(
+    StoreInArrayLiteral* expr) {
+  PROCESS_EXPRESSION(expr);
+  RECURSE_EXPRESSION(Visit(expr->array()));
+  RECURSE_EXPRESSION(Visit(expr->index()));
+  RECURSE_EXPRESSION(Visit(expr->value()));
+}
+
 template <class Subclass>
 void AstTraversalVisitor<Subclass>::VisitEmptyParentheses(
     EmptyParentheses* expr) {

src/ast/ast.h

@@ -97,6 +97,7 @@ namespace internal {
   V(ResolvedProperty)           \
   V(RewritableExpression)       \
   V(Spread)                     \
+  V(StoreInArrayLiteral)        \
   V(SuperCallReference)         \
   V(SuperPropertyReference)     \
   V(ThisFunction)               \
@@ -1946,6 +1947,29 @@ class Spread final : public Expression {
   Expression* expression_;
 };
 
+// The StoreInArrayLiteral node corresponds to the StaInArrayLiteral bytecode.
+// It is used in the rewriting of destructuring assignments that contain an
+// array rest pattern.
+class StoreInArrayLiteral final : public Expression {
+ public:
+  Expression* array() const { return array_; }
+  Expression* index() const { return index_; }
+  Expression* value() const { return value_; }
+
+ private:
+  friend class AstNodeFactory;
+
+  StoreInArrayLiteral(Expression* array, Expression* index, Expression* value,
+                      int position)
+      : Expression(position, kStoreInArrayLiteral),
+        array_(array),
+        index_(index),
+        value_(value) {}
+
+  Expression* array_;
+  Expression* index_;
+  Expression* value_;
+};
 
 class Conditional final : public Expression {
  public:
@@ -3070,6 +3094,12 @@ class AstNodeFactory final BASE_EMBEDDED {
     return new (zone_) Spread(expression, pos, expr_pos);
   }
 
+  StoreInArrayLiteral* NewStoreInArrayLiteral(Expression* array,
+                                              Expression* index,
+                                              Expression* value, int pos) {
+    return new (zone_) StoreInArrayLiteral(array, index, value, pos);
+  }
+
   Conditional* NewConditional(Expression* condition,
                               Expression* then_expression,
                               Expression* else_expression,

src/ast/prettyprinter.cc

@@ -442,6 +442,11 @@ void CallPrinter::VisitSpread(Spread* node) {
   Print(")");
 }
 
+void CallPrinter::VisitStoreInArrayLiteral(StoreInArrayLiteral* node) {
+  Find(node->array());
+  Find(node->index());
+  Find(node->value());
+}
 
 void CallPrinter::VisitEmptyParentheses(EmptyParentheses* node) {
   UNREACHABLE();
@@ -1318,6 +1323,12 @@ void AstPrinter::VisitSpread(Spread* node) {
   Visit(node->expression());
 }
 
+void AstPrinter::VisitStoreInArrayLiteral(StoreInArrayLiteral* node) {
+  IndentedScope indent(this, "STORE IN ARRAY LITERAL", node->position());
+  PrintIndentedVisit("ARRAY", node->array());
+  PrintIndentedVisit("INDEX", node->index());
+  PrintIndentedVisit("VALUE", node->value());
+}
 
 void AstPrinter::VisitEmptyParentheses(EmptyParentheses* node) {
   IndentedScope indent(this, "()", node->position());

src/interpreter/bytecode-generator.cc

@@ -2405,6 +2405,19 @@ void BytecodeGenerator::BuildArrayLiteralSpread(Spread* spread, Register array,
   loop_builder.JumpToHeader(loop_depth_);
 }
 
+void BytecodeGenerator::VisitStoreInArrayLiteral(StoreInArrayLiteral* expr) {
+  builder()->SetExpressionAsStatementPosition(expr);
+  RegisterAllocationScope register_scope(this);
+  Register array = register_allocator()->NewRegister();
+  Register index = register_allocator()->NewRegister();
+  VisitForRegisterValue(expr->array(), array);
+  VisitForRegisterValue(expr->index(), index);
+  VisitForAccumulatorValue(expr->value());
+  builder()->StoreInArrayLiteral(
+      array, index,
+      feedback_index(feedback_spec()->AddStoreInArrayLiteralICSlot()));
+}
+
 void BytecodeGenerator::VisitVariableProxy(VariableProxy* proxy) {
   builder()->SetExpressionPosition(proxy);
   BuildVariableLoad(proxy->var(), proxy->hole_check_mode());
@@ -4017,6 +4030,8 @@ void BytecodeGenerator::VisitNaryArithmeticExpression(NaryOperation* expr) {
   }
 }
 
+// Note: the actual spreading is performed by the surrounding expression's
+// visitor.
 void BytecodeGenerator::VisitSpread(Spread* expr) { Visit(expr->expression()); }
 
 void BytecodeGenerator::VisitEmptyParentheses(EmptyParentheses* expr) {

src/parsing/pattern-rewriter.cc

@@ -577,16 +577,16 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
     // A spread can only occur as the last component.  It is not handled by
     // RecurseIntoSubpattern above.
 
-    // TODO(neis): Use IC instead of %AppendElement.
-
     // let array = [];
+    // let index = 0;
     // while (!done) {
     //   done = true;  // If .next, .done or .value throws, don't close.
     //   result = IteratorNext(iterator);
     //   if (!result.done) {
-    //     %AppendElement(array, result.value);
+    //     StoreInArrayLiteral(array, index, result.value);
     //     done = false;
     //   }
+    //   index++;
     // }
 
     // let array = [];
@@ -597,6 +597,10 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
           factory()->NewArrayLiteral(empty_exprs, kNoSourcePosition));
     }
 
+    // let index = 0;
+    Variable* index =
+        CreateTempVar(factory()->NewSmiLiteral(0, kNoSourcePosition));
+
     // done = true;
     Statement* set_done = factory()->NewExpressionStatement(
         factory()->NewAssignment(
@@ -611,19 +615,18 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
                                          result, IteratorType::kNormal, nopos),
         nopos);
 
-    // %AppendElement(array, result.value);
-    Statement* append_element;
+    // StoreInArrayLiteral(array, index, result.value);
+    Statement* store;
     {
-      auto args = new (zone()) ZoneList<Expression*>(2, zone());
-      args->Add(factory()->NewVariableProxy(array), zone());
-      args->Add(factory()->NewProperty(
-                    factory()->NewVariableProxy(result),
-                    factory()->NewStringLiteral(
-                        ast_value_factory()->value_string(), nopos),
-                    nopos),
-                zone());
-      append_element = factory()->NewExpressionStatement(
-          factory()->NewCallRuntime(Runtime::kAppendElement, args, nopos),
+      auto value = factory()->NewProperty(
+          factory()->NewVariableProxy(result),
+          factory()->NewStringLiteral(ast_value_factory()->value_string(),
+                                      nopos),
+          nopos);
+      store = factory()->NewExpressionStatement(
+          factory()->NewStoreInArrayLiteral(factory()->NewVariableProxy(array),
+                                            factory()->NewVariableProxy(index),
+                                            value, nopos),
           nopos);
     }
 
@@ -634,8 +637,8 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
             factory()->NewBooleanLiteral(false, nopos), nopos),
         nopos);
 
-    // if (!result.done) { #append_element; #unset_done }
-    Statement* maybe_append_and_unset_done;
+    // if (!result.done) { #store; #unset_done }
+    Statement* maybe_store_and_unset_done;
     {
       Expression* result_done =
           factory()->NewProperty(factory()->NewVariableProxy(result),
@@ -644,27 +647,38 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
                                  nopos);
 
       Block* then = factory()->NewBlock(2, true);
-      then->statements()->Add(append_element, zone());
+      then->statements()->Add(store, zone());
       then->statements()->Add(unset_done, zone());
 
-      maybe_append_and_unset_done = factory()->NewIfStatement(
+      maybe_store_and_unset_done = factory()->NewIfStatement(
           factory()->NewUnaryOperation(Token::NOT, result_done, nopos), then,
           factory()->NewEmptyStatement(nopos), nopos);
     }
 
+    // index++;
+    Statement* increment_index;
+    {
+      increment_index = factory()->NewExpressionStatement(
+          factory()->NewCountOperation(
+              Token::INC, false, factory()->NewVariableProxy(index), nopos),
+          nopos);
+    }
+
     // while (!done) {
     //   #set_done;
     //   #get_next;
-    //   #maybe_append_and_unset_done;
+    //   #maybe_store_and_unset_done;
+    //   #increment_index;
     // }
     WhileStatement* loop = factory()->NewWhileStatement(nullptr, nopos);
     {
       Expression* condition = factory()->NewUnaryOperation(
           Token::NOT, factory()->NewVariableProxy(done), nopos);
-      Block* body = factory()->NewBlock(3, true);
+      Block* body = factory()->NewBlock(4, true);
       body->statements()->Add(set_done, zone());
       body->statements()->Add(get_next, zone());
-      body->statements()->Add(maybe_append_and_unset_done, zone());
+      body->statements()->Add(maybe_store_and_unset_done, zone());
+      body->statements()->Add(increment_index, zone());
       loop->Initialize(condition, body);
     }
 
@@ -769,6 +783,7 @@ NOT_A_PATTERN(ResolvedProperty)
 NOT_A_PATTERN(ReturnStatement)
 NOT_A_PATTERN(SloppyBlockFunctionStatement)
 NOT_A_PATTERN(Spread)
+NOT_A_PATTERN(StoreInArrayLiteral)
 NOT_A_PATTERN(SuperPropertyReference)
 NOT_A_PATTERN(SuperCallReference)
 NOT_A_PATTERN(SwitchStatement)

src/runtime/runtime-object.cc

@@ -588,24 +588,6 @@ RUNTIME_FUNCTION(Runtime_AddElement) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_AppendElement) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(2, args.length());
-
-  CONVERT_ARG_HANDLE_CHECKED(JSArray, array, 0);
-  CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
-  CHECK(!value->IsTheHole(isolate));
-
-  uint32_t index;
-  CHECK(array->length()->ToArrayIndex(&index));
-
-  RETURN_FAILURE_ON_EXCEPTION(
-      isolate, JSObject::AddDataElement(array, index, value, NONE));
-  JSObject::ValidateElements(*array);
-  return *array;
-}
-
-
 RUNTIME_FUNCTION(Runtime_SetProperty) {
   HandleScope scope(isolate);
   DCHECK_EQ(4, args.length());

src/runtime/runtime.h

@@ -373,7 +373,6 @@ namespace internal {
   F(AddNamedProperty, 4, 1)                                     \
   F(SetProperty, 4, 1)                                          \
   F(AddElement, 3, 1)                                           \
-  F(AppendElement, 2, 1)                                        \
   F(DeleteProperty, 3, 1)                                       \
   F(ShrinkPropertyDictionary, 1, 1)                             \
   F(HasProperty, 2, 1)                                          \