Add fast-path OOB support to KeyedLoadIC_Generic

R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/852313002

Cr-Commit-Position: refs/heads/master@{#26111}

src/ic/arm/ic-arm.cc

@@ -158,12 +158,10 @@ static void GenerateKeyedLoadReceiverCheck(MacroAssembler* masm,
 
 
 // Loads an indexed element from a fast case array.
-// If not_fast_array is NULL, doesn't perform the elements map check.
 static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
                                   Register key, Register elements,
                                   Register scratch1, Register scratch2,
-                                  Register result, Label* not_fast_array,
-                                  Label* out_of_range) {
+                                  Register result, Label* slow) {
   // Register use:
   //
   // receiver - holds the receiver on entry.
@@ -172,8 +170,6 @@ static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
   // key      - holds the smi key on entry.
   //            Unchanged unless 'result' is the same register.
   //
-  // elements - holds the elements of the receiver on exit.
-  //
   // result   - holds the result on exit if the load succeeded.
   //            Allowed to be the the same as 'receiver' or 'key'.
   //            Unchanged on bailout so 'receiver' and 'key' can be safely
@@ -181,34 +177,59 @@ static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
   //
   // Scratch registers:
   //
-  // scratch1 - used to hold elements map and elements length.
-  //            Holds the elements map if not_fast_array branch is taken.
+  // elements - holds the elements of the receiver and its prototypes.
+  //
+  // scratch1 - used to hold elements length, bit fields, base addresses.
   //
-  // scratch2 - used to hold the loaded value.
+  // scratch2 - used to hold maps, prototypes, and the loaded value.
+  Label check_prototypes, check_next_prototype;
+  Label done, in_bounds, return_undefined;
 
   __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset));
-  if (not_fast_array != NULL) {
-    // Check that the object is in fast mode and writable.
-    __ ldr(scratch1, FieldMemOperand(elements, HeapObject::kMapOffset));
-    __ LoadRoot(ip, Heap::kFixedArrayMapRootIndex);
-    __ cmp(scratch1, ip);
-    __ b(ne, not_fast_array);
-  } else {
-    __ AssertFastElements(elements);
-  }
+  __ AssertFastElements(elements);
+
   // Check that the key (index) is within bounds.
   __ ldr(scratch1, FieldMemOperand(elements, FixedArray::kLengthOffset));
   __ cmp(key, Operand(scratch1));
-  __ b(hs, out_of_range);
+  __ b(lo, &in_bounds);
+  // Out-of-bounds. Check the prototype chain to see if we can just return
+  // 'undefined'.
+  __ cmp(key, Operand(0));
+  __ b(lt, slow);  // Negative keys can't take the fast OOB path.
+  __ bind(&check_prototypes);
+  __ ldr(scratch2, FieldMemOperand(receiver, HeapObject::kMapOffset));
+  __ bind(&check_next_prototype);
+  __ ldr(scratch2, FieldMemOperand(scratch2, Map::kPrototypeOffset));
+  // scratch2: current prototype
+  __ CompareRoot(scratch2, Heap::kNullValueRootIndex);
+  __ b(eq, &return_undefined);
+  __ ldr(elements, FieldMemOperand(scratch2, JSObject::kElementsOffset));
+  __ ldr(scratch2, FieldMemOperand(scratch2, HeapObject::kMapOffset));
+  // elements: elements of current prototype
+  // scratch2: map of current prototype
+  __ CompareInstanceType(scratch2, scratch1, JS_OBJECT_TYPE);
+  __ b(lo, slow);
+  __ ldrb(scratch1, FieldMemOperand(scratch2, Map::kBitFieldOffset));
+  __ tst(scratch1, Operand((1 << Map::kIsAccessCheckNeeded) |
+                           (1 << Map::kHasIndexedInterceptor)));
+  __ b(ne, slow);
+  __ CompareRoot(elements, Heap::kEmptyFixedArrayRootIndex);
+  __ b(ne, slow);
+  __ jmp(&check_next_prototype);
+
+  __ bind(&return_undefined);
+  __ LoadRoot(result, Heap::kUndefinedValueRootIndex);
+  __ jmp(&done);
+
+  __ bind(&in_bounds);
   // Fast case: Do the load.
   __ add(scratch1, elements, Operand(FixedArray::kHeaderSize - kHeapObjectTag));
   __ ldr(scratch2, MemOperand::PointerAddressFromSmiKey(scratch1, key));
