Port FastCloneShallowArrayStub to Turbofan

src/code-stubs.cc

Index: src/code-stubs.cc
diff --git a/src/code-stubs.cc b/src/code-stubs.cc
index 016d40770208e395f8cd60aa7c845580333ff78a..741ad9dc26577db202364b2d517a0072918f5fa3 100644
--- a/src/code-stubs.cc
+++ b/src/code-stubs.cc
@@ -5146,15 +5146,6 @@ void NumberToStringStub::InitializeDescriptor(CodeStubDescriptor* descriptor) {
   descriptor->SetMissHandler(Runtime::kNumberToString);
 }
 
-
-void FastCloneShallowArrayStub::InitializeDescriptor(
-    CodeStubDescriptor* descriptor) {
-  FastCloneShallowArrayDescriptor call_descriptor(isolate());
-  descriptor->Initialize(
-      Runtime::FunctionForId(Runtime::kCreateArrayLiteralStubBailout)->entry);
-  descriptor->SetMissHandler(Runtime::kCreateArrayLiteralStubBailout);
-}
-
 void RegExpConstructResultStub::InitializeDescriptor(
     CodeStubDescriptor* descriptor) {
   descriptor->Initialize(
@@ -5663,6 +5654,175 @@ void FastCloneRegExpStub::GenerateAssembly(CodeStubAssembler* assembler) const {
       Generate(assembler, closure, literal_index, pattern, flags, context));
 }
 
+// static
+compiler::Node* FastCloneShallowArrayStub::NonEmptyShallowClone(
+    CodeStubAssembler* assembler, compiler::Node* boilerplate,
+    compiler::Node* boilerplate_map, compiler::Node* boilerplate_elements,
+    compiler::Node* allocation_site, compiler::Node* capacity,
+    ElementsKind kind) {
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::ParameterMode ParameterMode;
+
+  ParameterMode param_mode = CodeStubAssembler::SMI_PARAMETERS;
+
+  Node* length = assembler->LoadJSArrayLength(boilerplate);
+
+  if (assembler->Is64()) {

[rmcilroy, 2016/09/13 20:29:59]

Any idea why this is different on 64bit?

[klaasb, 2016/09/14 18:24:51]

This relates to the comment (now) at
https://cs.chromium.org/chromium/src/v8/src/code-stub-assembler.h?l=53
It's just passed on to ElementOffsetFromIndex.

We don't really need to untag the length, as we need it tagged to store it
anyway and we don't do any arithmetic with it anyway. The current state was the
last attempt to make it a bit more consistent, but I've changed it to have the
ParameterMode argument on AllocateUninitialized(/Empty)JSArray refer only to
capacity.

[rmcilroy, 2016/09/16 08:53:51]

Interesting. I wonder why it isn't faster to use SMIs on x64.
Sounds good, thanks.

+    length = assembler->SmiUntag(length);
+    capacity = assembler->SmiUntag(capacity);
+    param_mode = CodeStubAssembler::INTEGER_PARAMETERS;
+  }
+
+  Node* array = assembler->AllocateEmptyJSArray(
+      kind, boilerplate_map, length, allocation_site, param_mode, capacity);
+
+  int elements_offset =
+      JSArray::kSize +
+      (allocation_site != nullptr ? AllocationMemento::kSize : 0);
+  Node* elements = assembler->InnerAllocate(array, elements_offset);
+
+  assembler->Comment("copy elements header");
+  for (int offset = 0; offset < FixedArrayBase::kHeaderSize;
+       offset += kPointerSize) {
+    Node* value = assembler->LoadObjectField(boilerplate_elements, offset);
+    assembler->StoreObjectField(elements, offset, value);
+  }
+
+  assembler->Comment("copy boilerplate elements");
+  assembler->CopyFixedArrayElements(kind, boilerplate_elements, elements,
+                                    length, SKIP_WRITE_BARRIER, param_mode);
+  assembler->IncrementCounter(
+      assembler->isolate()->counters()->inlined_copied_elements(), 1);
+
+  return array;
+}
+
+// static
+compiler::Node* FastCloneShallowArrayStub::Generate(
+    CodeStubAssembler* assembler, compiler::Node* closure,
+    compiler::Node* literal_index, compiler::Node* constant_elements,
+    compiler::Node* context, AllocationSiteMode allocation_site_mode) {
+  typedef CodeStubAssembler::Label Label;
+  typedef CodeStubAssembler::Variable Variable;
+  typedef compiler::Node Node;
+
+  Label call_runtime(assembler, Label::kDeferred), zero_capacity(assembler),
+      fixed_cow(assembler), fixed(assembler), return_result(assembler);
+  Variable result(assembler, MachineRepresentation::kTagged);
+
+  Node* undefined = assembler->UndefinedConstant();
+
+  assembler->Comment("load literals array");
+  Node* literals_array =
+      assembler->LoadObjectField(closure, JSFunction::kLiteralsOffset);
+  assembler->Comment("load allocation site");
+  Node* allocation_site = assembler->LoadFixedArrayElement(
+      literals_array, literal_index,
+      LiteralsArray::kFirstLiteralIndex * kPointerSize,
+      CodeStubAssembler::SMI_PARAMETERS);
+
+  assembler->GotoIf(assembler->WordEqual(allocation_site, undefined),
+                    &call_runtime);
+
+  assembler->Comment("load boilerplate");
+  Node* boilerplate = assembler->LoadObjectField(
+      allocation_site, AllocationSite::kTransitionInfoOffset);
+  Node* boilerplate_map = assembler->LoadMap(boilerplate);
+  Node* boilerplate_elements = assembler->LoadElements(boilerplate);
+  assembler->Comment("load capacity");

[rmcilroy, 2016/09/13 20:29:59]

not sure if these "load.." comments add much. I'm easy either way though.

[klaasb, 2016/09/14 18:24:52]

Yes, they were mostly for my benefit when looking at the disassembly. Removed.

+  Node* capacity = assembler->LoadFixedArrayBaseLength(boilerplate_elements);
+  allocation_site =
+      allocation_site_mode == TRACK_ALLOCATION_SITE ? allocation_site : nullptr;
+
+  Node* zero = assembler->SmiConstant(Smi::FromInt(0));
+  assembler->GotoIf(assembler->SmiEqual(capacity, zero), &zero_capacity);
+
+  Node* elements_map = assembler->LoadMap(boilerplate_elements);
+  assembler->GotoIf(
+      assembler->WordEqual(elements_map, assembler->FixedCowArrayMapConstant()),
+      &fixed_cow);
+
+  assembler->GotoIf(
+      assembler->WordEqual(elements_map, assembler->FixedArrayMapConstant()),
+      &fixed);
+  {
+    assembler->Comment("non fixed path");

[rmcilroy, 2016/09/13 20:29:59]

I don't think this is non-fixed, it's a fixed double arrays isn't it? Should it
instead be "fast double elements path"? Is there anything we can "dcheck" (in
FLAG_debug_code) that the elements_map equals here?

[klaasb, 2016/09/14 18:24:52]

Done.

+    Node* array = NonEmptyShallowClone(assembler, boilerplate, boilerplate_map,
+                                       boilerplate_elements, allocation_site,
+                                       capacity, FAST_DOUBLE_ELEMENTS);
+    result.Bind(array);
+    assembler->Goto(&return_result);
+  }
+
+  assembler->Bind(&fixed);

[rmcilroy, 2016/09/13 20:29:59]

ditto - should this be fast_elements (and similar comment on Comment(...)

[klaasb, 2016/09/14 18:24:51]

Done.

+  {
+    assembler->Comment("fixed path");
+    Node* array = NonEmptyShallowClone(assembler, boilerplate, boilerplate_map,
+                                       boilerplate_elements, allocation_site,
+                                       capacity, FAST_ELEMENTS);
+    result.Bind(array);
+    assembler->Goto(&return_result);
+  }
+
+  assembler->Bind(&fixed_cow);

[rmcilroy, 2016/09/13 20:29:59]

fixed_cow_elements

[klaasb, 2016/09/14 18:24:51]

Renamed to cow_elements, to mirror fast_elements.

+  {
+    assembler->Comment("fixed cow path");
+    Node* length = assembler->LoadJSArrayLength(boilerplate);
+    Node* array = assembler->AllocateEmptyJSArray(
+        FAST_ELEMENTS, boilerplate_map, length, allocation_site,
+        CodeStubAssembler::SMI_PARAMETERS);
+
+    assembler->StoreObjectField(array, JSObject::kElementsOffset,
+                                boilerplate_elements);
+    result.Bind(array);
+    assembler->Goto(&return_result);
+  }
+
+  assembler->Bind(&zero_capacity);
+  {
+    assembler->Comment("zero capacity path");
+    Node* array = assembler->AllocateEmptyJSArray(
+        FAST_ELEMENTS, boilerplate_map, zero, allocation_site,
+        CodeStubAssembler::SMI_PARAMETERS);
+
+    assembler->StoreObjectFieldRoot(array, JSObject::kElementsOffset,
+                                    Heap::kEmptyFixedArrayRootIndex);

[rmcilroy, 2016/09/13 20:29:59]

Would it be worth merging the allocations for fixed cow and zero capacity (i.e.,
having length and the elements object be Variables set in the branches, then
merging to a common allocation branch which does the allocation?

[klaasb, 2016/09/14 18:24:51]

I thought about that before, but seem to have forgotten to try it. It certainly
reduces code size and removes a load of a spilled value. Done.

Sharing more code at runtime for the other two branches is not easily done,
however.

[rmcilroy, 2016/09/16 08:53:51]

Yeah I realized the other branches couldn't be shared quite as easily. Thanks
for merging this bit.

+    result.Bind(array);
+    assembler->Goto(&return_result);
+  }
+
+  assembler->Bind(&call_runtime);
+  {
+    assembler->Comment("call runtime");
+    Node* flags = assembler->SmiConstant(
+        Smi::FromInt(ArrayLiteral::kShallowElements |
+                     (allocation_site_mode == TRACK_ALLOCATION_SITE
+                          ? 0
+                          : ArrayLiteral::kDisableMementos)));
+    Node* array =
+        assembler->CallRuntime(Runtime::kCreateArrayLiteral, context, closure,
+                               literal_index, constant_elements, flags);
+    result.Bind(array);
+    assembler->Goto(&return_result);
+  }
+
+  assembler->Bind(&return_result);
+  return result.value();
+}
+
+void FastCloneShallowArrayStub::GenerateAssembly(
+    CodeStubAssembler* assembler) const {
+  typedef compiler::Node Node;
+  Node* closure = assembler->Parameter(Descriptor::kClosure);
+  Node* literal_index = assembler->Parameter(Descriptor::kLiteralIndex);
+  Node* constant_elements = assembler->Parameter(Descriptor::kConstantElements);
+  Node* context = assembler->Parameter(Descriptor::kContext);
+
+  assembler->Return(Generate(assembler, closure, literal_index,
+                             constant_elements, context,
+                             allocation_site_mode()));
+}
+
 void CreateAllocationSiteStub::GenerateAheadOfTime(Isolate* isolate) {
   CreateAllocationSiteStub stub(isolate);
   stub.GetCode();
@@ -5885,7 +6045,7 @@ void ArrayNoArgumentConstructorStub::GenerateAssembly(
                             : nullptr;
   Node* array_map =
       assembler->LoadJSArrayElementsMap(elements_kind(), native_context);
-  Node* array = assembler->AllocateJSArray(
+  Node* array = assembler->AllocateFilledJSArray(
       elements_kind(), array_map,
       assembler->IntPtrConstant(JSArray::kPreallocatedArrayElements),
       assembler->IntPtrConstant(0), allocation_site);
@@ -5898,7 +6058,7 @@ void InternalArrayNoArgumentConstructorStub::GenerateAssembly(
   Node* array_map =
       assembler->LoadObjectField(assembler->Parameter(Descriptor::kFunction),
                                  JSFunction::kPrototypeOrInitialMapOffset);
-  Node* array = assembler->AllocateJSArray(
+  Node* array = assembler->AllocateFilledJSArray(
       elements_kind(), array_map,
       assembler->IntPtrConstant(JSArray::kPreallocatedArrayElements),
       assembler->IntPtrConstant(0), nullptr);
@@ -5952,7 +6112,7 @@ void SingleArgumentConstructorCommon(CodeStubAssembler* assembler,
 
   assembler->Bind(&small_smi_size);
   {
-    Node* array = assembler->AllocateJSArray(
+    Node* array = assembler->AllocateFilledJSArray(
         elements_kind, array_map, size, size,
         mode == DONT_TRACK_ALLOCATION_SITE ? nullptr : allocation_site,
         CodeStubAssembler::SMI_PARAMETERS);