Revision: 6528
Author: [email protected]
Date: Fri Jan 28 07:07:04 2011
Log: Revert "Add custom typed ICs for pixel array loads. "
This change caused failures in (out of bounds) keyed loads of strings.
TBR'd.
Review URL: http://codereview.chromium.org/6298019
http://code.google.com/p/v8/source/detail?r=6528
Modified:
/branches/bleeding_edge/src/arm/ic-arm.cc
/branches/bleeding_edge/src/builtins.cc
/branches/bleeding_edge/src/builtins.h
/branches/bleeding_edge/src/ia32/ic-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/ic.h
/branches/bleeding_edge/src/x64/ic-x64.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/arm/ic-arm.cc Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/arm/ic-arm.cc Fri Jan 28 07:07:04 2011
@@ -499,66 +499,6 @@
__ b(eq, out_of_range);
__ mov(result, scratch2);
}
-
-
-static void GenerateFastPixelArrayLoad(MacroAssembler* masm,
- Register receiver,
- Register key,
- Register elements_map,
- Register elements,
- Register scratch1,
- Register scratch2,
- Register result,
- Label* not_pixel_array,
- Label* key_not_smi,
- Label* out_of_range) {
- // Register use:
- //
- // receiver - holds the receiver on entry.
- // Unchanged unless 'result' is the same register.
- //
- // key - holds the smi key on entry.
- // Unchanged unless 'result' is the same register.
- //
- // elements - set to be the receiver's elements on exit.
- //
- // elements_map - set to be the map of the receiver's elements
- // on exit.
- //
- // result - holds the result of the pixel array load on exit,
- // tagged as a smi if successful.
- //
- // Scratch registers:
- //
- // scratch1 - holds the receiver's elements, the length of the
- // pixel array, the pointer to external elements and
- // the untagged result.
- //
- // scratch2 - holds the untaged key.
-
- // Verify that the receiver has pixel array elements.
- __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset));
- __ LoadRoot(scratch1, Heap::kPixelArrayMapRootIndex);
- __ ldr(elements_map, FieldMemOperand(elements, JSObject::kMapOffset));
- __ cmp(elements_map, scratch1);
- __ b(ne, not_pixel_array);
-
- // Key must be a smi that is in the range of the pixel array.
- if (key_not_smi != NULL) {
- __ JumpIfNotSmi(key, key_not_smi);
- }
- __ ldr(scratch1, FieldMemOperand(elements, PixelArray::kLengthOffset));
- __ SmiUntag(scratch2, key);
- __ cmp(scratch2, scratch1);
- __ b(hs, out_of_range);
-
- // Perform the indexed load and tag the result as a smi.
- __ ldr(scratch1,
- FieldMemOperand(elements, PixelArray::kExternalPointerOffset));
- __ ldrb(scratch1, MemOperand(scratch1, scratch2));
- __ SmiTag(r0, scratch1);
- __ Ret();
-}
// Checks whether a key is an array index string or a symbol string.
@@ -1249,18 +1189,19 @@
// r0: key
// r1: receiver
__ bind(&check_pixel_array);
-
- GenerateFastPixelArrayLoad(masm,
- r1,
- r0,
- r3,
- r4,
- r2,
- r5,
- r0,
- &check_number_dictionary,
- NULL,
- &slow);
+ __ ldr(r4, FieldMemOperand(r1, JSObject::kElementsOffset));
+ __ ldr(r3, FieldMemOperand(r4, HeapObject::kMapOffset));
+ __ LoadRoot(ip, Heap::kPixelArrayMapRootIndex);
+ __ cmp(r3, ip);
+ __ b(ne, &check_number_dictionary);
+ __ ldr(ip, FieldMemOperand(r4, PixelArray::kLengthOffset));
+ __ mov(r2, Operand(key, ASR, kSmiTagSize));
+ __ cmp(r2, ip);
+ __ b(hs, &slow);
+ __ ldr(ip, FieldMemOperand(r4, PixelArray::kExternalPointerOffset));
+ __ ldrb(r2, MemOperand(ip, r2));
+ __ mov(r0, Operand(r2, LSL, kSmiTagSize)); // Tag result as smi.
+ __ Ret();
__ bind(&check_number_dictionary);
// Check whether the elements is a number dictionary.
@@ -1432,41 +1373,6 @@
__ bind(&slow);
GenerateMiss(masm);
}
-
-
-void KeyedLoadIC::GeneratePixelArray(MacroAssembler* masm) {
- // ---------- S t a t e --------------
- // -- lr : return address
- // -- r0 : key
- // -- r1 : receiver
- // -----------------------------------
-
- // Register usage.
- Register key = r0;
- Register receiver = r1;
-
- Label slow;
-
- // Verify that it's safe to access the receiver's elements.
- GenerateKeyedLoadReceiverCheck(
- masm, receiver, r5, r6,
- Map::kHasIndexedInterceptor, &slow);
-
- GenerateFastPixelArrayLoad(masm,
- receiver,
- key,
- r2,
- r3,
- r4,
- r5,
- r0,
- &slow,
- &slow,
- &slow);
-
- __ bind(&slow);
- GenerateMiss(masm);
-}
void KeyedStoreIC::GenerateMiss(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/src/builtins.cc Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/builtins.cc Fri Jan 28 07:07:04 2011
@@ -1285,11 +1285,6 @@
static void Generate_KeyedLoadIC_String(MacroAssembler* masm) {
KeyedLoadIC::GenerateString(masm);
}
-
-
-static void Generate_KeyedLoadIC_PixelArray(MacroAssembler* masm) {
- KeyedLoadIC::GeneratePixelArray(masm);
-}
static void Generate_KeyedLoadIC_PreMonomorphic(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/src/builtins.h Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/builtins.h Fri Jan 28 07:07:04 2011
@@ -94,7 +94,6 @@
V(KeyedLoadIC_PreMonomorphic, KEYED_LOAD_IC, PREMONOMORPHIC) \
V(KeyedLoadIC_Generic, KEYED_LOAD_IC, MEGAMORPHIC) \
V(KeyedLoadIC_String, KEYED_LOAD_IC, MEGAMORPHIC) \
- V(KeyedLoadIC_PixelArray, KEYED_LOAD_IC, MONOMORPHIC) \
V(KeyedLoadIC_IndexedInterceptor, KEYED_LOAD_IC, MEGAMORPHIC) \
\
V(StoreIC_Initialize, STORE_IC, UNINITIALIZED) \
=======================================
--- /branches/bleeding_edge/src/ia32/ic-ia32.cc Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Fri Jan 28 07:07:04 2011
@@ -491,48 +491,6 @@
}
}
-// Loads a indexed element from a pixel array.
-static void GenerateFastPixelArrayLoad(MacroAssembler* masm,
- Register receiver,
- Register key,
- Register elements,
- Register untagged_key,
- Register result,
- Label* not_pixel_array,
- Label* key_not_smi,
- Label* out_of_range) {
- // Register use:
- // receiver - holds the receiver and is unchanged.
- // key - holds the key and is unchanged (must be a smi).
- // elements - is set to the the receiver's element if
- // the receiver doesn't have a pixel array or the
- // key is not a smi, otherwise it's the elements'
- // external pointer.
- // untagged_key - is set to the untagged key
-
- // Key must be a smi.
- if (key_not_smi != NULL) {
- __ test(key, Immediate(kSmiTagMask));
- __ j(not_zero, key_not_smi, not_taken);
- }
- __ mov(untagged_key, key);
- __ SmiUntag(untagged_key);
-
- // Verify that the receiver has pixel array elements.
- __ mov(elements, FieldOperand(receiver, JSObject::kElementsOffset));
- __ CheckMap(elements, Factory::pixel_array_map(), not_pixel_array, true);
-
- // Key must be in range.
- __ cmp(untagged_key, FieldOperand(elements, PixelArray::kLengthOffset));
- __ j(above_equal, out_of_range);
-
- // Perform the indexed load and tag the result as a smi.
- __ mov(elements, FieldOperand(elements,
PixelArray::kExternalPointerOffset));
- __ movzx_b(result, Operand(elements, untagged_key, times_1, 0));
- __ SmiTag(result);
- __ ret(0);
-}
-
// Checks whether a key is an array index string or a symbol string.
// Falls through if the key is a symbol.
@@ -598,15 +556,19 @@
__ ret(0);
__ bind(&check_pixel_array);
- GenerateFastPixelArrayLoad(masm,
- edx,
- eax,
- ecx,
- ebx,
- eax,
- &check_number_dictionary,
- NULL,
- &slow);
+ // Check whether the elements is a pixel array.
+ // edx: receiver
+ // eax: key
+ __ mov(ecx, FieldOperand(edx, JSObject::kElementsOffset));
+ __ mov(ebx, eax);
+ __ SmiUntag(ebx);
+ __ CheckMap(ecx, Factory::pixel_array_map(), &check_number_dictionary,
true);
+ __ cmp(ebx, FieldOperand(ecx, PixelArray::kLengthOffset));
+ __ j(above_equal, &slow);
+ __ mov(eax, FieldOperand(ecx, PixelArray::kExternalPointerOffset));
+ __ movzx_b(eax, Operand(eax, ebx, times_1, 0));
+ __ SmiTag(eax);
+ __ ret(0);
__ bind(&check_number_dictionary);
// Check whether the elements is a number dictionary.
@@ -798,33 +760,6 @@
__ bind(&slow);
GenerateMiss(masm);
}
-
-
-void KeyedLoadIC::GeneratePixelArray(MacroAssembler* masm) {
- // ----------- S t a t e -------------
- // -- eax : key
- // -- edx : receiver
- // -- esp[0] : return address
- // -----------------------------------
- Label slow;
-
- // Verify that it's safe to access the receiver's elements.
- GenerateKeyedLoadReceiverCheck(
- masm, edx, ecx, Map::kHasNamedInterceptor, &slow);
-
- GenerateFastPixelArrayLoad(masm,
- edx,
- eax,
- ecx,
- ebx,
- eax,
- &slow,
- &slow,
- &slow);
-
- __ bind(&slow);
- GenerateMiss(masm);
-}
void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/src/ic.cc Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/ic.cc Fri Jan 28 07:07:04 2011
@@ -1204,27 +1204,23 @@
if (use_ic) {
Code* stub = generic_stub();
- if (state == UNINITIALIZED) {
- if (object->IsString() && key->IsNumber()) {
- stub = string_stub();
- } else if (object->IsJSObject()) {
- Handle<JSObject> receiver = Handle<JSObject>::cast(object);
- if (receiver->HasExternalArrayElements()) {
- MaybeObject* probe =
+ if (object->IsString() && key->IsNumber()) {
+ stub = string_stub();
+ } else if (object->IsJSObject()) {
+ Handle<JSObject> receiver = Handle<JSObject>::cast(object);
+ if (receiver->HasExternalArrayElements()) {
+ MaybeObject* probe =
StubCache::ComputeKeyedLoadOrStoreExternalArray(*receiver,
false);
- stub =
+ stub =
probe->IsFailure() ? NULL :
Code::cast(probe->ToObjectUnchecked());
- } else if (receiver->HasPixelElements()) {
- stub = pixel_array_stub();
- } else if (receiver->HasIndexedInterceptor()) {
- stub = indexed_interceptor_stub();
- } else if (key->IsSmi() &&
- receiver->map()->has_fast_elements()) {
- MaybeObject* probe =
- StubCache::ComputeKeyedLoadSpecialized(*receiver);
- stub =
+ } else if (receiver->HasIndexedInterceptor()) {
+ stub = indexed_interceptor_stub();
+ } else if (state == UNINITIALIZED &&
+ key->IsSmi() &&
+ receiver->map()->has_fast_elements()) {
+ MaybeObject* probe =
StubCache::ComputeKeyedLoadSpecialized(*receiver);
+ stub =
probe->IsFailure() ? NULL :
Code::cast(probe->ToObjectUnchecked());
- }
}
}
if (stub != NULL) set_target(stub);
=======================================
--- /branches/bleeding_edge/src/ic.h Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/ic.h Fri Jan 28 07:07:04 2011
@@ -348,9 +348,6 @@
static void GenerateIndexedInterceptor(MacroAssembler* masm);
- // Generator for loading bytes from a pixel array.
- static void GeneratePixelArray(MacroAssembler* masm);
-
// Clear the use of the inlined version.
static void ClearInlinedVersion(Address address);
@@ -385,9 +382,6 @@
return Builtins::builtin(Builtins::KeyedLoadIC_String);
}
- static Code* pixel_array_stub() {
- return Builtins::builtin(Builtins::KeyedLoadIC_PixelArray);
- }
static Code* indexed_interceptor_stub() {
return Builtins::builtin(Builtins::KeyedLoadIC_IndexedInterceptor);
}
=======================================
--- /branches/bleeding_edge/src/x64/ic-x64.cc Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/src/x64/ic-x64.cc Fri Jan 28 07:07:04 2011
@@ -513,49 +513,6 @@
__ movq(result, scratch);
}
}
-
-
-// Loads a indexed element from a pixel array.
-static void GenerateFastPixelArrayLoad(MacroAssembler* masm,
- Register receiver,
- Register key,
- Register elements,
- Register untagged_key,
- Register result,
- Label* not_pixel_array,
- Label* key_not_smi,
- Label* out_of_range) {
- // Register use:
- // receiver - holds the receiver and is unchanged.
- // key - holds the key and is unchanged (must be a smi).
- // elements - is set to the the receiver's element if
- // the receiver doesn't have a pixel array or the
- // key is not a smi, otherwise it's the elements'
- // external pointer.
- // untagged_key - is set to the untagged key
-
- // Check that the key is a smi.
- if (key_not_smi != NULL) {
- __ JumpIfNotSmi(key, key_not_smi);
- }
- __ SmiToInteger32(untagged_key, key);
-
- // Verify that the receiver has pixel array elements.
- __ movq(elements, FieldOperand(receiver, JSObject::kElementsOffset));
- __ CompareRoot(FieldOperand(elements, HeapObject::kMapOffset),
- Heap::kPixelArrayMapRootIndex);
- __ j(not_equal, not_pixel_array);
-
- // Check that the smi is in range.
- __ cmpl(untagged_key, FieldOperand(elements, PixelArray::kLengthOffset));
- __ j(above_equal, out_of_range);
-
- // Load and tag the element as a smi.
- __ movq(elements, FieldOperand(elements,
PixelArray::kExternalPointerOffset));
- __ movzxbq(result, Operand(elements, untagged_key, times_1, 0));
- __ Integer32ToSmi(result, result);
- __ ret(0);
-}
// Checks whether a key is an array index string or a symbol string.
@@ -623,15 +580,20 @@
__ ret(0);
__ bind(&check_pixel_array);
- GenerateFastPixelArrayLoad(masm,
- rdx,
- rax,
- rcx,
- rbx,
- rax,
- &check_number_dictionary,
- NULL,
- &slow);
+ // Check whether the elements object is a pixel array.
+ // rdx: receiver
+ // rax: key
+ __ movq(rcx, FieldOperand(rdx, JSObject::kElementsOffset));
+ __ SmiToInteger32(rbx, rax); // Used on both directions of next branch.
+ __ CompareRoot(FieldOperand(rcx, HeapObject::kMapOffset),
+ Heap::kPixelArrayMapRootIndex);
+ __ j(not_equal, &check_number_dictionary);
+ __ cmpl(rbx, FieldOperand(rcx, PixelArray::kLengthOffset));
+ __ j(above_equal, &slow);
+ __ movq(rax, FieldOperand(rcx, PixelArray::kExternalPointerOffset));
+ __ movzxbq(rax, Operand(rax, rbx, times_1, 0));
+ __ Integer32ToSmi(rax, rax);
+ __ ret(0);
__ bind(&check_number_dictionary);
// Check whether the elements is a number dictionary.
@@ -804,33 +766,6 @@
__ bind(&slow);
GenerateMiss(masm);
}
-
-void KeyedLoadIC::GeneratePixelArray(MacroAssembler* masm) {
- // ----------- S t a t e -------------
- // -- rax : key
- // -- rdx : receiver
- // -- rsp[0] : return address
- // -----------------------------------
- Label slow;
-
- // Verify that it's safe to access the receiver's elements.
- GenerateKeyedLoadReceiverCheck(
- masm, rdx, rcx, Map::kHasNamedInterceptor, &slow);
-
- // Generate the indexed load from the pixel array.
- GenerateFastPixelArrayLoad(masm,
- rdx,
- rax,
- rcx,
- rbx,
- rax,
- &slow,
- &slow,
- &slow);
-
- __ bind(&slow);
- GenerateMiss(masm);
-}
void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Fri Jan 28 06:06:20 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Jan 28 07:07:04 2011
@@ -10344,93 +10344,6 @@
"i");
CHECK_EQ(255, result->Int32Value());
- // Make sure that pixel array ICs recognize when a non-pixel array
- // is passed to it.
- result = CompileRun("function pa_load(p) {"
- " var sum = 0;"
- " for (var j = 0; j < 256; j++) { sum += p[j]; }"
- " return sum;"
- "}"
- "for (var i = 0; i < 256; ++i) { pixels[i] = i; }"
- "for (var i = 0; i < 10; ++i) { pa_load(pixels); }"
- "just_ints = new Object();"
- "for (var i = 0; i < 256; ++i) { just_ints[i] = i; }"
- "for (var i = 0; i < 10; ++i) {"
- " result = pa_load(just_ints);"
- "}"
- "result");
- CHECK_EQ(32640, result->Int32Value());
-
- // Make sure that pixel array ICs recognize out-of-bound accesses.
- result = CompileRun("function pa_load(p, start) {"
- " var sum = 0;"
- " for (var j = start; j < 256; j++) { sum += p[j];
}"
- " return sum;"
- "}"
- "for (var i = 0; i < 256; ++i) { pixels[i] = i; }"
- "for (var i = 0; i < 10; ++i) { pa_load(pixels,0); }"
- "for (var i = 0; i < 10; ++i) {"
- " result = pa_load(pixels,-10);"
- "}"
- "result");
- CHECK_EQ(0, result->Int32Value());
-
- // Make sure that generic ICs properly handles a pixel array.
- result = CompileRun("function pa_load(p) {"
- " var sum = 0;"
- " for (var j = 0; j < 256; j++) { sum += p[j]; }"
- " return sum;"
- "}"
- "for (var i = 0; i < 256; ++i) { pixels[i] = i; }"
- "just_ints = new Object();"
- "for (var i = 0; i < 256; ++i) { just_ints[i] = i; }"
- "for (var i = 0; i < 10; ++i) { pa_load(just_ints);
}"
- "for (var i = 0; i < 10; ++i) {"
- " result = pa_load(pixels);"
- "}"
- "result");
- CHECK_EQ(32640, result->Int32Value());
-
- // Make sure that generic load ICs recognize out-of-bound accesses in
- // pixel arrays.
- result = CompileRun("function pa_load(p, start) {"
- " var sum = 0;"
- " for (var j = start; j < 256; j++) { sum += p[j];
}"
- " return sum;"
- "}"
- "for (var i = 0; i < 256; ++i) { pixels[i] = i; }"
- "just_ints = new Object();"
- "for (var i = 0; i < 256; ++i) { just_ints[i] = i; }"
- "for (var i = 0; i < 10; ++i) {
pa_load(just_ints,0); }"
- "for (var i = 0; i < 10; ++i) { pa_load(pixels,0); }"
- "for (var i = 0; i < 10; ++i) {"
- " result = pa_load(pixels,-10);"
- "}"
- "result");
- CHECK_EQ(0, result->Int32Value());
-
- // Make sure that generic ICs properly handles other types than pixel
- // arrays (that the inlined fast pixel array test leaves the right
information
- // in the right registers).
- result = CompileRun("function pa_load(p) {"
- " var sum = 0;"
- " for (var j = 0; j < 256; j++) { sum += p[j]; }"
- " return sum;"
- "}"
- "for (var i = 0; i < 256; ++i) { pixels[i] = i; }"
- "just_ints = new Object();"
- "for (var i = 0; i < 256; ++i) { just_ints[i] = i; }"
- "for (var i = 0; i < 10; ++i) { pa_load(just_ints);
}"
- "for (var i = 0; i < 10; ++i) { pa_load(pixels); }"
- "sparse_array = new Object();"
- "for (var i = 0; i < 256; ++i) { sparse_array[i] =
i; }"
- "sparse_array[1000000] = 3;"
- "for (var i = 0; i < 10; ++i) {"
- " result = pa_load(sparse_array);"
- "}"
- "result");
- CHECK_EQ(32640, result->Int32Value());
-
free(pixel_data);
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
