From f92da58e136912dc0862519315275b370d686f6f Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 24 Oct 2011 15:06:20 +0000 Subject: [PATCH] Handle COW-arrays correctly when converting smi->double fast elements. TEST=mjsunit/elements-transition.js Review URL: http://codereview.chromium.org/8383002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9759 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/x64/code-stubs-x64.cc | 7 ++- src/x64/codegen-x64.cc | 86 ++++++++++++++++++++++++++----------- test/mjsunit/elements-transition.js | 13 ++++++ 3 files changed, 78 insertions(+), 28 deletions(-) diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 7242493..cfe323e 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -5689,11 +5689,14 @@ struct AheadOfTimeWriteBarrierStubList kAheadOfTime[] = { // KeyedStoreStubCompiler::GenerateStoreFastElement. { rdi, rdx, rcx, EMIT_REMEMBERED_SET}, // ElementsTransitionGenerator::GenerateSmiOnlyToObject + // and ElementsTransitionGenerator::GenerateSmiOnlyToObject // and ElementsTransitionGenerator::GenerateDoubleToObject { rdx, rbx, rdi, EMIT_REMEMBERED_SET}, + // ElementsTransitionGenerator::GenerateSmiOnlyToDouble + // and ElementsTransitionGenerator::GenerateDoubleToObject + { rdx, r11, r15, EMIT_REMEMBERED_SET}, // ElementsTransitionGenerator::GenerateDoubleToObject - { rax, r11, r15, EMIT_REMEMBERED_SET}, - { rdx, rax, rdi, EMIT_REMEMBERED_SET}, + { r11, rax, r15, EMIT_REMEMBERED_SET}, // Null termination. { no_reg, no_reg, no_reg, EMIT_REMEMBERED_SET} }; diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index ee40c6e..4c216e8 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -182,6 +182,25 @@ void ElementsTransitionGenerator::GenerateSmiOnlyToDouble( // -- rsp[0] : return address // ----------------------------------- // The fail label is not actually used since we do not allocate. + Label allocated, cow_array; + + // Check backing store for COW-ness. If the negative case, we do not have to + // allocate a new array, since FixedArray and FixedDoubleArray do not differ + // in size. + __ movq(r8, FieldOperand(rdx, JSObject::kElementsOffset)); + __ SmiToInteger32(r9, FieldOperand(r8, FixedDoubleArray::kLengthOffset)); + __ CompareRoot(FieldOperand(r8, HeapObject::kMapOffset), + Heap::kFixedCOWArrayMapRootIndex); + __ j(equal, &cow_array); + __ movq(r14, r8); // Destination array equals source array. + + __ bind(&allocated); + // r8 : source FixedArray + // r9 : elements array length + // r14: destination FixedDoubleArray + // Set backing store's map + __ LoadRoot(rdi, Heap::kFixedDoubleArrayMapRootIndex); + __ movq(FieldOperand(r14, HeapObject::kMapOffset), rdi); // Set transitioned map. __ movq(FieldOperand(rdx, HeapObject::kMapOffset), rbx); @@ -192,22 +211,37 @@ void ElementsTransitionGenerator::GenerateSmiOnlyToDouble( kDontSaveFPRegs, EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); - // Set backing store's map - __ movq(r8, FieldOperand(rdx, JSObject::kElementsOffset)); - __ LoadRoot(rdi, Heap::kFixedDoubleArrayMapRootIndex); - __ movq(FieldOperand(r8, HeapObject::kMapOffset), rdi); - // Convert smis to doubles and holes to hole NaNs. Since FixedArray and - // FixedDoubleArray do not differ in size, we do not allocate a new array. + // Convert smis to doubles and holes to hole NaNs. The Array's length + // remains unchanged. STATIC_ASSERT(FixedDoubleArray::kLengthOffset == FixedArray::kLengthOffset); STATIC_ASSERT(FixedDoubleArray::kHeaderSize == FixedArray::kHeaderSize); - __ SmiToInteger32(r9, FieldOperand(r8, FixedDoubleArray::kLengthOffset)); - // r8 : elements array - // r9 : elements array length + Label loop, entry, convert_hole; __ movq(r15, BitCast(kHoleNanInt64), RelocInfo::NONE); // r15: the-hole NaN __ jmp(&entry); + + // Allocate new array if the source array is a COW array. + __ bind(&cow_array); + __ lea(rdi, Operand(r9, times_pointer_size, FixedArray::kHeaderSize)); + __ AllocateInNewSpace(rdi, r14, r11, r15, fail, TAG_OBJECT); + // Set receiver's backing store. + __ movq(FieldOperand(rdx, JSObject::kElementsOffset), r14); + __ movq(r11, r14); + __ RecordWriteField(rdx, + JSObject::kElementsOffset, + r11, + r15, + kDontSaveFPRegs, + EMIT_REMEMBERED_SET, + OMIT_SMI_CHECK); + // Set backing store's length. + __ Integer32ToSmi(r11, r9); + __ movq(FieldOperand(r14, FixedDoubleArray::kLengthOffset), r11); + __ jmp(&allocated); + + // Conversion loop. __ bind(&loop); __ decq(r9); __ movq(rbx, @@ -217,11 +251,11 @@ void ElementsTransitionGenerator::GenerateSmiOnlyToDouble( __ JumpIfNotSmi(rbx, &convert_hole); __ SmiToInteger32(rbx, rbx); __ cvtlsi2sd(xmm0, rbx); - __ movsd(FieldOperand(r8, r9, times_8, FixedDoubleArray::kHeaderSize), + __ movsd(FieldOperand(r14, r9, times_8, FixedDoubleArray::kHeaderSize), xmm0); __ jmp(&entry); __ bind(&convert_hole); - __ movq(FieldOperand(r8, r9, times_8, FixedDoubleArray::kHeaderSize), r15); + __ movq(FieldOperand(r14, r9, times_8, FixedDoubleArray::kHeaderSize), r15); __ bind(&entry); __ testq(r9, r9); __ j(not_zero, &loop); @@ -245,12 +279,12 @@ void ElementsTransitionGenerator::GenerateDoubleToObject( // r8 : source FixedDoubleArray // r9 : number of elements __ lea(rdi, Operand(r9, times_pointer_size, FixedArray::kHeaderSize)); - __ AllocateInNewSpace(rdi, rax, r14, r15, &gc_required, TAG_OBJECT); - // rax: destination FixedArray + __ AllocateInNewSpace(rdi, r11, r14, r15, &gc_required, TAG_OBJECT); + // r11: destination FixedArray __ LoadRoot(rdi, Heap::kFixedArrayMapRootIndex); - __ movq(FieldOperand(rax, HeapObject::kMapOffset), rdi); + __ movq(FieldOperand(r11, HeapObject::kMapOffset), rdi); __ Integer32ToSmi(r14, r9); - __ movq(FieldOperand(rax, FixedArray::kLengthOffset), r14); + __ movq(FieldOperand(r11, FixedArray::kLengthOffset), r14); // Prepare for conversion loop. __ movq(rsi, BitCast(kHoleNanInt64), RelocInfo::NONE); @@ -278,17 +312,17 @@ void ElementsTransitionGenerator::GenerateDoubleToObject( __ j(equal, &convert_hole); // Non-hole double, copy value into a heap number. - __ AllocateHeapNumber(r11, r15, &gc_required); - // r11: new heap number - __ movq(FieldOperand(r11, HeapNumber::kValueOffset), r14); - __ movq(FieldOperand(rax, + __ AllocateHeapNumber(rax, r15, &gc_required); + // rax: new heap number + __ movq(FieldOperand(rax, HeapNumber::kValueOffset), r14); + __ movq(FieldOperand(r11, r9, times_pointer_size, FixedArray::kHeaderSize), - r11); + rax); __ movq(r15, r9); - __ RecordWriteArray(rax, - r11, + __ RecordWriteArray(r11, + rax, r15, kDontSaveFPRegs, EMIT_REMEMBERED_SET, @@ -297,7 +331,7 @@ void ElementsTransitionGenerator::GenerateDoubleToObject( // Replace the-hole NaN with the-hole pointer. __ bind(&convert_hole); - __ movq(FieldOperand(rax, + __ movq(FieldOperand(r11, r9, times_pointer_size, FixedArray::kHeaderSize), @@ -317,11 +351,11 @@ void ElementsTransitionGenerator::GenerateDoubleToObject( EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); // Replace receiver's backing store with newly created and filled FixedArray. - __ movq(FieldOperand(rdx, JSObject::kElementsOffset), rax); + __ movq(FieldOperand(rdx, JSObject::kElementsOffset), r11); __ RecordWriteField(rdx, JSObject::kElementsOffset, - rax, - rdi, + r11, + r15, kDontSaveFPRegs, EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); diff --git a/test/mjsunit/elements-transition.js b/test/mjsunit/elements-transition.js index 7b90725..5f6cc4f 100644 --- a/test/mjsunit/elements-transition.js +++ b/test/mjsunit/elements-transition.js @@ -89,6 +89,19 @@ if (support_smi_only_arrays) { test(true, false, function(a,i,v){ a[i] = v; }, 10000); test(false, true, function(a,i,v){ a[i] = v; }, 10000); test(true, true, function(a,i,v){ a[i] = v; }, 10000); + + // Check COW arrays + function get_cow() { return [1, 2, 3]; } + + function transition(x) { x[0] = 1.5; } + + var ignore = get_cow(); + transition(ignore); // Handled by runtime. + var a = get_cow(); + var b = get_cow(); + transition(a); // Handled by IC. + assertEquals(1.5, a[0]); + assertEquals(1, b[0]); } else { print("Test skipped because smi only arrays are not supported."); } \ No newline at end of file -- 2.7.4