-  __ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
-  __ cmp(scratch2, ip);
-  // In case the loaded value is the_hole we have to consult GetProperty
-  // to ensure the prototype chain is searched.
-  __ b(eq, out_of_range);
+  __ CompareRoot(scratch2, Heap::kTheHoleValueRootIndex);
+  // In case the loaded value is the_hole we have to check the prototype chain.
+  __ b(eq, &check_prototypes);
   __ mov(result, scratch2);
+  __ bind(&done);
 }
 
 
@@ -468,7 +489,7 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   // Check the receiver's map to see if it has fast elements.
   __ CheckFastElements(r0, r3, &check_number_dictionary);
 
-  GenerateFastArrayLoad(masm, receiver, key, r0, r3, r4, r0, NULL, &slow);
+  GenerateFastArrayLoad(masm, receiver, key, r0, r3, r4, r0, &slow);
   __ IncrementCounter(isolate->counters()->keyed_load_generic_smi(), 1, r4, r3);
   __ Ret();
 

src/ic/arm64/ic-arm64.cc

@@ -151,63 +151,77 @@ static void GenerateKeyedLoadReceiverCheck(MacroAssembler* masm,
 
 
 // Loads an indexed element from a fast case array.
-// If not_fast_array is NULL, doesn't perform the elements map check.
 //
-// receiver     - holds the receiver on entry.
-//                Unchanged unless 'result' is the same register.
+// 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.
+// key      - holds the smi key on entry.
+//            Unchanged unless 'result' is the same register.
 //
-// elements     - holds the elements of the receiver on exit.
+// elements - holds the elements of the receiver and its prototypes. Clobbered.
 //
-// elements_map - holds the elements map on exit if the not_fast_array branch is
-//                taken. Otherwise, this is used as a scratch register.
-//
-// result       - holds the result on exit if the load succeeded.
-//                Allowed to be the the same as 'receiver' or 'key'.
-//                Unchanged on bailout so 'receiver' and 'key' can be safely
-//                used by further computation.
+// result   - holds the result on exit if the load succeeded.
+//            Allowed to be the the same as 'receiver' or 'key'.
+//            Unchanged on bailout so 'receiver' and 'key' can be safely
+//            used by further computation.
 static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
                                   Register key, Register elements,
-                                  Register elements_map, Register scratch2,
-                                  Register result, Label* not_fast_array,
-                                  Label* slow) {
-  DCHECK(!AreAliased(receiver, key, elements, elements_map, scratch2));
+                                  Register scratch1, Register scratch2,
+                                  Register result, Label* slow) {
+  DCHECK(!AreAliased(receiver, key, elements, scratch1, scratch2));
+
+  Label check_prototypes, check_next_prototype;
+  Label done, in_bounds, return_undefined;
 
   // Check for fast array.
   __ Ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset));
-  if (not_fast_array != NULL) {
-    // Check that the object is in fast mode and writable.
-    __ Ldr(elements_map, FieldMemOperand(elements, HeapObject::kMapOffset));
-    __ JumpIfNotRoot(elements_map, Heap::kFixedArrayMapRootIndex,
-                     not_fast_array);
-  } else {
-    __ AssertFastElements(elements);
-  }
-
-  // The elements_map register is only used for the not_fast_array path, which
-  // was handled above. From this point onward it is a scratch register.
-  Register scratch1 = elements_map;
+  __ AssertFastElements(elements);
 
   // Check that the key (index) is within bounds.
   __ Ldr(scratch1, FieldMemOperand(elements, FixedArray::kLengthOffset));
   __ Cmp(key, scratch1);
