Revert "Add custom typed ICs for pixel array loads. "
authorkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 28 Jan 2011 15:07:04 +0000 (15:07 +0000)
committerkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 28 Jan 2011 15:07:04 +0000 (15:07 +0000)
This change caused failures in (out of bounds) keyed loads of strings.

TBR'd.

Review URL: http://codereview.chromium.org/6298019

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

src/arm/ic-arm.cc
src/builtins.cc
src/builtins.h
src/ia32/ic-ia32.cc
src/ic.cc
src/ic.h
src/x64/ic-x64.cc
test/cctest/test-api.cc

index 45a4119..d74468c 100644 (file)
@@ -501,66 +501,6 @@ static void GenerateFastArrayLoad(MacroAssembler* masm,
 }
 
 
-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.
 // Falls through if a key is a symbol.
 static void GenerateKeyStringCheck(MacroAssembler* masm,
@@ -1249,18 +1189,19 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   // 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.
@@ -1434,41 +1375,6 @@ void KeyedLoadIC::GenerateIndexedInterceptor(MacroAssembler* 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) {
   // ---------- S t a t e --------------
   //  -- r0     : value
index e93bdb4..58dd439 100644 (file)
@@ -1287,11 +1287,6 @@ static void Generate_KeyedLoadIC_String(MacroAssembler* masm) {
 }
 
 
-static void Generate_KeyedLoadIC_PixelArray(MacroAssembler* masm) {
-  KeyedLoadIC::GeneratePixelArray(masm);
-}
-
-
 static void Generate_KeyedLoadIC_PreMonomorphic(MacroAssembler* masm) {
   KeyedLoadIC::GeneratePreMonomorphic(masm);
 }
index 5c5ffd8..88d31c7 100644 (file)
@@ -94,7 +94,6 @@ enum BuiltinExtraArguments {
   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)                  \
index 10144e7..c136977 100644 (file)
@@ -491,48 +491,6 @@ static void GenerateFastArrayLoad(MacroAssembler* masm,
   }
 }
 
-// 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 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   __ 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.
@@ -800,33 +762,6 @@ void KeyedLoadIC::GenerateIndexedInterceptor(MacroAssembler* 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) {
   // ----------- S t a t e -------------
   //  -- eax    : value
index 194b188..9a277d6 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1204,27 +1204,23 @@ MaybeObject* KeyedLoadIC::Load(State state,
 
   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);
index 1634e94..409ad38 100644 (file)
--- a/src/ic.h
+++ b/src/ic.h
@@ -348,9 +348,6 @@ class KeyedLoadIC: public IC {
 
   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 @@ class KeyedLoadIC: public IC {
     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);
   }
index 2fb4be8..d060e31 100644 (file)
@@ -515,49 +515,6 @@ static void GenerateFastArrayLoad(MacroAssembler* masm,
 }
 
 
-// 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.
 // Falls through if the key is a symbol.
 static void GenerateKeyStringCheck(MacroAssembler* masm,
@@ -623,15 +580,20 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   __ 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.
@@ -805,33 +767,6 @@ void KeyedLoadIC::GenerateIndexedInterceptor(MacroAssembler* masm) {
   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) {
   // ----------- S t a t e -------------
index bfa6bfa..e06062b 100644 (file)
@@ -10344,93 +10344,6 @@ THREADED_TEST(PixelArray) {
                       "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);
 }