Ensure double arrays are filled with holes when extended from variations of empty...
authorverwaest@chromium.org <verwaest@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 26 Nov 2012 14:29:21 +0000 (14:29 +0000)
committerverwaest@chromium.org <verwaest@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 26 Nov 2012 14:29:21 +0000 (14:29 +0000)
BUG=162085

Review URL: https://chromiumcodereview.appspot.com/11414155

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13056 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/stub-cache-arm.cc
src/elements.cc
src/ia32/stub-cache-ia32.cc
src/parser.cc
src/x64/stub-cache-x64.cc
test/mjsunit/array-natives-elements.js
test/mjsunit/regress/regress-crbug-162085.js [new file with mode: 0644]

index 60a2129..866ab55 100644 (file)
@@ -4783,8 +4783,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     __ AllocateInNewSpace(size, elements_reg, scratch1, scratch2, &slow,
                           TAG_OBJECT);
 
-    // Initialize the new FixedDoubleArray. Leave elements unitialized for
-    // efficiency, they are guaranteed to be initialized before use.
+    // Initialize the new FixedDoubleArray.
     __ LoadRoot(scratch1, Heap::kFixedDoubleArrayMapRootIndex);
     __ str(scratch1, FieldMemOperand(elements_reg, JSObject::kMapOffset));
     __ mov(scratch1,
@@ -4792,6 +4791,24 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     __ str(scratch1,
            FieldMemOperand(elements_reg, FixedDoubleArray::kLengthOffset));
 
+    __ mov(scratch1, Operand(kHoleNanLower32));
+    __ mov(scratch2, Operand(kHoleNanUpper32));
+    for (int i = 1; i < JSArray::kPreallocatedArrayElements; i++) {
+      int offset = FixedDoubleArray::OffsetOfElementAt(i);
+      __ str(scratch1, FieldMemOperand(elements_reg, offset));
+      __ str(scratch2, FieldMemOperand(elements_reg, offset + kPointerSize));
+    }
+
+    __ StoreNumberToDoubleElements(value_reg,
+                                   key_reg,
+                                   // All registers after this are overwritten.
+                                   elements_reg,
+                                   scratch1,
+                                   scratch2,
+                                   scratch3,
+                                   scratch4,
+                                   &transition_elements_kind);
+
     // Install the new backing store in the JSArray.
     __ str(elements_reg,
            FieldMemOperand(receiver_reg, JSObject::kElementsOffset));
@@ -4804,7 +4821,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     __ str(length_reg, FieldMemOperand(receiver_reg, JSArray::kLengthOffset));
     __ ldr(elements_reg,
            FieldMemOperand(receiver_reg, JSObject::kElementsOffset));
-    __ jmp(&finish_store);
+    __ Ret();
 
     __ bind(&check_capacity);
     // Make sure that the backing store can hold additional elements.
index eb021e5..6103da4 100644 (file)
@@ -375,6 +375,9 @@ static void CopyPackedSmiToDoubleElements(FixedArray* from,
     copy_size = from->length() - from_start;
     if (raw_copy_size == ElementsAccessor::kCopyToEndAndInitializeToHole) {
       to_end = to->length();
+      for (uint32_t i = to_start + copy_size; i < to_end; ++i) {
+        to->set_the_hole(i);
+      }
     } else {
       to_end = to_start + static_cast<uint32_t>(copy_size);
     }
@@ -392,10 +395,6 @@ static void CopyPackedSmiToDoubleElements(FixedArray* from,
     ASSERT(!smi->IsTheHole());
     to->set(to_start, Smi::cast(smi)->value());
   }
-
-  while (to_start < to_end) {
-    to->set_the_hole(to_start++);
-  }
 }
 
 
index 28a3994..c3a2654 100644 (file)
@@ -4353,13 +4353,22 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     // ecx: key
     // edx: receiver
     // edi: elements
-    // Initialize the new FixedDoubleArray. Leave elements unitialized for
-    // efficiency, they are guaranteed to be initialized before use.
+    // Initialize the new FixedDoubleArray.
     __ mov(FieldOperand(edi, JSObject::kMapOffset),
            Immediate(masm->isolate()->factory()->fixed_double_array_map()));
     __ mov(FieldOperand(edi, FixedDoubleArray::kLengthOffset),
            Immediate(Smi::FromInt(JSArray::kPreallocatedArrayElements)));
 
+    for (int i = 1; i < JSArray::kPreallocatedArrayElements; i++) {
+      int offset = FixedDoubleArray::OffsetOfElementAt(i);
+      __ mov(FieldOperand(edi, offset), Immediate(kHoleNanLower32));
+      __ mov(FieldOperand(edi, offset + kPointerSize),
+             Immediate(kHoleNanUpper32));
+    }
+
+    __ StoreNumberToDoubleElements(eax, edi, ecx, ebx, xmm0,
+                                   &transition_elements_kind, true);
+
     // Install the new backing store in the JSArray.
     __ mov(FieldOperand(edx, JSObject::kElementsOffset), edi);
     __ RecordWriteField(edx, JSObject::kElementsOffset, edi, ebx,
@@ -4369,7 +4378,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     __ add(FieldOperand(edx, JSArray::kLengthOffset),
            Immediate(Smi::FromInt(1)));
     __ mov(edi, FieldOperand(edx, JSObject::kElementsOffset));
-    __ jmp(&finish_store);
+    __ ret(0);
 
     __ bind(&check_capacity);
     // eax: value
index 75fd7b7..a7bb4e7 100644 (file)
@@ -3718,7 +3718,6 @@ Expression* Parser::ParseArrayLiteral(bool* ok) {
       isolate()->factory()->NewFixedArray(values->length(), TENURED);
   Handle<FixedDoubleArray> double_literals;
   ElementsKind elements_kind = FAST_SMI_ELEMENTS;
-  bool has_only_undefined_values = true;
   bool has_hole_values = false;
 
   // Fill in the literals.
@@ -3750,7 +3749,6 @@ Expression* Parser::ParseArrayLiteral(bool* ok) {
       // FAST_DOUBLE_ELEMENTS and FAST_ELEMENTS as necessary.  Always remember
       // the tagged value, no matter what the ElementsKind is in case we
       // ultimately end up in FAST_ELEMENTS.
-      has_only_undefined_values = false;
       object_literals->set(i, *boilerplate_value);
       if (elements_kind == FAST_SMI_ELEMENTS) {
         // Smi only elements. Notice if a transition to FAST_DOUBLE_ELEMENTS or
@@ -3789,13 +3787,6 @@ Expression* Parser::ParseArrayLiteral(bool* ok) {
     }
   }
 
-  // Very small array literals that don't have a concrete hint about their type
-  // from a constant value should default to the slow case to avoid lots of
-  // elements transitions on really small objects.
-  if (has_only_undefined_values && values->length() <= 2) {
-    elements_kind = FAST_ELEMENTS;
-  }
-
   // Simple and shallow arrays can be lazily copied, we transform the
   // elements array to a copy-on-write array.
   if (is_simple && depth == 1 && values->length() > 0 &&
index 26a97ab..cd0124b 100644 (file)
@@ -4026,7 +4026,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
   //  -- rsp[0] : return address
   // -----------------------------------
   Label miss_force_generic, transition_elements_kind, finish_store;
-  Label grow, slow, check_capacity;
+  Label grow, slow, check_capacity, restore_key_transition_elements_kind;
 
   // This stub is meant to be tail-jumped to, the receiver must already
   // have been verified by the caller to not be a smi.
@@ -4055,7 +4055,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
   __ bind(&finish_store);
   __ SmiToInteger32(rcx, rcx);
   __ StoreNumberToDoubleElements(rax, rdi, rcx, xmm0,
-                                 &transition_elements_kind);
+                                 &restore_key_transition_elements_kind);
   __ ret(0);
 
   // Handle store cache miss, replacing the ic with the generic stub.
@@ -4064,9 +4064,10 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
       masm->isolate()->builtins()->KeyedStoreIC_MissForceGeneric();
   __ jmp(ic_force_generic, RelocInfo::CODE_TARGET);
 
-  __ bind(&transition_elements_kind);
+  __ bind(&restore_key_transition_elements_kind);
   // Restore smi-tagging of rcx.
   __ Integer32ToSmi(rcx, rcx);
+  __ bind(&transition_elements_kind);
   Handle<Code> ic_miss = masm->isolate()->builtins()->KeyedStoreIC_Miss();
   __ jmp(ic_miss, RelocInfo::CODE_TARGET);
 
@@ -4107,6 +4108,16 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     __ Move(FieldOperand(rdi, FixedDoubleArray::kLengthOffset),
             Smi::FromInt(JSArray::kPreallocatedArrayElements));
 
+    __ movq(r8, BitCast<int64_t, uint64_t>(kHoleNanInt64), RelocInfo::NONE);
+    for (int i = 1; i < JSArray::kPreallocatedArrayElements; i++) {
+      __ movq(FieldOperand(rdi, FixedDoubleArray::OffsetOfElementAt(i)), r8);
+    }
+
+    // Increment the length of the array.
+    __ SmiToInteger32(rcx, rcx);
+    __ StoreNumberToDoubleElements(rax, rdi, rcx, xmm0,
+                                   &restore_key_transition_elements_kind);
+
     // Install the new backing store in the JSArray.
     __ movq(FieldOperand(rdx, JSObject::kElementsOffset), rdi);
     __ RecordWriteField(rdx, JSObject::kElementsOffset, rdi, rbx,
@@ -4115,7 +4126,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(
     // Increment the length of the array.
     __ Move(FieldOperand(rdx, JSArray::kLengthOffset), Smi::FromInt(1));
     __ movq(rdi, FieldOperand(rdx, JSObject::kElementsOffset));
-    __ jmp(&finish_store);
+    __ ret(0);
 
     __ bind(&check_capacity);
     // rax: value
index dde33a5..96a8cb5 100644 (file)
@@ -49,7 +49,7 @@ function get(foo) { return foo; }  // Used to generate dynamic values.
 function array_natives_test() {
 
   // Ensure small array literals start in specific element kind mode.
-  assertTrue(%HasFastObjectElements([]));
+  assertTrue(%HasFastSmiElements([]));
   assertTrue(%HasFastSmiElements([1]));
   assertTrue(%HasFastSmiElements([1,2]));
   assertTrue(%HasFastDoubleElements([1.1]));
@@ -73,7 +73,7 @@ function array_natives_test() {
   // Concat
   var a1;
   a1 = [1,2,3].concat([]);
-  assertTrue(%HasFastObjectElements(a1));
+  assertTrue(%HasFastSmiElements(a1));
   assertEquals([1,2,3], a1);
   a1 = [1,2,3].concat([4,5,6]);
   assertTrue(%HasFastSmiElements(a1));
@@ -82,7 +82,7 @@ function array_natives_test() {
   assertTrue(%HasFastSmiElements(a1));
   assertEquals([1,2,3,4,5,6,7,8,9], a1);
   a1 = [1.1,2,3].concat([]);
-  assertTrue(%HasFastObjectElements(a1));
+  assertTrue(%HasFastDoubleElements(a1));
   assertEquals([1.1,2,3], a1);
   a1 = [1,2,3].concat([1.1, 2]);
   assertTrue(%HasFastDoubleElements(a1));
@@ -173,7 +173,7 @@ function array_natives_test() {
   a3r = a3.splice(0, 0, 2);
   // Commented out since handled in js, which takes the best fit.
   // assertTrue(%HasFastDoubleElements(a3r));
-  assertTrue(%HasFastObjectElements(a3r));
+  assertTrue(%HasFastSmiElements(a3r));
   assertTrue(%HasFastDoubleElements(a3));
   assertEquals([], a3r);
   assertEquals([2, 1.1, 2, 3], a3);
@@ -187,7 +187,7 @@ function array_natives_test() {
   a3r = a3.splice(0, 0, 2.1);
   // Commented out since handled in js, which takes the best fit.
   // assertTrue(%HasFastDoubleElements(a3r));
-  assertTrue(%HasFastObjectElements(a3r));
+  assertTrue(%HasFastSmiElements(a3r));
   assertTrue(%HasFastDoubleElements(a3));
   assertEquals([], a3r);
   assertEquals([2.1, 1.1, 2, 3], a3);
@@ -201,7 +201,7 @@ function array_natives_test() {
   a3r = a3.splice(0, 0, 2.1);
   // Commented out since handled in js, which takes the best fit.
   // assertTrue(%HasFastDoubleElements(a3r));
-  assertTrue(%HasFastObjectElements(a3r));
+  assertTrue(%HasFastSmiElements(a3r));
   assertTrue(%HasFastDoubleElements(a3));
   assertEquals([], a3r);
   assertEquals([2.1, 1, 2, 3], a3);
diff --git a/test/mjsunit/regress/regress-crbug-162085.js b/test/mjsunit/regress/regress-crbug-162085.js
new file mode 100644 (file)
index 0000000..f26c711
--- /dev/null
@@ -0,0 +1,55 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Ensure extending an empty packed smi array with a double initializes the
+// array with holes.
+var a = [1,2,3];
+a.length = 0;
+a[0] = 1.4;
+assertEquals(undefined, a[1]);
+assertEquals(undefined, a[2]);
+assertEquals(undefined, a[3]);
+
+// Ensure the double array growstub initializes the array with holes.
+function grow_store(a,i,v) {
+  a[i] = v;
+}
+
+var a2 = [1.3];
+grow_store(a2,1,1.4);
+a2.length = 0;
+grow_store(a2,0,1.5);
+assertEquals(undefined, a2[1]);
+assertEquals(undefined, a2[2]);
+assertEquals(undefined, a2[3]);
+
+// Check storing objects using the double grow stub.
+var a3 = [1.3];
+var o = {};
+grow_store(a3, 1, o);
+assertEquals(1.3, a3[0]);
+assertEquals(o, a3[1]);