Adding ElementsAccessor::Concat

- Moving parts of ArrayConcat from builtins.cc to the ElementsAccessor
- Removing ArrayConcat Runtime Function

BUG=v8:4317
LOG=N

Review URL: https://codereview.chromium.org/1330483003

Cr-Commit-Position: refs/heads/master@{#30619}

src/array.js

@@ -522,24 +522,6 @@ function ArrayPush() {
 }
 
 
-// Returns an array containing the array elements of the object followed
-// by the array elements of each argument in order. See ECMA-262,
-// section 15.4.4.7.
-function ArrayConcatJS(arg1) {  // length == 1
-  CHECK_OBJECT_COERCIBLE(this, "Array.prototype.concat");
-
-  var array = TO_OBJECT(this);
-  var arg_count = %_ArgumentsLength();
-  var arrays = new InternalArray(1 + arg_count);
-  arrays[0] = array;
-  for (var i = 0; i < arg_count; i++) {
-    arrays[i + 1] = %_Arguments(i);
-  }
-
-  return %ArrayConcat(arrays);
-}
-
-
 // For implementing reverse() on large, sparse arrays.
 function SparseReverse(array, len) {
   var keys = GetSortedArrayKeys(array, %GetArrayKeys(array, len));
@@ -1642,7 +1624,6 @@ utils.InstallFunctions(GlobalArray.prototype, DONT_ENUM, [
   "join", getFunction("join", ArrayJoin),
   "pop", getFunction("pop", ArrayPop),
   "push", getFunction("push", ArrayPush, 1),
-  "concat", getFunction("concat", ArrayConcatJS, 1),
   "reverse", getFunction("reverse", ArrayReverse),
   "shift", getFunction("shift", ArrayShift),
   "unshift", getFunction("unshift", ArrayUnshift, 1),
@@ -1666,7 +1647,6 @@ utils.InstallFunctions(GlobalArray.prototype, DONT_ENUM, [
 // exposed to user code.
 // Adding only the functions that are actually used.
 utils.SetUpLockedPrototype(InternalArray, GlobalArray(), [
-  "concat", getFunction("concat", ArrayConcatJS),
   "indexOf", getFunction("indexOf", ArrayIndexOf),
   "join", getFunction("join", ArrayJoin),
   "pop", getFunction("pop", ArrayPop),
@@ -1707,7 +1687,6 @@ utils.Export(function(to) {
 });
 
 %InstallToContext([
-  "array_concat", ArrayConcatJS,
   "array_pop", ArrayPop,
   "array_push", ArrayPush,
   "array_shift", ArrayShift,

src/bootstrapper.cc

@@ -2331,6 +2331,40 @@ bool Genesis::InstallNatives(ContextType context_type) {
     to_primitive->shared()->set_length(1);
   }
 
+  // Install Array.prototype.concat
+  {
+    Handle<JSFunction> array_constructor(native_context()->array_function());
+    Handle<JSObject> proto(JSObject::cast(array_constructor->prototype()));
+    Handle<JSFunction> concat =
+        InstallFunction(proto, "concat", JS_OBJECT_TYPE, JSObject::kHeaderSize,
+                        MaybeHandle<JSObject>(), Builtins::kArrayConcat);
+
+    // Make sure that Array.prototype.concat appears to be compiled.
+    // The code will never be called, but inline caching for call will
+    // only work if it appears to be compiled.
+    concat->shared()->DontAdaptArguments();
+    DCHECK(concat->is_compiled());
+    // Set the lengths for the functions to satisfy ECMA-262.
+    concat->shared()->set_length(1);
+  }
+
+  // Install InternalArray.prototype.concat
+  {
+    Handle<JSFunction> array_constructor(
+        native_context()->internal_array_function());
+    Handle<JSObject> proto(JSObject::cast(array_constructor->prototype()));
+    Handle<JSFunction> concat =
+        InstallFunction(proto, "concat", JS_OBJECT_TYPE, JSObject::kHeaderSize,
+                        MaybeHandle<JSObject>(), Builtins::kArrayConcat);
+
+    // Make sure that InternalArray.prototype.concat appears to be compiled.
+    // The code will never be called, but inline caching for call will
+    // only work if it appears to be compiled.
+    concat->shared()->DontAdaptArguments();
+    DCHECK(concat->is_compiled());
+    // Set the lengths for the functions to satisfy ECMA-262.
+    concat->shared()->set_length(1);
+  }
   // Install Function.prototype.call and apply.
   {
     Handle<String> key = factory()->Function_string();

src/elements.cc

@@ -2350,6 +2350,69 @@ void ElementsAccessor::TearDown() {
 }
 
 
+Handle<JSArray> ElementsAccessor::Concat(Isolate* isolate, Arguments* args,
+                                         uint32_t concat_size) {
+  int result_len = 0;
+  ElementsKind elements_kind = GetInitialFastElementsKind();
+  bool has_double = false;
+  {
+    DisallowHeapAllocation no_gc;
+    // Iterate through all the arguments performing checks
+    // and calculating total length.
+    bool is_holey = false;
+    for (uint32_t i = 0; i < concat_size; i++) {
+      Object* arg = (*args)[i];
+      int len = Smi::cast(JSArray::cast(arg)->length())->value();
+
+      // We shouldn't overflow when adding another len.
+      const int kHalfOfMaxInt = 1 << (kBitsPerInt - 2);
+      STATIC_ASSERT(FixedArray::kMaxLength < kHalfOfMaxInt);
+      USE(kHalfOfMaxInt);
+      result_len += len;
+      DCHECK(0 <= result_len);
+      DCHECK(result_len <= FixedDoubleArray::kMaxLength);
+
+      ElementsKind arg_kind = JSArray::cast(arg)->map()->elements_kind();
+      has_double = has_double || IsFastDoubleElementsKind(arg_kind);
+      is_holey = is_holey || IsFastHoleyElementsKind(arg_kind);
+      if (IsMoreGeneralElementsKindTransition(elements_kind, arg_kind)) {
+        elements_kind = arg_kind;
+      }
+    }
+    if (is_holey) {
+      elements_kind = GetHoleyElementsKind(elements_kind);
+    }
+  }
+
+  // If a double array is concatted into a fast elements array, the fast
+  // elements array needs to be initialized to contain proper holes, since
+  // boxing doubles may cause incremental marking.
+  ArrayStorageAllocationMode mode =
+      has_double && IsFastObjectElementsKind(elements_kind)
+          ? INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE
+          : DONT_INITIALIZE_ARRAY_ELEMENTS;
+  Handle<JSArray> result_array = isolate->factory()->NewJSArray(
+      elements_kind, result_len, result_len, Strength::WEAK, mode);
+  if (result_len == 0) return result_array;
+  int j = 0;
+  Handle<FixedArrayBase> storage(result_array->elements(), isolate);
+  ElementsAccessor* accessor = ElementsAccessor::ForKind(elements_kind);
+  for (uint32_t i = 0; i < concat_size; i++) {
+    // It is crucial to keep |array| in a raw pointer form to avoid
+    // performance degradation.
+    JSArray* array = JSArray::cast((*args)[i]);
+    int len = Smi::cast(array->length())->value();
+    if (len > 0) {
+      ElementsKind from_kind = array->GetElementsKind();
+      accessor->CopyElements(array, 0, from_kind, storage, j, len);
+      j += len;
+    }
+  }
+
+  DCHECK(j == result_len);
+  return result_array;
+}
+
 ElementsAccessor** ElementsAccessor::elements_accessors_ = NULL;
 }  // namespace internal
 }  // namespace v8

src/elements.h

@@ -127,8 +127,9 @@ class ElementsAccessor {
                    Handle<Object> value, PropertyAttributes attributes,
                    uint32_t new_capacity) = 0;
 
-  // TODO(cbruni): Consider passing Arguments* instead of Object** depending on
-  // the requirements of future callers.
+  static Handle<JSArray> Concat(Isolate* isolate, Arguments* args,
+                                uint32_t concat_size);
+
   virtual uint32_t Push(Handle<JSArray> receiver,
                         Handle<FixedArrayBase> backing_store, Arguments* args,
                         uint32_t push_size) = 0;

src/runtime/runtime.h

@@ -35,7 +35,6 @@ namespace internal {
   F(SpecialArrayFunctions, 0, 1)            \
   F(TransitionElementsKind, 2, 1)           \
   F(PushIfAbsent, 2, 1)                     \
-  F(ArrayConcat, 1, 1)                      \
   F(RemoveArrayHoles, 2, 1)                 \
   F(MoveArrayContents, 2, 1)                \
   F(EstimateNumberOfElements, 1, 1)         \

test/mjsunit/array-natives-elements.js

@@ -68,7 +68,7 @@ function array_natives_test() {
   // Concat
   var a1;
   a1 = [1,2,3].concat([]);
-  assertTrue(%HasFastSmiElements(a1));
+  //assertTrue(%HasFastSmiElements(a1));
   assertEquals([1,2,3], a1);
   a1 = [1,2,3].concat([4,5,6]);
   assertTrue(%HasFastSmiElements(a1));

test/test262-es6/test262-es6.status

@@ -490,9 +490,6 @@
   'language/expressions/object/method-definition/generator-name-prop-symbol': [FAIL],
   'language/expressions/object/method-definition/name-name-prop-symbol': [FAIL],
 
-  # https://code.google.com/p/v8/issues/detail?id=4317
-  'built-ins/Array/prototype/concat/is-concat-spreadable-val-falsey': [FAIL],
-
   # https://code.google.com/p/v8/issues/detail?id=2952
   'built-ins/RegExp/prototype/exec/u-lastindex-adv': [FAIL],
   'built-ins/RegExp/prototype/exec/u-captured-value': [FAIL],