[stubs] Port builtin for Array.push fast-case from Crankshaft to TF

src/builtins/builtins-array.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/builtins/builtins.h"
 #include "src/builtins/builtins-utils.h"
 
 #include "src/code-factory.h"
 #include "src/contexts.h"
 #include "src/elements.h"
@@ skipping 132 matching lines @@
   HandleScope handleScope(isolate);
   int argc = args.length() - 1;
   ScopedVector<Handle<Object>> argv(argc);
   for (int i = 0; i < argc; ++i) {
     argv[i] = args.at<Object>(i + 1);
   }
   RETURN_RESULT_OR_FAILURE(
       isolate,
       Execution::Call(isolate, function, args.receiver(), argc, argv.start()));
 }
+}  // namespace
 
-Object* DoArrayPush(Isolate* isolate, BuiltinArguments args) {
+BUILTIN(ArrayPush) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
   if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1)) {
     return CallJsIntrinsic(isolate, isolate->array_push(), args);
   }
   // Fast Elements Path
   int to_add = args.length() - 1;
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   int len = Smi::cast(array->length())->value();
   if (to_add == 0) return Smi::FromInt(len);
 
   // Currently fixed arrays cannot grow too big, so we should never hit this.
   DCHECK_LE(to_add, Smi::kMaxValue - Smi::cast(array->length())->value());
 
   if (JSArray::HasReadOnlyLength(array)) {
     return CallJsIntrinsic(isolate, isolate->array_push(), args);
   }
 
   ElementsAccessor* accessor = array->GetElementsAccessor();
   int new_length = accessor->Push(array, &args, to_add);
   return Smi::FromInt(new_length);
 }
-}  // namespace
 
