From 1b5a2381ec7d4ff3898e23ba745a07ac0f157dab Mon Sep 17 00:00:00 2001 From: "danno@chromium.org" Date: Tue, 27 Sep 2011 16:15:29 +0000 Subject: [PATCH] Optimize KeyedStoreGeneric for Smi arrays. BUG= TEST= Review URL: http://codereview.chromium.org/8022002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9456 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ia32/ic-ia32.cc | 76 +++++++++++++++++++++++--------- src/ia32/macro-assembler-ia32.cc | 73 ++++++++++++++++++++++++++++++ src/ia32/macro-assembler-ia32.h | 11 +++++ src/ia32/stub-cache-ia32.cc | 63 ++++---------------------- src/serialize.cc | 4 ++ 5 files changed, 150 insertions(+), 77 deletions(-) diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index 65ecaeced0..3c95356642 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -734,7 +734,9 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // -- edx : receiver // -- esp[0] : return address // ----------------------------------- - Label slow, fast, array, extra; + Label slow, fast_object_with_map_check, fast_object_without_map_check; + Label fast_double_with_map_check, fast_double_without_map_check; + Label check_if_double_array, array, extra; // Check that the object isn't a smi. __ JumpIfSmi(edx, &slow); @@ -761,11 +763,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // eax: value // edx: JSObject // ecx: key (a smi) - __ mov(edi, FieldOperand(edx, JSObject::kElementsOffset)); + // edi: receiver map + __ mov(ebx, FieldOperand(edx, JSObject::kElementsOffset)); // Check that the object is in fast mode and writable. - __ CheckMap(edi, FACTORY->fixed_array_map(), &slow, DONT_DO_SMI_CHECK); - __ cmp(ecx, FieldOperand(edi, FixedArray::kLengthOffset)); - __ j(below, &fast); + __ cmp(ecx, FieldOperand(ebx, FixedArray::kLengthOffset)); + __ j(below, &fast_object_with_map_check); // Slow case: call runtime. __ bind(&slow); @@ -778,16 +780,26 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // eax: value // edx: receiver, a JSArray // ecx: key, a smi. - // edi: receiver->elements, a FixedArray + // ebx: receiver->elements, a FixedArray + // edi: receiver map // flags: compare (ecx, edx.length()) // do not leave holes in the array: __ j(not_equal, &slow); - __ cmp(ecx, FieldOperand(edi, FixedArray::kLengthOffset)); + __ cmp(ecx, FieldOperand(ebx, FixedArray::kLengthOffset)); __ j(above_equal, &slow); - // Add 1 to receiver->length, and go to fast array write. + __ CheckMap(ebx, FACTORY->fixed_array_map(), + &check_if_double_array, DONT_DO_SMI_CHECK); + // Add 1 to receiver->length, and go to common element store code for Objects. __ add(FieldOperand(edx, JSArray::kLengthOffset), Immediate(Smi::FromInt(1))); - __ jmp(&fast); + __ jmp(&fast_object_without_map_check); + + __ bind(&check_if_double_array); + __ CheckMap(ebx, FACTORY->fixed_double_array_map(), &slow, DONT_DO_SMI_CHECK); + // Add 1 to receiver->length, and go to common element store code for doubles. + __ add(FieldOperand(edx, JSArray::kLengthOffset), + Immediate(Smi::FromInt(1))); + __ jmp(&fast_double_without_map_check); // Array case: Get the length and the elements array from the JS // array. Check that the array is in fast mode (and writable); if it @@ -796,39 +808,59 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // eax: value // edx: receiver, a JSArray // ecx: key, a smi. - __ mov(edi, FieldOperand(edx, JSObject::kElementsOffset)); - __ CheckMap(edi, FACTORY->fixed_array_map(), &slow, DONT_DO_SMI_CHECK); + // edi: receiver map + __ mov(ebx, FieldOperand(edx, JSObject::kElementsOffset)); - // Check the key against the length in the array, compute the - // address to store into and fall through to fast case. + // Check the key against the length in the array and fall through to the + // common store code. __ cmp(ecx, FieldOperand(edx, JSArray::kLengthOffset)); // Compare smis. __ j(above_equal, &extra); - // Fast case: Do the store. - __ bind(&fast); + // Fast case: Do the store, could either Object or double. + __ bind(&fast_object_with_map_check); // eax: value // ecx: key (a smi) // edx: receiver - // edi: FixedArray receiver->elements - + // ebx: FixedArray receiver->elements + // edi: receiver map + __ CheckMap(ebx, FACTORY->fixed_array_map(), + &fast_double_with_map_check, DONT_DO_SMI_CHECK); + __ bind(&fast_object_without_map_check); + // Smi stores don't require further checks. Label non_smi_value; __ JumpIfNotSmi(eax, &non_smi_value); // It's irrelevant whether array is smi-only or not when writing a smi. - __ mov(CodeGenerator::FixedArrayElementOperand(edi, ecx), eax); + __ mov(CodeGenerator::FixedArrayElementOperand(ebx, ecx), eax); __ ret(0); __ bind(&non_smi_value); if (FLAG_smi_only_arrays) { // Escape to slow case when writing non-smi into smi-only array. - __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset)); - __ CheckFastObjectElements(ebx, &slow, Label::kNear); + __ CheckFastObjectElements(edi, &slow, Label::kNear); } + // Fast elements array, store the value to the elements backing store. - __ mov(CodeGenerator::FixedArrayElementOperand(edi, ecx), eax); + __ mov(CodeGenerator::FixedArrayElementOperand(ebx, ecx), eax); // Update write barrier for the elements array address. __ mov(edx, Operand(eax)); // Preserve the value which is returned. __ RecordWriteArray( - edi, edx, ecx, kDontSaveFPRegs, EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); + ebx, edx, ecx, kDontSaveFPRegs, EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); + __ ret(0); + + __ bind(&fast_double_with_map_check); + // Check for fast double array case. If this fails, call through to the + // runtime. + __ CheckMap(ebx, FACTORY->fixed_double_array_map(), &slow, DONT_DO_SMI_CHECK); + __ bind(&fast_double_without_map_check); + // If the value is a number, store it as a double in the FastDoubleElements + // array. + __ StoreNumberToDoubleElements(eax, + ebx, + ecx, + edx, + xmm0, + &slow, + false); __ ret(0); } diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index 2e0560d6a8..c234980f9b 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -406,6 +406,79 @@ void MacroAssembler::CheckFastSmiOnlyElements(Register map, } +void MacroAssembler::StoreNumberToDoubleElements( + Register maybe_number, + Register elements, + Register key, + Register scratch1, + XMMRegister scratch2, + Label* fail, + bool specialize_for_processor) { + Label smi_value, done, maybe_nan, not_nan, is_nan, have_double_value; + JumpIfSmi(maybe_number, &smi_value, Label::kNear); + + CheckMap(maybe_number, + isolate()->factory()->heap_number_map(), + fail, + DONT_DO_SMI_CHECK); + + // Double value, canonicalize NaN. + uint32_t offset = HeapNumber::kValueOffset + sizeof(kHoleNanLower32); + cmp(FieldOperand(maybe_number, offset), + Immediate(kNaNOrInfinityLowerBoundUpper32)); + j(greater_equal, &maybe_nan, Label::kNear); + + bind(¬_nan); + ExternalReference canonical_nan_reference = + ExternalReference::address_of_canonical_non_hole_nan(); + if (CpuFeatures::IsSupported(SSE2) && specialize_for_processor) { + CpuFeatures::Scope use_sse2(SSE2); + movdbl(scratch2, FieldOperand(maybe_number, HeapNumber::kValueOffset)); + bind(&have_double_value); + movdbl(FieldOperand(elements, key, times_4, FixedDoubleArray::kHeaderSize), + scratch2); + } else { + fld_d(FieldOperand(maybe_number, HeapNumber::kValueOffset)); + bind(&have_double_value); + fstp_d(FieldOperand(elements, key, times_4, FixedDoubleArray::kHeaderSize)); + } + jmp(&done); + + bind(&maybe_nan); + // Could be NaN or Infinity. If fraction is not zero, it's NaN, otherwise + // it's an Infinity, and the non-NaN code path applies. + j(greater, &is_nan, Label::kNear); + cmp(FieldOperand(maybe_number, HeapNumber::kValueOffset), Immediate(0)); + j(zero, ¬_nan); + bind(&is_nan); + if (CpuFeatures::IsSupported(SSE2) && specialize_for_processor) { + CpuFeatures::Scope use_sse2(SSE2); + movdbl(scratch2, Operand::StaticVariable(canonical_nan_reference)); + } else { + fld_d(Operand::StaticVariable(canonical_nan_reference)); + } + jmp(&have_double_value, Label::kNear); + + bind(&smi_value); + // Value is a smi. Convert to a double and store. + // Preserve original value. + mov(scratch1, maybe_number); + SmiUntag(scratch1); + if (CpuFeatures::IsSupported(SSE2) && specialize_for_processor) { + CpuFeatures::Scope fscope(SSE2); + cvtsi2sd(scratch2, Operand(scratch1)); + movdbl(FieldOperand(elements, key, times_4, FixedDoubleArray::kHeaderSize), + scratch2); + } else { + push(scratch1); + fild_s(Operand(esp, 0)); + pop(scratch1); + fstp_d(FieldOperand(elements, key, times_4, FixedDoubleArray::kHeaderSize)); + } + bind(&done); +} + + void MacroAssembler::CheckMap(Register obj, Handle map, Label* fail, diff --git a/src/ia32/macro-assembler-ia32.h b/src/ia32/macro-assembler-ia32.h index d097da8e93..ff9f747eba 100644 --- a/src/ia32/macro-assembler-ia32.h +++ b/src/ia32/macro-assembler-ia32.h @@ -324,6 +324,17 @@ class MacroAssembler: public Assembler { Label* fail, Label::Distance distance = Label::kFar); + // Check to see if maybe_number can be stored as a double in + // FastDoubleElements. If it can, store it at the index specified by key in + // the FastDoubleElements array elements, otherwise jump to fail. + void StoreNumberToDoubleElements(Register maybe_number, + Register elements, + Register key, + Register scratch1, + XMMRegister scratch2, + Label* fail, + bool specialize_for_processor); + // Check if the map of an object is equal to a specified map and branch to // label if not. Skip the smi check if not required (object is known to be a // heap object) diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index addf9e134d..f52aa87553 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -3977,8 +3977,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( // -- edx : receiver // -- esp[0] : return address // ----------------------------------- - Label miss_force_generic, smi_value, is_nan, maybe_nan; - Label have_double_value, not_nan; + Label miss_force_generic; // This stub is meant to be tail-jumped to, the receiver must already // have been verified by the caller to not be a smi. @@ -3999,59 +3998,13 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( } __ j(above_equal, &miss_force_generic); - __ JumpIfSmi(eax, &smi_value, Label::kNear); - - __ CheckMap(eax, - masm->isolate()->factory()->heap_number_map(), - &miss_force_generic, - DONT_DO_SMI_CHECK); - - // Double value, canonicalize NaN. - uint32_t offset = HeapNumber::kValueOffset + sizeof(kHoleNanLower32); - __ cmp(FieldOperand(eax, offset), Immediate(kNaNOrInfinityLowerBoundUpper32)); - __ j(greater_equal, &maybe_nan, Label::kNear); - - __ bind(¬_nan); - ExternalReference canonical_nan_reference = - ExternalReference::address_of_canonical_non_hole_nan(); - if (CpuFeatures::IsSupported(SSE2)) { - CpuFeatures::Scope use_sse2(SSE2); - __ movdbl(xmm0, FieldOperand(eax, HeapNumber::kValueOffset)); - __ bind(&have_double_value); - __ movdbl(FieldOperand(edi, ecx, times_4, FixedDoubleArray::kHeaderSize), - xmm0); - __ ret(0); - } else { - __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); - __ bind(&have_double_value); - __ fstp_d(FieldOperand(edi, ecx, times_4, FixedDoubleArray::kHeaderSize)); - __ ret(0); - } - - __ bind(&maybe_nan); - // Could be NaN or Infinity. If fraction is not zero, it's NaN, otherwise - // it's an Infinity, and the non-NaN code path applies. - __ j(greater, &is_nan, Label::kNear); - __ cmp(FieldOperand(eax, HeapNumber::kValueOffset), Immediate(0)); - __ j(zero, ¬_nan); - __ bind(&is_nan); - if (CpuFeatures::IsSupported(SSE2)) { - CpuFeatures::Scope use_sse2(SSE2); - __ movdbl(xmm0, Operand::StaticVariable(canonical_nan_reference)); - } else { - __ fld_d(Operand::StaticVariable(canonical_nan_reference)); - } - __ jmp(&have_double_value, Label::kNear); - - __ bind(&smi_value); - // Value is a smi. convert to a double and store. - // Preserve original value. - __ mov(edx, eax); - __ SmiUntag(edx); - __ push(edx); - __ fild_s(Operand(esp, 0)); - __ pop(edx); - __ fstp_d(FieldOperand(edi, ecx, times_4, FixedDoubleArray::kHeaderSize)); + __ StoreNumberToDoubleElements(eax, + edi, + ecx, + edx, + xmm0, + &miss_force_generic, + true); __ ret(0); // Handle store cache miss, replacing the ic with the generic stub. diff --git a/src/serialize.cc b/src/serialize.cc index 371b797434..49af760ad5 100644 --- a/src/serialize.cc +++ b/src/serialize.cc @@ -494,6 +494,10 @@ void ExternalReferenceTable::PopulateTable(Isolate* isolate) { UNCLASSIFIED, 44, "Factory::arguments_marker().location()"); + Add(ExternalReference::address_of_canonical_non_hole_nan().address(), + UNCLASSIFIED, + 45, + "canonical_nan"); }