-  __ B(hs, slow);
-
+  __ B(lo, &in_bounds);
+
+  // Out of bounds. Check the prototype chain to see if we can just return
+  // 'undefined'.
+  __ Cmp(key, Operand(Smi::FromInt(0)));
+  __ B(lt, slow);  // Negative keys can't take the fast OOB path.
+  __ Bind(&check_prototypes);
+  __ Ldr(scratch2, FieldMemOperand(receiver, HeapObject::kMapOffset));
+  __ Bind(&check_next_prototype);
+  __ Ldr(scratch2, FieldMemOperand(scratch2, Map::kPrototypeOffset));
+  // scratch2: current prototype
+  __ JumpIfRoot(scratch2, Heap::kNullValueRootIndex, &return_undefined);
+  __ Ldr(elements, FieldMemOperand(scratch2, JSObject::kElementsOffset));
+  __ Ldr(scratch2, FieldMemOperand(scratch2, HeapObject::kMapOffset));
+  // elements: elements of current prototype
+  // scratch2: map of current prototype
+  __ CompareInstanceType(scratch2, scratch1, JS_OBJECT_TYPE);
+  __ B(lo, slow);
+  __ Ldrb(scratch1, FieldMemOperand(scratch2, Map::kBitFieldOffset));
+  __ Tbnz(scratch1, Map::kIsAccessCheckNeeded, slow);
+  __ Tbnz(scratch1, Map::kHasIndexedInterceptor, slow);
+  __ JumpIfNotRoot(elements, Heap::kEmptyFixedArrayRootIndex, slow);
+  __ B(&check_next_prototype);
+
+  __ Bind(&return_undefined);
+  __ LoadRoot(result, Heap::kUndefinedValueRootIndex);
+  __ B(&done);
+
+  __ Bind(&in_bounds);
   // Fast case: Do the load.
   __ Add(scratch1, elements, FixedArray::kHeaderSize - kHeapObjectTag);
   __ SmiUntag(scratch2, key);
   __ Ldr(scratch2, MemOperand(scratch1, scratch2, LSL, kPointerSizeLog2));
 
-  // In case the loaded value is the_hole we have to consult GetProperty
-  // to ensure the prototype chain is searched.
-  __ JumpIfRoot(scratch2, Heap::kTheHoleValueRootIndex, slow);
+  // In case the loaded value is the_hole we have to check the prototype chain.
+  __ JumpIfRoot(scratch2, Heap::kTheHoleValueRootIndex, &check_prototypes);
 
   // Move the value to the result register.
   // 'result' can alias with 'receiver' or 'key' but these two must be
   // preserved if we jump to 'slow'.
   __ Mov(result, scratch2);
+  __ Bind(&done);
 }
 
 
@@ -480,7 +494,7 @@ static void GenerateKeyedLoadWithSmiKey(MacroAssembler* masm, Register key,
   __ CheckFastElements(scratch1, scratch2, &check_number_dictionary);
 
   GenerateFastArrayLoad(masm, receiver, key, scratch3, scratch2, scratch1,
-                        result, NULL, slow);
+                        result, slow);
   __ IncrementCounter(isolate->counters()->keyed_load_generic_smi(), 1,
                       scratch1, scratch2);
   __ Ret();

src/ic/ia32/ic-ia32.cc

