[stubs] Cleanup FixedArray copying builtin

src/builtins/builtins-internal.cc

Index: src/builtins/builtins-internal.cc
diff --git a/src/builtins/builtins-internal.cc b/src/builtins/builtins-internal.cc
index 03728764c6ace49613316280896869472f48fbe1..5ee294f5dcf1861f29f98912b8c902ba85c35a37 100644
--- a/src/builtins/builtins-internal.cc
+++ b/src/builtins/builtins-internal.cc
@@ -56,124 +56,40 @@ void Builtins::Generate_StackCheck(MacroAssembler* masm) {
 void Builtins::Generate_CopyFixedArray(CodeStubAssembler* assembler) {
   typedef CodeStubAssembler::Label Label;
   typedef compiler::Node Node;
-  typedef CodeStubAssembler::Variable Variable;
   typedef CopyFixedArrayDescriptor Descriptor;
 
   Node* source = assembler->Parameter(Descriptor::kSource);
-
-  // Load the {source} length.
-  Node* source_length_tagged =
-      assembler->LoadObjectField(source, FixedArray::kLengthOffset);
-  Node* source_length = assembler->SmiToWord(source_length_tagged);
-
-  // Compute the size of {source} in bytes.
-  Node* source_size = assembler->IntPtrAdd(
-      assembler->WordShl(source_length,
-                         assembler->IntPtrConstant(kPointerSizeLog2)),
-      assembler->IntPtrConstant(FixedArray::kHeaderSize));
+  CodeStubAssembler::ParameterMode mode =
+      assembler->Is64() ? CodeStubAssembler::INTEGER_PARAMETERS
+                        : CodeStubAssembler::SMI_PARAMETERS;
+  Node* length = (mode == CodeStubAssembler::INTEGER_PARAMETERS)
+                     ? assembler->LoadAndUntagFixedArrayBaseLength(source)
+                     : assembler->LoadFixedArrayBaseLength(source);
 
   // Check if we can allocate in new space.
+  ElementsKind kind = FAST_ELEMENTS;
+  int max_elements = FixedArrayBase::GetMaxLengthForNewSpaceAllocation(kind);
   Label if_newspace(assembler), if_oldspace(assembler);
-  assembler->Branch(assembler->UintPtrLessThan(
-                        source_size, assembler->IntPtrConstant(
-                                         Page::kMaxRegularHeapObjectSize)),
-                    &if_newspace, &if_oldspace);
+  assembler->Branch(
+      assembler->UintPtrLessThan(
+          length, assembler->IntPtrOrSmiConstant(max_elements, mode)),
+      &if_newspace, &if_oldspace);
 
   assembler->Bind(&if_newspace);
   {
-    // Allocate the targeting FixedArray in new space.
-    Node* target = assembler->Allocate(source_size);
-    assembler->StoreMapNoWriteBarrier(
-        target, assembler->LoadRoot(Heap::kFixedArrayMapRootIndex));
-    assembler->StoreObjectFieldNoWriteBarrier(target, FixedArray::kLengthOffset,
-                                              source_length_tagged);
-
-    // Compute the limit.
-    Node* limit = assembler->IntPtrSub(
-        source_size, assembler->IntPtrConstant(kHeapObjectTag));
-
-    // Copy the {source} to the {target}.
-    Variable var_offset(assembler, MachineType::PointerRepresentation());
-    Label loop(assembler, &var_offset), done_loop(assembler);
-    var_offset.Bind(
-        assembler->IntPtrConstant(FixedArray::kHeaderSize - kHeapObjectTag));
-    assembler->Goto(&loop);
-    assembler->Bind(&loop);
-    {
-      // Determine the current {offset}.
-      Node* offset = var_offset.value();
-
-      // Check if we are done.
-      assembler->GotoUnless(assembler->UintPtrLessThan(offset, limit),
-                            &done_loop);
-
-      // Load the value from {source}.
-      Node* value = assembler->Load(MachineType::AnyTagged(), source, offset);
-
-      // Store the {value} to the {target} without a write barrier, since we
-      // know that the {target} is allocated in new space.
-      assembler->StoreNoWriteBarrier(MachineRepresentation::kTagged, target,
-                                     offset, value);
-
-      // Increment {offset} and continue.
-      var_offset.Bind(assembler->IntPtrAdd(
-          offset, assembler->IntPtrConstant(kPointerSize)));
-      assembler->Goto(&loop);
-    }
-
-    assembler->Bind(&done_loop);
-    assembler->Return(target);
+    Node* destination = assembler->AllocateFixedArray(kind, length, mode);
+    assembler->CopyFixedArrayElements(kind, source, destination, length,
+                                      SKIP_WRITE_BARRIER, mode);
+    assembler->Return(destination);
   }
 
   assembler->Bind(&if_oldspace);
   {
-    // Allocate the targeting FixedArray in old space
-    // (maybe even in large object space).
-    Node* flags = assembler->SmiConstant(
-        Smi::FromInt(AllocateDoubleAlignFlag::encode(false) |
-                     AllocateTargetSpace::encode(AllocationSpace::OLD_SPACE)));
-    Node* source_size_tagged = assembler->SmiFromWord(source_size);
-    Node* target = assembler->CallRuntime(Runtime::kAllocateInTargetSpace,
-                                          assembler->NoContextConstant(),
-                                          source_size_tagged, flags);
-    assembler->StoreMapNoWriteBarrier(
-        target, assembler->LoadRoot(Heap::kFixedArrayMapRootIndex));
-    assembler->StoreObjectFieldNoWriteBarrier(target, FixedArray::kLengthOffset,
-                                              source_length_tagged);
-
-    // Compute the limit.
-    Node* limit = assembler->IntPtrSub(
-        source_size, assembler->IntPtrConstant(kHeapObjectTag));
-
-    // Copy the {source} to the {target}.
-    Variable var_offset(assembler, MachineType::PointerRepresentation());
-    Label loop(assembler, &var_offset), done_loop(assembler);
-    var_offset.Bind(
-        assembler->IntPtrConstant(FixedArray::kHeaderSize - kHeapObjectTag));
-    assembler->Goto(&loop);
-    assembler->Bind(&loop);
-    {
-      // Determine the current {offset}.
-      Node* offset = var_offset.value();
-
-      // Check if we are done.
-      assembler->GotoUnless(assembler->UintPtrLessThan(offset, limit),
-                            &done_loop);
-
-      // Load the value from {source}.
-      Node* value = assembler->Load(MachineType::AnyTagged(), source, offset);
-
-      // Store the {value} to the {target} with a proper write barrier.
-      assembler->Store(MachineRepresentation::kTagged, target, offset, value);
-
-      // Increment {offset} and continue.
-      var_offset.Bind(assembler->IntPtrAdd(
-          offset, assembler->IntPtrConstant(kPointerSize)));
-      assembler->Goto(&loop);
-    }
-
-    assembler->Bind(&done_loop);
-    assembler->Return(target);
+    Node* destination = assembler->AllocateFixedArray(

[Benedikt Meurer, 2016/08/08 03:46:06]

Looking into this again, I don't think this is correct wrt. to what the GC
expects, as we'd allocate FixedArrays that are larger than
Page::kMaxRegularHeapObjectSize in old space here if they fit, although those
should end up in large object space instead. Please check back with GC folks.

[danno, 2016/08/08 14:19:15]

Talked to the GC folks. Turns out that this will go down the slow path since it
won't fit into an old-space page, and that will end up in the runtime. For
better or worse, kPretenured is the way to specify old space for large
allocations in the runtime call, the decision is made based on size inside the
runtime routine. So if it doesn't fit in the page, it will be in old space...
and the code here should be correct, albeit ugly.

+        kind, length, mode, compiler::CodeAssembler::kPretenured);
+    assembler->CopyFixedArrayElements(kind, source, destination, length,
+                                      UPDATE_WRITE_BARRIER, mode);
+    assembler->Return(destination);
   }
 }