From d5f00cf6ccb1c557d5ecfdea491997f4076f42eb Mon Sep 17 00:00:00 2001 From: "kbr@chromium.org" Date: Thu, 22 Oct 2009 14:49:00 +0000 Subject: [PATCH] Add optimized ICs for new CanvasArray types introduced in WebGL specification under development. This is a follow-on CL to http://codereview.chromium.org/293023 . Based on review feedback, defined the behavior of storing NaN and +/-Infinity into external arrays of integer types as storing 0. Added test cases. Added fucomi instruction to assembler. Fixed bug in KeyedLoadIC::GenerateExternalArray when allocation of HeapNumber failed. Fixed bug in encoding of 16-bit immediate arithmetic instructions in 64-bit port. Removed raising of exceptions for negative array indices passed to external arrays and associated tests. Based on current discussion in WebGL working group, will probably end up removing the exception throwing altogether. Review URL: http://codereview.chromium.org/294022 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3113 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ia32/assembler-ia32.cc | 8 + src/ia32/assembler-ia32.h | 1 + src/ia32/ic-ia32.cc | 348 ++++++++++++++++++++++++++++++++++++- src/objects.cc | 4 +- src/runtime.cc | 24 --- src/x64/assembler-x64.cc | 14 +- src/x64/assembler-x64.h | 1 + src/x64/ic-x64.cc | 307 +++++++++++++++++++++++++++++++- test/cctest/test-api.cc | 57 +++--- 9 files changed, 707 insertions(+), 57 deletions(-) diff --git a/src/ia32/assembler-ia32.cc b/src/ia32/assembler-ia32.cc index afafbf16da..698377a0c8 100644 --- a/src/ia32/assembler-ia32.cc +++ b/src/ia32/assembler-ia32.cc @@ -1850,6 +1850,14 @@ void Assembler::fucompp() { } +void Assembler::fucomi(int i) { + EnsureSpace ensure_space(this); + last_pc_ = pc_; + EMIT(0xDB); + EMIT(0xE8 + i); +} + + void Assembler::fucomip() { EnsureSpace ensure_space(this); last_pc_ = pc_; diff --git a/src/ia32/assembler-ia32.h b/src/ia32/assembler-ia32.h index e31cd3f2fd..4d9f08b066 100644 --- a/src/ia32/assembler-ia32.h +++ b/src/ia32/assembler-ia32.h @@ -702,6 +702,7 @@ class Assembler : public Malloced { void ftst(); void fucomp(int i); void fucompp(); + void fucomi(int i); void fucomip(); void fcompp(); void fnstsw_ax(); diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index 92632d77f7..58603d4630 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -344,7 +344,162 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) { void KeyedLoadIC::GenerateExternalArray(MacroAssembler* masm, ExternalArrayType array_type) { - GenerateGeneric(masm); + // ----------- S t a t e ------------- + // -- esp[0] : return address + // -- esp[4] : key + // -- esp[8] : receiver + // ----------------------------------- + Label slow, failed_allocation; + + // Load name and receiver. + __ mov(eax, Operand(esp, kPointerSize)); + __ mov(ecx, Operand(esp, 2 * kPointerSize)); + + // Check that the object isn't a smi. + __ test(ecx, Immediate(kSmiTagMask)); + __ j(zero, &slow, not_taken); + + // Check that the key is a smi. + __ test(eax, Immediate(kSmiTagMask)); + __ j(not_zero, &slow, not_taken); + + // Get the map of the receiver. + __ mov(edx, FieldOperand(ecx, HeapObject::kMapOffset)); + // Check that the receiver does not require access checks. We need + // to check this explicitly since this generic stub does not perform + // map checks. + __ movzx_b(ebx, FieldOperand(edx, Map::kBitFieldOffset)); + __ test(ebx, Immediate(1 << Map::kIsAccessCheckNeeded)); + __ j(not_zero, &slow, not_taken); + + // Get the instance type from the map of the receiver. + __ movzx_b(edx, FieldOperand(edx, Map::kInstanceTypeOffset)); + // Check that the object is a JS object. + __ cmp(edx, JS_OBJECT_TYPE); + __ j(not_equal, &slow, not_taken); + + // Check that the elements array is the appropriate type of + // ExternalArray. + // eax: index (as a smi) + // ecx: JSObject + __ mov(ecx, FieldOperand(ecx, JSObject::kElementsOffset)); + Handle map(Heap::MapForExternalArrayType(array_type)); + __ cmp(FieldOperand(ecx, HeapObject::kMapOffset), + Immediate(map)); + __ j(not_equal, &slow, not_taken); + + // Check that the index is in range. + __ sar(eax, kSmiTagSize); // Untag the index. + __ cmp(eax, FieldOperand(ecx, ExternalArray::kLengthOffset)); + // Unsigned comparison catches both negative and too-large values. + __ j(above_equal, &slow); + + // eax: untagged index + // ecx: elements array + __ mov(ecx, FieldOperand(ecx, ExternalArray::kExternalPointerOffset)); + // ecx: base pointer of external storage + switch (array_type) { + case kExternalByteArray: + __ movsx_b(eax, Operand(ecx, eax, times_1, 0)); + break; + case kExternalUnsignedByteArray: + __ mov_b(eax, Operand(ecx, eax, times_1, 0)); + break; + case kExternalShortArray: + __ movsx_w(eax, Operand(ecx, eax, times_2, 0)); + break; + case kExternalUnsignedShortArray: + __ mov_w(eax, Operand(ecx, eax, times_2, 0)); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: + __ mov(eax, Operand(ecx, eax, times_4, 0)); + break; + case kExternalFloatArray: + __ fld_s(Operand(ecx, eax, times_4, 0)); + break; + default: + UNREACHABLE(); + break; + } + + // For integer array types: + // eax: value + // For floating-point array type: + // FP(0): value + + if (array_type == kExternalIntArray || + array_type == kExternalUnsignedIntArray) { + + // For the Int and UnsignedInt array types, we need to see whether + // the value can be represented in a Smi. If not, we need to convert + // it to a HeapNumber. + Label box_int; + if (array_type == kExternalIntArray) { + // See Smi::IsValid for why this works. + __ mov(ebx, eax); + __ add(Operand(ebx), Immediate(0x40000000)); + __ cmp(ebx, 0x80000000); + __ j(above_equal, &box_int); + } else { + ASSERT_EQ(array_type, kExternalUnsignedIntArray); + // The test is different for unsigned int values. Since we need + // the Smi-encoded result to be treated as unsigned, we can't + // handle either of the top two bits being set in the value. + __ test(eax, Immediate(0xC0000000)); + __ j(not_zero, &box_int); + } + + __ shl(eax, kSmiTagSize); + __ ret(0); + + __ bind(&box_int); + + // Allocate a HeapNumber for the int and perform int-to-double + // conversion. + if (array_type == kExternalIntArray) { + __ push(eax); + __ fild_s(Operand(esp, 0)); + __ pop(eax); + } else { + ASSERT(array_type == kExternalUnsignedIntArray); + // Need to zero-extend the value. + // There's no fild variant for unsigned values, so zero-extend + // to a 64-bit int manually. + __ push(Immediate(0)); + __ push(eax); + __ fild_d(Operand(esp, 0)); + __ pop(eax); + __ pop(eax); + } + // FP(0): value + __ AllocateHeapNumber(eax, ebx, ecx, &failed_allocation); + // Set the value. + __ fstp_d(FieldOperand(eax, HeapNumber::kValueOffset)); + __ ret(0); + } else if (array_type == kExternalFloatArray) { + // For the floating-point array type, we need to always allocate a + // HeapNumber. + __ AllocateHeapNumber(eax, ebx, ecx, &failed_allocation); + // Set the value. + __ fstp_d(FieldOperand(eax, HeapNumber::kValueOffset)); + __ ret(0); + } else { + __ shl(eax, kSmiTagSize); + __ ret(0); + } + + // If we fail allocation of the HeapNumber, we still have a value on + // top of the FPU stack. Remove it. + __ bind(&failed_allocation); + __ ffree(); + __ fincstp(); + // Fall through to slow case. + + // Slow case: Load name and receiver from stack and jump to runtime. + __ bind(&slow); + __ IncrementCounter(&Counters::keyed_load_external_array_slow, 1); + Generate(masm, ExternalReference(Runtime::kKeyedGetProperty)); } @@ -487,7 +642,196 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) { void KeyedStoreIC::GenerateExternalArray(MacroAssembler* masm, ExternalArrayType array_type) { - GenerateGeneric(masm); + // ----------- S t a t e ------------- + // -- eax : value + // -- esp[0] : return address + // -- esp[4] : key + // -- esp[8] : receiver + // ----------------------------------- + Label slow, check_heap_number; + + // Get the receiver from the stack. + __ mov(edx, Operand(esp, 2 * kPointerSize)); + // Check that the object isn't a smi. + __ test(edx, Immediate(kSmiTagMask)); + __ j(zero, &slow); + // Get the map from the receiver. + __ mov(ecx, FieldOperand(edx, HeapObject::kMapOffset)); + // Check that the receiver does not require access checks. We need + // to do this because this generic stub does not perform map checks. + __ movzx_b(ebx, FieldOperand(ecx, Map::kBitFieldOffset)); + __ test(ebx, Immediate(1 << Map::kIsAccessCheckNeeded)); + __ j(not_zero, &slow); + // Get the key from the stack. + __ mov(ebx, Operand(esp, 1 * kPointerSize)); // 1 ~ return address + // Check that the key is a smi. + __ test(ebx, Immediate(kSmiTagMask)); + __ j(not_zero, &slow); + // Get the instance type from the map of the receiver. + __ movzx_b(ecx, FieldOperand(ecx, Map::kInstanceTypeOffset)); + // Check that the object is a JS object. + __ cmp(ecx, JS_OBJECT_TYPE); + __ j(not_equal, &slow); + + // Check that the elements array is the appropriate type of + // ExternalArray. + // eax: value + // edx: JSObject + // ebx: index (as a smi) + __ mov(ecx, FieldOperand(edx, JSObject::kElementsOffset)); + Handle map(Heap::MapForExternalArrayType(array_type)); + __ cmp(FieldOperand(ecx, HeapObject::kMapOffset), + Immediate(map)); + __ j(not_equal, &slow); + + // Check that the index is in range. + __ sar(ebx, kSmiTagSize); // Untag the index. + __ cmp(ebx, FieldOperand(ecx, ExternalArray::kLengthOffset)); + // Unsigned comparison catches both negative and too-large values. + __ j(above_equal, &slow); + + // Handle both smis and HeapNumbers in the fast path. Go to the + // runtime for all other kinds of values. + // eax: value + // ecx: elements array + // ebx: untagged index + __ test(eax, Immediate(kSmiTagMask)); + __ j(not_equal, &check_heap_number); + // smi case + __ mov(edx, eax); // Save the value. + __ sar(eax, kSmiTagSize); // Untag the value. + __ mov(ecx, FieldOperand(ecx, ExternalArray::kExternalPointerOffset)); + // ecx: base pointer of external storage + switch (array_type) { + case kExternalByteArray: + case kExternalUnsignedByteArray: + __ mov_b(Operand(ecx, ebx, times_1, 0), eax); + break; + case kExternalShortArray: + case kExternalUnsignedShortArray: + __ mov_w(Operand(ecx, ebx, times_2, 0), eax); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: + __ mov(Operand(ecx, ebx, times_4, 0), eax); + break; + case kExternalFloatArray: + // Need to perform int-to-float conversion. + __ push(eax); + __ fild_s(Operand(esp, 0)); + __ pop(eax); + __ fstp_s(Operand(ecx, ebx, times_4, 0)); + break; + default: + UNREACHABLE(); + break; + } + __ mov(eax, edx); // Return the original value. + __ ret(0); + + __ bind(&check_heap_number); + __ cmp(FieldOperand(eax, HeapObject::kMapOffset), + Immediate(Factory::heap_number_map())); + __ j(not_equal, &slow); + + // The WebGL specification leaves the behavior of storing NaN and + // +/-Infinity into integer arrays basically undefined. For more + // reproducible behavior, convert these to zero. + __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); + __ mov(edx, eax); // Save the value. + __ mov(ecx, FieldOperand(ecx, ExternalArray::kExternalPointerOffset)); + // ebx: untagged index + // ecx: base pointer of external storage + // top of FPU stack: value + if (array_type == kExternalFloatArray) { + __ fstp_s(Operand(ecx, ebx, times_4, 0)); + __ mov(eax, edx); // Return the original value. + __ ret(0); + } else { + // Need to perform float-to-int conversion. + // Test the top of the FP stack for NaN. + Label is_nan; + __ fucomi(0); + __ j(parity_even, &is_nan); + + if (array_type != kExternalUnsignedIntArray) { + __ push(eax); // Make room on stack + __ fistp_s(Operand(esp, 0)); + __ pop(eax); + } else { + // fistp stores values as signed integers. + // To represent the entire range, we need to store as a 64-bit + // int and discard the high 32 bits. + __ push(eax); // Make room on stack + __ push(eax); // Make room on stack + __ fistp_d(Operand(esp, 0)); + __ pop(eax); + __ mov(Operand(esp, 0), eax); + __ pop(eax); + } + // eax: untagged integer value + switch (array_type) { + case kExternalByteArray: + case kExternalUnsignedByteArray: + __ mov_b(Operand(ecx, ebx, times_1, 0), eax); + break; + case kExternalShortArray: + case kExternalUnsignedShortArray: + __ mov_w(Operand(ecx, ebx, times_2, 0), eax); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: { + // We also need to explicitly check for +/-Infinity. These are + // converted to MIN_INT, but we need to be careful not to + // confuse with legal uses of MIN_INT. + Label not_infinity; + // This test would apparently detect both NaN and Infinity, + // but we've already checked for NaN using the FPU hardware + // above. + __ mov_w(edi, FieldOperand(edx, HeapNumber::kValueOffset + 6)); + __ and_(edi, 0x7FF0); + __ cmp(edi, 0x7FF0); + __ j(not_equal, ¬_infinity); + __ mov(eax, 0); + __ bind(¬_infinity); + __ mov(Operand(ecx, ebx, times_4, 0), eax); + break; + } + default: + UNREACHABLE(); + break; + } + __ mov(eax, edx); // Return the original value. + __ ret(0); + + __ bind(&is_nan); + __ ffree(); + __ fincstp(); + switch (array_type) { + case kExternalByteArray: + case kExternalUnsignedByteArray: + __ mov_b(Operand(ecx, ebx, times_1, 0), 0); + break; + case kExternalShortArray: + case kExternalUnsignedShortArray: + __ mov(eax, 0); + __ mov_w(Operand(ecx, ebx, times_2, 0), eax); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: + __ mov(Operand(ecx, ebx, times_4, 0), Immediate(0)); + break; + default: + UNREACHABLE(); + break; + } + __ mov(eax, edx); // Return the original value. + __ ret(0); + } + + // Slow case: call runtime. + __ bind(&slow); + Generate(masm, ExternalReference(Runtime::kSetProperty)); } diff --git a/src/objects.cc b/src/objects.cc index 15a31b1e4e..99a398a797 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -7313,7 +7313,7 @@ static Object* ExternalArrayIntSetter(ExternalArrayClass* receiver, cast_value = static_cast(int_value); } else if (value->IsHeapNumber()) { double double_value = HeapNumber::cast(value)->value(); - cast_value = static_cast(double_value); + cast_value = static_cast(DoubleToInt32(double_value)); } else { // Clamp undefined to zero (default). All other types have been // converted to a number type further up in the call chain. @@ -7365,7 +7365,7 @@ Object* ExternalUnsignedIntArray::SetValue(uint32_t index, Object* value) { cast_value = static_cast(int_value); } else if (value->IsHeapNumber()) { double double_value = HeapNumber::cast(value)->value(); - cast_value = static_cast(double_value); + cast_value = static_cast(DoubleToUint32(double_value)); } else { // Clamp undefined to zero (default). All other types have been // converted to a number type further up in the call chain. diff --git a/src/runtime.cc b/src/runtime.cc index c747659dda..1f8af1e911 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -2580,18 +2580,6 @@ Object* Runtime::GetObjectProperty(Handle object, Handle key) { return GetElementOrCharAt(object, index); } - // If the target object is a JSObject and has an ExternalArray as - // its elements, we need to check for negative indices and report - // exceptions. Indices larger than the array's length will be caught - // elsewhere. - if (key->IsSmi() && Smi::cast(*key)->value() < 0) { - if (object->IsJSObject() && - JSObject::cast(*object)->HasExternalArrayElements()) { - uint32_t index = static_cast(Smi::cast(*key)->value()); - return Top::Throw(*Factory::NewIndexError(index)); - } - } - // Convert the key to a string - possibly by calling back into JavaScript. Handle name; if (key->IsString()) { @@ -2726,18 +2714,6 @@ Object* Runtime::SetObjectProperty(Handle object, return *value; } - // If the target object is a JSObject and has an ExternalArray as - // its elements, we need to check for negative indices and report - // exceptions. Indices larger than the array's length will be caught - // elsewhere. - if (key->IsSmi() && Smi::cast(*key)->value() < 0) { - if (object->IsJSObject() && - JSObject::cast(*object)->HasExternalArrayElements()) { - uint32_t index = static_cast(Smi::cast(*key)->value()); - return Top::Throw(*Factory::NewIndexError(index)); - } - } - if (key->IsString()) { Handle result; if (Handle::cast(key)->AsArrayIndex(&index)) { diff --git a/src/x64/assembler-x64.cc b/src/x64/assembler-x64.cc index 3a8ec72c3d..93b7e90041 100644 --- a/src/x64/assembler-x64.cc +++ b/src/x64/assembler-x64.cc @@ -574,11 +574,11 @@ void Assembler::immediate_arithmetic_op_16(byte subcode, emit(src.value_); } else if (dst.is(rax)) { emit(0x05 | (subcode << 3)); - emitl(src.value_); + emitw(src.value_); } else { emit(0x81); emit_modrm(subcode, dst); - emitl(src.value_); + emitw(src.value_); } } @@ -597,7 +597,7 @@ void Assembler::immediate_arithmetic_op_16(byte subcode, } else { emit(0x81); emit_operand(subcode, dst); - emitl(src.value_); + emitw(src.value_); } } @@ -2229,6 +2229,14 @@ void Assembler::fucompp() { } +void Assembler::fucomi(int i) { + EnsureSpace ensure_space(this); + last_pc_ = pc_; + emit(0xDB); + emit(0xE8 + i); +} + + void Assembler::fucomip() { EnsureSpace ensure_space(this); last_pc_ = pc_; diff --git a/src/x64/assembler-x64.h b/src/x64/assembler-x64.h index 265f839a4a..3477ca6c10 100644 --- a/src/x64/assembler-x64.h +++ b/src/x64/assembler-x64.h @@ -1049,6 +1049,7 @@ class Assembler : public Malloced { void ftst(); void fucomp(int i); void fucompp(); + void fucomi(int i); void fucomip(); void fcompp(); diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc index 499557297a..391d1594d0 100644 --- a/src/x64/ic-x64.cc +++ b/src/x64/ic-x64.cc @@ -362,7 +362,142 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) { void KeyedLoadIC::GenerateExternalArray(MacroAssembler* masm, ExternalArrayType array_type) { - GenerateGeneric(masm); + // ----------- S t a t e ------------- + // -- rsp[0] : return address + // -- rsp[8] : name + // -- rsp[16] : receiver + // ----------------------------------- + Label slow, failed_allocation; + + // Load name and receiver. + __ movq(rax, Operand(rsp, kPointerSize)); + __ movq(rcx, Operand(rsp, 2 * kPointerSize)); + + // Check that the object isn't a smi. + __ JumpIfSmi(rcx, &slow); + + // Check that the key is a smi. + __ JumpIfNotSmi(rax, &slow); + + // Check that the object is a JS object. + __ CmpObjectType(rcx, JS_OBJECT_TYPE, rdx); + __ j(not_equal, &slow); + // Check that the receiver does not require access checks. We need + // to check this explicitly since this generic stub does not perform + // map checks. The map is already in rdx. + __ testb(FieldOperand(rdx, Map::kBitFieldOffset), + Immediate(1 << Map::kIsAccessCheckNeeded)); + __ j(not_zero, &slow); + + // Check that the elements array is the appropriate type of + // ExternalArray. + // rax: index (as a smi) + // rcx: JSObject + __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset)); + __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset), + Heap::RootIndexForExternalArrayType(array_type)); + __ j(not_equal, &slow); + + // Check that the index is in range. + __ SmiToInteger32(rax, rax); + __ cmpl(rax, FieldOperand(rcx, ExternalArray::kLengthOffset)); + // Unsigned comparison catches both negative and too-large values. + __ j(above_equal, &slow); + + // rax: untagged index + // rcx: elements array + __ movq(rcx, FieldOperand(rcx, ExternalArray::kExternalPointerOffset)); + // rcx: base pointer of external storage + switch (array_type) { + case kExternalByteArray: + __ movsxbq(rax, Operand(rcx, rax, times_1, 0)); + break; + case kExternalUnsignedByteArray: + __ movb(rax, Operand(rcx, rax, times_1, 0)); + break; + case kExternalShortArray: + __ movsxwq(rax, Operand(rcx, rax, times_2, 0)); + break; + case kExternalUnsignedShortArray: + __ movzxwq(rax, Operand(rcx, rax, times_2, 0)); + break; + case kExternalIntArray: + __ movsxlq(rax, Operand(rcx, rax, times_4, 0)); + break; + case kExternalUnsignedIntArray: + __ movl(rax, Operand(rcx, rax, times_4, 0)); + break; + case kExternalFloatArray: + __ fld_s(Operand(rcx, rax, times_4, 0)); + break; + default: + UNREACHABLE(); + break; + } + + // For integer array types: + // rax: value + // For floating-point array type: + // FP(0): value + + if (array_type == kExternalIntArray || + array_type == kExternalUnsignedIntArray) { + + // For the Int and UnsignedInt array types, we need to see whether + // the value can be represented in a Smi. If not, we need to convert + // it to a HeapNumber. + Label box_int; + if (array_type == kExternalIntArray) { + __ JumpIfNotValidSmiValue(rax, &box_int); + } else { + ASSERT_EQ(array_type, kExternalUnsignedIntArray); + __ JumpIfUIntNotValidSmiValue(rax, &box_int); + } + + __ Integer32ToSmi(rax, rax); + __ ret(0); + + __ bind(&box_int); + + // Allocate a HeapNumber for the int and perform int-to-double + // conversion. + __ push(rax); + if (array_type == kExternalIntArray) { + __ fild_s(Operand(rsp, 0)); + } else { + ASSERT(array_type == kExternalUnsignedIntArray); + // Need to zero-extend the value. + __ fild_d(Operand(rsp, 0)); + } + __ pop(rax); + // FP(0): value + __ AllocateHeapNumber(rax, rbx, &failed_allocation); + // Set the value. + __ fstp_d(FieldOperand(rax, HeapNumber::kValueOffset)); + __ ret(0); + } else if (array_type == kExternalFloatArray) { + // For the floating-point array type, we need to always allocate a + // HeapNumber. + __ AllocateHeapNumber(rax, rbx, &failed_allocation); + // Set the value. + __ fstp_d(FieldOperand(rax, HeapNumber::kValueOffset)); + __ ret(0); + } else { + __ Integer32ToSmi(rax, rax); + __ ret(0); + } + + // If we fail allocation of the HeapNumber, we still have a value on + // top of the FPU stack. Remove it. + __ bind(&failed_allocation); + __ ffree(); + __ fincstp(); + // Fall through to slow case. + + // Slow case: Load name and receiver from stack and jump to runtime. + __ bind(&slow); + __ IncrementCounter(&Counters::keyed_load_external_array_slow, 1); + Generate(masm, ExternalReference(Runtime::kKeyedGetProperty)); } @@ -560,7 +695,175 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) { void KeyedStoreIC::GenerateExternalArray(MacroAssembler* masm, ExternalArrayType array_type) { - GenerateGeneric(masm); + // ----------- S t a t e ------------- + // -- rax : value + // -- rsp[0] : return address + // -- rsp[8] : key + // -- rsp[16] : receiver + // ----------------------------------- + Label slow, check_heap_number; + + // Get the receiver from the stack. + __ movq(rdx, Operand(rsp, 2 * kPointerSize)); + // Check that the object isn't a smi. + __ JumpIfSmi(rdx, &slow); + // Get the map from the receiver. + __ movq(rcx, FieldOperand(rdx, HeapObject::kMapOffset)); + // Check that the receiver does not require access checks. We need + // to do this because this generic stub does not perform map checks. + __ testb(FieldOperand(rcx, Map::kBitFieldOffset), + Immediate(1 << Map::kIsAccessCheckNeeded)); + __ j(not_zero, &slow); + // Get the key from the stack. + __ movq(rbx, Operand(rsp, 1 * kPointerSize)); // 1 ~ return address + // Check that the key is a smi. + __ JumpIfNotSmi(rbx, &slow); + + // Check that the object is a JS object. + __ CmpInstanceType(rcx, JS_OBJECT_TYPE); + __ j(not_equal, &slow); + + // Check that the elements array is the appropriate type of + // ExternalArray. + // rax: value + // rdx: JSObject + // rbx: index (as a smi) + __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset)); + __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset), + Heap::RootIndexForExternalArrayType(array_type)); + __ j(not_equal, &slow); + + // Check that the index is in range. + __ SmiToInteger32(rbx, rbx); // Untag the index. + __ cmpl(rbx, FieldOperand(rcx, ExternalArray::kLengthOffset)); + // Unsigned comparison catches both negative and too-large values. + __ j(above_equal, &slow); + + // Handle both smis and HeapNumbers in the fast path. Go to the + // runtime for all other kinds of values. + // rax: value + // rcx: elements array + // rbx: untagged index + __ JumpIfNotSmi(rax, &check_heap_number); + __ movq(rdx, rax); // Save the value. + __ SmiToInteger32(rax, rax); + __ movq(rcx, FieldOperand(rcx, ExternalArray::kExternalPointerOffset)); + // rcx: base pointer of external storage + switch (array_type) { + case kExternalByteArray: + case kExternalUnsignedByteArray: + __ movb(Operand(rcx, rbx, times_1, 0), rax); + break; + case kExternalShortArray: + case kExternalUnsignedShortArray: + __ movw(Operand(rcx, rbx, times_2, 0), rax); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: + __ movl(Operand(rcx, rbx, times_4, 0), rax); + break; + case kExternalFloatArray: + // Need to perform int-to-float conversion. + __ push(rax); + __ fild_s(Operand(rsp, 0)); + __ pop(rax); + __ fstp_s(Operand(rcx, rbx, times_4, 0)); + break; + default: + UNREACHABLE(); + break; + } + __ movq(rax, rdx); // Return the original value. + __ ret(0); + + __ bind(&check_heap_number); + __ CmpObjectType(rax, HEAP_NUMBER_TYPE, rdx); + __ j(not_equal, &slow); + + // The WebGL specification leaves the behavior of storing NaN and + // +/-Infinity into integer arrays basically undefined. For more + // reproducible behavior, convert these to zero. + __ fld_d(FieldOperand(rax, HeapNumber::kValueOffset)); + __ movq(rdx, rax); // Save the value. + __ movq(rcx, FieldOperand(rcx, ExternalArray::kExternalPointerOffset)); + // rbx: untagged index + // rcx: base pointer of external storage + // top of FPU stack: value + if (array_type == kExternalFloatArray) { + __ fstp_s(Operand(rcx, rbx, times_4, 0)); + } else { + // Need to perform float-to-int conversion. + // Test the top of the FP stack for NaN. + Label is_nan; + __ fucomi(0); + __ j(parity_even, &is_nan); + + __ push(rax); // Make room on stack + __ fistp_d(Operand(rsp, 0)); + __ pop(rax); + // rax: untagged integer value + switch (array_type) { + case kExternalByteArray: + case kExternalUnsignedByteArray: + __ movb(Operand(rcx, rbx, times_1, 0), rax); + break; + case kExternalShortArray: + case kExternalUnsignedShortArray: + __ movw(Operand(rcx, rbx, times_2, 0), rax); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: { + // We also need to explicitly check for +/-Infinity. These are + // converted to MIN_INT, but we need to be careful not to + // confuse with legal uses of MIN_INT. + Label not_infinity; + // This test would apparently detect both NaN and Infinity, + // but we've already checked for NaN using the FPU hardware + // above. + __ movzxwq(rdi, FieldOperand(rdx, HeapNumber::kValueOffset + 6)); + __ and_(rdi, Immediate(0x7FF0)); + __ cmpw(rdi, Immediate(0x7FF0)); + __ j(not_equal, ¬_infinity); + __ movq(rax, Immediate(0)); + __ bind(¬_infinity); + __ movl(Operand(rcx, rbx, times_4, 0), rax); + break; + } + default: + UNREACHABLE(); + break; + } + __ movq(rax, rdx); // Return the original value. + __ ret(0); + + __ bind(&is_nan); + __ ffree(); + __ fincstp(); + __ movq(rax, Immediate(0)); + switch (array_type) { + case kExternalByteArray: + case kExternalUnsignedByteArray: + __ movb(Operand(rcx, rbx, times_1, 0), rax); + break; + case kExternalShortArray: + case kExternalUnsignedShortArray: + __ movw(Operand(rcx, rbx, times_2, 0), rax); + break; + case kExternalIntArray: + case kExternalUnsignedIntArray: + __ movl(Operand(rcx, rbx, times_4, 0), rax); + break; + default: + UNREACHABLE(); + break; + } + __ movq(rax, rdx); // Return the original value. + __ ret(0); + } + + // Slow case: call runtime. + __ bind(&slow); + Generate(masm, ExternalReference(Runtime::kSetProperty)); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 41bd1bb62e..15d48d395d 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8197,15 +8197,6 @@ static void ExternalArrayTestHelper(v8::ExternalArrayType array_type, CHECK_EQ(28, result->Int32Value()); // Check out-of-range loads. - result = CompileRun("var caught_exception = false;" - "try {" - " ext_array[-1];" - "} catch (e) {" - " caught_exception = true;" - "}" - "caught_exception;"); - CHECK_EQ(true, result->BooleanValue()); - i::OS::SNPrintF(test_buf, "var caught_exception = false;" "try {" @@ -8219,15 +8210,6 @@ static void ExternalArrayTestHelper(v8::ExternalArrayType array_type, CHECK_EQ(true, result->BooleanValue()); // Check out-of-range stores. - result = CompileRun("var caught_exception = false;" - "try {" - " ext_array[-1] = 1;" - "} catch (e) {" - " caught_exception = true;" - "}" - "caught_exception;"); - CHECK_EQ(true, result->BooleanValue()); - i::OS::SNPrintF(test_buf, "var caught_exception = false;" "try {" @@ -8240,9 +8222,6 @@ static void ExternalArrayTestHelper(v8::ExternalArrayType array_type, result = CompileRun(test_buf.start()); CHECK_EQ(true, result->BooleanValue()); - // TODO(kbr): check what happens during IC misses on the type of the object. - // Re-assign array object halfway through a loop. - // Check other boundary conditions, values and operations. result = CompileRun("for (var i = 0; i < 8; i++) {" " ext_array[7] = undefined;" @@ -8258,9 +8237,39 @@ static void ExternalArrayTestHelper(v8::ExternalArrayType array_type, CHECK_EQ(2, result->Int32Value()); CHECK_EQ(2, static_cast(jsobj->GetElement(6)->Number())); - // Result for storing NaNs and +/-Infinity isn't defined for these - // types; they do not have the clamping behavior CanvasPixelArray - // specifies. + if (array_type != v8::kExternalFloatArray) { + // Though the specification doesn't state it, be explicit about + // converting NaNs and +/-Infinity to zero. + result = CompileRun("for (var i = 0; i < 8; i++) {" + " ext_array[i] = 5;" + "}" + "for (var i = 0; i < 8; i++) {" + " ext_array[i] = NaN;" + "}" + "ext_array[5];"); + CHECK_EQ(0, result->Int32Value()); + CHECK_EQ(0, i::Smi::cast(jsobj->GetElement(5))->value()); + + result = CompileRun("for (var i = 0; i < 8; i++) {" + " ext_array[i] = 5;" + "}" + "for (var i = 0; i < 8; i++) {" + " ext_array[i] = Infinity;" + "}" + "ext_array[5];"); + CHECK_EQ(0, result->Int32Value()); + CHECK_EQ(0, i::Smi::cast(jsobj->GetElement(5))->value()); + + result = CompileRun("for (var i = 0; i < 8; i++) {" + " ext_array[i] = 5;" + "}" + "for (var i = 0; i < 8; i++) {" + " ext_array[i] = -Infinity;" + "}" + "ext_array[5];"); + CHECK_EQ(0, result->Int32Value()); + CHECK_EQ(0, i::Smi::cast(jsobj->GetElement(5))->value()); + } result = CompileRun("ext_array[3] = 33;" "delete ext_array[3];"