@@ -169,40 +169,65 @@ static void GenerateKeyedLoadReceiverCheck(MacroAssembler* masm,
 
 
 // Loads an indexed element from a fast case array.
-// If not_fast_array is NULL, doesn't perform the elements map check.
 static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
                                   Register key, Register scratch,
-                                  Register result, Label* not_fast_array,
-                                  Label* out_of_range) {
+                                  Register scratch2, Register result,
+                                  Label* slow) {
   // Register use:
   //   receiver - holds the receiver and is unchanged.
   //   key - holds the key and is unchanged (must be a smi).
   // Scratch registers:
   //   scratch - used to hold elements of the receiver and the loaded value.
+  //   scratch2 - holds maps and prototypes during prototype chain check.
   //   result - holds the result on exit if the load succeeds and
   //            we fall through.
+  Label check_prototypes, check_next_prototype;
+  Label done, in_bounds, return_undefined;
 
   __ mov(scratch, FieldOperand(receiver, JSObject::kElementsOffset));
-  if (not_fast_array != NULL) {
-    // Check that the object is in fast mode and writable.
-    __ CheckMap(scratch, masm->isolate()->factory()->fixed_array_map(),
-                not_fast_array, DONT_DO_SMI_CHECK);
-  } else {
-    __ AssertFastElements(scratch);
-  }
+  __ AssertFastElements(scratch);
+
   // Check that the key (index) is within bounds.
   __ cmp(key, FieldOperand(scratch, FixedArray::kLengthOffset));
-  __ j(above_equal, out_of_range);
+  __ j(below, &in_bounds);
+  // Out-of-bounds. Check the prototype chain to see if we can just return
+  // 'undefined'.
+  __ cmp(key, 0);
+  __ j(less, slow);  // Negative keys can't take the fast OOB path.
+  __ bind(&check_prototypes);
+  __ mov(scratch2, FieldOperand(receiver, HeapObject::kMapOffset));
+  __ bind(&check_next_prototype);
+  __ mov(scratch2, FieldOperand(scratch2, Map::kPrototypeOffset));
+  // scratch2: current prototype
+  __ cmp(scratch2, masm->isolate()->factory()->null_value());
+  __ j(equal, &return_undefined);
+  __ mov(scratch, FieldOperand(scratch2, JSObject::kElementsOffset));
+  __ mov(scratch2, FieldOperand(scratch2, HeapObject::kMapOffset));
+  // scratch: elements of current prototype
+  // scratch2: map of current prototype
+  __ CmpInstanceType(scratch2, JS_OBJECT_TYPE);
+  __ j(below, slow);
+  __ test_b(
+      FieldOperand(scratch2, Map::kBitFieldOffset),
+      (1 << Map::kIsAccessCheckNeeded) | (1 << Map::kHasIndexedInterceptor));
+  __ j(not_zero, slow);
+  __ cmp(scratch, masm->isolate()->factory()->empty_fixed_array());
+  __ j(not_equal, slow);
+  __ jmp(&check_next_prototype);
+
+  __ bind(&return_undefined);
+  __ mov(result, masm->isolate()->factory()->undefined_value());
+  __ jmp(&done);
+
+  __ bind(&in_bounds);
   // Fast case: Do the load.
   STATIC_ASSERT((kPointerSize == 4) && (kSmiTagSize == 1) && (kSmiTag == 0));
   __ mov(scratch, FieldOperand(scratch, key, times_2, FixedArray::kHeaderSize));
   __ cmp(scratch, Immediate(masm->isolate()->factory()->the_hole_value()));
-  // In case the loaded value is the_hole we have to consult GetProperty
-  // to ensure the prototype chain is searched.
-  __ j(equal, out_of_range);
-  if (!result.is(scratch)) {
-    __ mov(result, scratch);
-  }
+  // In case the loaded value is the_hole we have to check the prototype chain.
+  __ j(equal, &check_prototypes);
+  __ Move(result, scratch);
+  __ bind(&done);
 }
 
 
@@ -327,7 +352,7 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   // Check the receiver's map to see if it has fast elements.
   __ CheckFastElements(eax, &check_number_dictionary);
 
-  GenerateFastArrayLoad(masm, receiver, key, eax, eax, NULL, &slow);
+  GenerateFastArrayLoad(masm, receiver, key, eax, ebx, eax, &slow);
   Isolate* isolate = masm->isolate();
   Counters* counters = isolate->counters();
   __ IncrementCounter(counters->keyed_load_generic_smi(), 1);

src/ic/x64/ic-x64.cc