-BUILTIN(ArrayPush) { return DoArrayPush(isolate, args); }
+void Builtins::Generate_FastArrayPush(compiler::CodeAssemblerState* state) {
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+  CodeStubAssembler assembler(state);
+  Label runtime(&assembler, Label::kDeferred);
 
-// TODO(verwaest): This is a temporary helper until the FastArrayPush stub can
-// tailcall to the builtin directly.
-RUNTIME_FUNCTION(Runtime_ArrayPush) {
-  DCHECK_EQ(2, args.length());
-  Arguments* incoming = reinterpret_cast<Arguments*>(args[0]);
-  // Rewrap the arguments as builtins arguments.
-  int argc = incoming->length() + BuiltinArguments::kNumExtraArgsWithReceiver;
-  BuiltinArguments caller_args(argc, incoming->arguments() + 1);
-  return DoArrayPush(isolate, caller_args);
+  Node* argc = assembler.Parameter(1);
+  Node* context = assembler.Parameter(2);
+  Node* new_target = assembler.Parameter(0);
+
+  CodeStubArguments args(&assembler, argc);
+  Node* receiver = args.GetReceiver();
+
+  Label fast(&assembler);
+  assembler.BranchIfFastJSArray(
+      receiver, context, CodeStubAssembler::FastJSArrayAccessMode::ANY_ACCESS,
+      &fast, &runtime);
+
+  assembler.Bind(&fast);
+  {

[Jakob Kummerow, 2016/11/23 17:17:05]

nit: this identation level doesn't add anything (except whitespace). I'd just
drop it; if you want to keep it, then at least you should pull the
"Bind(&runtime)" section out of it.

[danno, 2016/11/29 14:39:59]

Done.

+    // Disallow pushing onto prototypes. It might be the JSArray prototype.
+    // Disallow pushing onto non-extensible objects.
+    assembler.Comment("Disallow pushing onto prototypes");
+    Node* map = assembler.LoadMap(receiver);
+    Node* bit_field2 = assembler.LoadObjectField(map, Map::kBitField2Offset,

[Jakob Kummerow, 2016/11/23 17:17:05]

... = assembler.LoadMapBitField2(map);

[danno, 2016/11/29 14:39:59]

Done.

+                                                 MachineType::Uint8());
+    int mask = static_cast<int>(Map::IsPrototypeMapBits::kMask) |
+               (1 << Map::kIsExtensible);
+    Node* test = assembler.Word32And(bit_field2, assembler.Int32Constant(mask));
+    assembler.GotoIf(
+        assembler.Word32NotEqual(
+            test, assembler.Int32Constant(1 << Map::kIsExtensible)),
+        &runtime);
+
+    // Disallow pushing onto arrays in dictionary named property mode. We need
+    // to figure out whether the length property is still writable.
+    assembler.Comment(
+        "Disallow pushing onto arrays in dictionary named property mode");
+    Node* bit_field3 = assembler.LoadObjectField(map, Map::kBitField3Offset,

[Jakob Kummerow, 2016/11/23 17:17:05]

... = assembler.LoadMapBitField3(map);

[danno, 2016/11/29 14:39:59]

Done.

+                                                 MachineType::Uint32());
+    mask = static_cast<int>(Map::DictionaryMap::kMask);
+    Node* mask_node = assembler.Int32Constant(mask);
+    test = assembler.Word32And(bit_field3, mask_node);
+    assembler.GotoIf(assembler.Word32Equal(test, mask_node), &runtime);

[Jakob Kummerow, 2016/11/23 17:17:05]

assembler.GotoIf(assembler.IsSetWord32<Map::DictionaryMap>(bit_field3),
&runtime);

+
+    // Check whether the length property is writable. The length property is the

[Jakob Kummerow, 2016/11/23 17:17:05]

Most excellent point. The KeyedStoreGeneric stub has never done this. I've filed
https://bugs.chromium.org/p/v8/issues/detail?id=5669.

[danno, 2016/11/29 14:39:59]

Done.

+    // only default named property on arrays. It's nonconfigurable, hence is
+    // guaranteed to stay the first property.
+    Node* descriptors = assembler.LoadObjectField(map, Map::kDescriptorsOffset);

[Jakob Kummerow, 2016/11/23 17:17:05]

... = assembler.LoadMapDescriptors(map);

[danno, 2016/11/29 14:39:59]

Done.

+    Node* details = assembler.LoadFixedArrayElement(
+        descriptors,
+        assembler.Int32Constant(DescriptorArray::ToDetailsIndex(0)));
+    mask = READ_ONLY << PropertyDetails::AttributesField::kShift;
+    mask_node = assembler.SmiConstant(Smi::FromInt(mask));

[Jakob Kummerow, 2016/11/23 17:17:05]

nit: s/Smi::FromInt(mask)/mask/

[danno, 2016/11/29 14:39:59]

Done.

+    test = assembler.WordAnd(details, mask_node);
+    assembler.GotoIf(assembler.WordEqual(test, mask_node), &runtime);
+
+    Variable arg_index(&assembler, MachineType::PointerRepresentation());
+    arg_index.Bind(assembler.IntPtrConstant(0));
+    Node* kind = assembler.DecodeWord32<Map::ElementsKindBits>(bit_field2);
+    CodeStubAssembler::VariableList vars({&arg_index}, assembler.zone());
+    Label default_label(&assembler, vars);

[Jakob Kummerow, 2016/11/23 17:17:05]

nit: why not just s/vars/&arg_index/ here and once more in line 244?

[danno, 2016/11/29 14:39:59]

Done.

+    Label smi_transition(&assembler);
+    Label object_push_pre(&assembler);
+    Label object_push(&assembler, vars);
+    Label double_push(&assembler);
+    Label double_transition(&assembler);
+
+    assembler.GotoIf(
+        assembler.IntPtrGreaterThan(
+            kind, assembler.IntPtrConstant(FAST_HOLEY_SMI_ELEMENTS)),
+        &object_push_pre);
+
+    {
+      Node* new_length =
+          assembler.BuildAppendJSArray(FAST_SMI_ELEMENTS, context, receiver,
+                                       args, arg_index, &smi_transition);
+      args.PopAndReturn(new_length);
+    }
+
+    // If the argument is not a smi, then use a heavyweight SetProperty to
+    // transition the array for only the single next element. If the argument is
+    // a smi, the failure is due to some other reason and we should fall back on
+    // the most generic implementation for the rest of the array.
+    assembler.Bind(&smi_transition);
+    {
+      Node* arg = args.AtIndex(arg_index.value());
+      assembler.GotoIf(assembler.TaggedIsSmi(arg), &default_label);
+      Node* length = assembler.LoadJSArrayLength(receiver);
+      // TODO(danno): Use the KeyedStoreGeneric stub here when possible,
+      // calling into the runtime to do the elements transition is overkill.
+      assembler.CallRuntime(Runtime::kSetProperty, context, receiver, length,
+                            arg, assembler.SmiConstant(Smi::FromInt(STRICT)));

[Jakob Kummerow, 2016/11/23 17:17:05]

nit: s/Smi::FromInt(STRICT)/STRICT/

[danno, 2016/11/29 14:39:59]

Done.

+      assembler.Increment(arg_index);
+      assembler.GotoIfNotNumber(arg, &object_push);
+      assembler.Goto(&double_push);
+    }
+
+    assembler.Bind(&object_push_pre);
+    assembler.Branch(assembler.IntPtrGreaterThan(
+                         kind, assembler.IntPtrConstant(FAST_HOLEY_ELEMENTS)),
+                     &double_push, &object_push);
+
+    assembler.Bind(&object_push);
+    {
+      Node* new_length = assembler.BuildAppendJSArray(
+          FAST_ELEMENTS, context, receiver, args, arg_index, &default_label);
+      args.PopAndReturn(new_length);
+    }
+
+    assembler.Bind(&double_push);
+    {
+      Node* new_length =
+          assembler.BuildAppendJSArray(FAST_DOUBLE_ELEMENTS, context, receiver,
+                                       args, arg_index, &double_transition);
+      args.PopAndReturn(new_length);
+    }
+
+    // If the argument is not a double, then use a heavyweight SetProperty to
+    // transition the array for only the single next element. If the argument is
+    // a double, the failure is due to some other reason and we should fall back
+    // on the most generic implementation for the rest of the array.
+    assembler.Bind(&double_transition);
+    {
+      Node* arg = args.AtIndex(arg_index.value());
+      assembler.GotoIfNumber(arg, &default_label);
+      Node* length = assembler.LoadJSArrayLength(receiver);
+      // TODO(danno): Use the KeyedStoreGeneric stub here when possible,
+      // calling into the runtime to do the elements transition is overkill.
+      assembler.CallRuntime(Runtime::kSetProperty, context, receiver, length,
+                            arg, assembler.SmiConstant(Smi::FromInt(STRICT)));

[Jakob Kummerow, 2016/11/23 17:17:05]

nit: s/Smi::FromInt(STRICT)/STRICT/

[danno, 2016/11/29 14:39:59]

Done.

+      assembler.Increment(arg_index);
+      assembler.Goto(&object_push);
+    }
+
+    // Fallback that stores un-processed arguments using the full, heavyweight
+    // SetProperty machinery.
+    assembler.Bind(&default_label);
+    {
+      args.ForEach(
+          [receiver, context, &arg_index](CodeStubAssembler* assembler,
+                                          Node* arg) {
+            Node* length = assembler->LoadJSArrayLength(receiver);
+            assembler->CallRuntime(
+                Runtime::kSetProperty, context, receiver, length, arg,
+                assembler->SmiConstant(Smi::FromInt(STRICT)));

[Jakob Kummerow, 2016/11/23 17:17:05]

nit: s/Smi::FromInt(STRICT)/STRICT/

[danno, 2016/11/29 14:39:59]

Done.

+          },
+          arg_index.value());
+      args.PopAndReturn(assembler.LoadJSArrayLength(receiver));
+    }
+
+    assembler.Bind(&runtime);
+    {
+      Node* target =
+          assembler.LoadFromFrame(StandardFrameConstants::kFunctionOffset,
+                                  MachineType::TaggedPointer());
+      assembler.TailCallStub(CodeFactory::ArrayPush(assembler.isolate()),
+                             context, target, new_target, argc);
+    }
+  }
 }
 
 BUILTIN(ArrayPop) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
   if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, nullptr, 0)) {
     return CallJsIntrinsic(isolate, isolate->array_pop(), args);
   }
 
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
@@ skipping 1086 matching lines @@
   Label init_k(&assembler), return_true(&assembler), return_false(&assembler),
       call_runtime(&assembler);
 
   Label init_len(&assembler);
 
   index_var.Bind(intptr_zero);
   len_var.Bind(intptr_zero);
 
   // Take slow path if not a JSArray, if retrieving elements requires
   // traversing prototype, or if access checks are required.