@@ -168,11 +168,10 @@ static void GenerateKeyedLoadReceiverCheck(MacroAssembler* masm,
 
 
 // Loads an indexed element from a fast case array.
-// If not_fast_array is NULL, doesn't perform the elements map check.
 static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
                                   Register key, Register elements,
                                   Register scratch, Register result,
-                                  Label* not_fast_array, Label* out_of_range) {
+                                  Label* slow) {
   // Register use:
   //
   // receiver - holds the receiver on entry.
@@ -181,8 +180,6 @@ static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
   // key      - holds the smi key on entry.
   //            Unchanged unless 'result' is the same register.
   //
-  // elements - holds the elements of the receiver on exit.
-  //
   // result   - holds the result on exit if the load succeeded.
   //            Allowed to be the the same as 'receiver' or 'key'.
   //            Unchanged on bailout so 'receiver' and 'key' can be safely
@@ -190,32 +187,58 @@ static void GenerateFastArrayLoad(MacroAssembler* masm, Register receiver,
   //
   // Scratch registers:
   //
-  //   scratch - used to hold elements of the receiver and the loaded value.
+  // elements - holds the elements of the receiver and its prototypes.
+  //
+  // scratch  - used to hold maps, prototypes, and the loaded value.
+  Label check_prototypes, check_next_prototype;
+  Label done, in_bounds, return_undefined;
 
   __ movp(elements, FieldOperand(receiver, JSObject::kElementsOffset));
-  if (not_fast_array != NULL) {
-    // Check that the object is in fast mode and writable.
-    __ CompareRoot(FieldOperand(elements, HeapObject::kMapOffset),
-                   Heap::kFixedArrayMapRootIndex);
-    __ j(not_equal, not_fast_array);
-  } else {
-    __ AssertFastElements(elements);
-  }
+  __ AssertFastElements(elements);
   // Check that the key (index) is within bounds.
   __ SmiCompare(key, FieldOperand(elements, FixedArray::kLengthOffset));
   // Unsigned comparison rejects negative indices.
-  __ j(above_equal, out_of_range);
+  __ j(below, &in_bounds);
+
+  // Out-of-bounds. Check the prototype chain to see if we can just return
+  // 'undefined'.
+  __ SmiCompare(key, Smi::FromInt(0));
+  __ j(less, slow);  // Negative keys can't take the fast OOB path.
+  __ bind(&check_prototypes);
+  __ movp(scratch, FieldOperand(receiver, HeapObject::kMapOffset));
+  __ bind(&check_next_prototype);
+  __ movp(scratch, FieldOperand(scratch, Map::kPrototypeOffset));
+  // scratch: current prototype
+  __ CompareRoot(scratch, Heap::kNullValueRootIndex);
+  __ j(equal, &return_undefined);
+  __ movp(elements, FieldOperand(scratch, JSObject::kElementsOffset));
+  __ movp(scratch, FieldOperand(scratch, HeapObject::kMapOffset));
+  // elements: elements of current prototype
+  // scratch: map of current prototype
+  __ CmpInstanceType(scratch, JS_OBJECT_TYPE);
+  __ j(below, slow);
+  __ testb(FieldOperand(scratch, Map::kBitFieldOffset),
+           Immediate((1 << Map::kIsAccessCheckNeeded) |
+                     (1 << Map::kHasIndexedInterceptor)));
+  __ j(not_zero, slow);
+  __ CompareRoot(elements, Heap::kEmptyFixedArrayRootIndex);
+  __ j(not_equal, slow);
+  __ jmp(&check_next_prototype);
+
+  __ bind(&return_undefined);
+  __ LoadRoot(result, Heap::kUndefinedValueRootIndex);
+  __ jmp(&done);
+
+  __ bind(&in_bounds);
   // Fast case: Do the load.
   SmiIndex index = masm->SmiToIndex(scratch, key, kPointerSizeLog2);
   __ movp(scratch, FieldOperand(elements, index.reg, index.scale,
                                 FixedArray::kHeaderSize));
   __ CompareRoot(scratch, Heap::kTheHoleValueRootIndex);
-  // In case the loaded value is the_hole we have to consult GetProperty
-  // to ensure the prototype chain is searched.
-  __ j(equal, out_of_range);
-  if (!result.is(scratch)) {
-    __ movp(result, scratch);
-  }
+  // In case the loaded value is the_hole we have to check the prototype chain.
+  __ j(equal, &check_prototypes);
+  __ Move(result, scratch);
+  __ bind(&done);
 }
 
 
@@ -273,7 +296,7 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   // Check the receiver's map to see if it has fast elements.
   __ CheckFastElements(rax, &check_number_dictionary);
 
-  GenerateFastArrayLoad(masm, receiver, key, rax, rbx, rax, NULL, &slow);
+  GenerateFastArrayLoad(masm, receiver, key, rax, rbx, rax, &slow);
   Counters* counters = masm->isolate()->counters();
   __ IncrementCounter(counters->keyed_load_generic_smi(), 1);
   __ ret(0);