-  assembler.BranchIfFastJSArray(array, context, &init_len, &call_runtime);
+  assembler.BranchIfFastJSArray(
+      array, context, CodeStubAssembler::FastJSArrayAccessMode::INBOUNDS_READ,
+      &init_len, &call_runtime);
 
   assembler.Bind(&init_len);
   {
     // Handle case where JSArray length is not an Smi in the runtime
     Node* len = assembler.LoadObjectField(array, JSArray::kLengthOffset);
     assembler.GotoUnless(assembler.TaggedIsSmi(len), &call_runtime);
 
     len_var.Bind(assembler.SmiToWord(len));
     assembler.Branch(assembler.WordEqual(len_var.value(), intptr_zero),
                      &return_false, &init_k);
@@ skipping 420 matching lines @@
   Label init_k(&assembler), return_found(&assembler),
       return_not_found(&assembler), call_runtime(&assembler);
 
   Label init_len(&assembler);
 
   index_var.Bind(intptr_zero);
   len_var.Bind(intptr_zero);
 
   // Take slow path if not a JSArray, if retrieving elements requires
   // traversing prototype, or if access checks are required.
-  assembler.BranchIfFastJSArray(array, context, &init_len, &call_runtime);
+  assembler.BranchIfFastJSArray(
+      array, context, CodeStubAssembler::FastJSArrayAccessMode::INBOUNDS_READ,
+      &init_len, &call_runtime);
 
   assembler.Bind(&init_len);
   {
     // Handle case where JSArray length is not an Smi in the runtime
     Node* len = assembler.LoadObjectField(array, JSArray::kLengthOffset);
     assembler.GotoUnless(assembler.TaggedIsSmi(len), &call_runtime);
 
     len_var.Bind(assembler.SmiToWord(len));
     assembler.Branch(assembler.WordEqual(len_var.value(), intptr_zero),
                      &return_not_found, &init_k);
@@ skipping 865 matching lines @@
         Runtime::kThrowIncompatibleMethodReceiver, context,
         assembler.HeapConstant(assembler.factory()->NewStringFromAsciiChecked(
             "Array Iterator.prototype.next", TENURED)),
         iterator);
     assembler.Return(result);
   }
 }
 
 }  // namespace internal
 }  // namespace v8