VectorICs: keyed element loads were kicking out non-smi keys unnecessarily

src/arm/code-stubs-arm.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #if V8_TARGET_ARCH_ARM
 
 #include "src/base/bits.h"
 #include "src/bootstrapper.h"
@@ skipping 1490 matching lines @@
   StringCharAtGenerator char_at_generator(receiver, index, scratch, result,
                                           &miss,  // When not a string.
                                           &miss,  // When not a number.
                                           &miss,  // When index out of range.
                                           STRING_INDEX_IS_ARRAY_INDEX,
                                           RECEIVER_IS_STRING);
   char_at_generator.GenerateFast(masm);
   __ Ret();
 
   StubRuntimeCallHelper call_helper;
-  char_at_generator.GenerateSlow(masm, call_helper);
+  char_at_generator.GenerateSlow(masm, PART_OF_IC_HANDLER, call_helper);
 
   __ bind(&miss);
   PropertyAccessCompiler::TailCallBuiltin(
       masm, PropertyAccessCompiler::MissBuiltin(Code::KEYED_LOAD_IC));
 }
 
 
 void ArgumentsAccessStub::GenerateReadElement(MacroAssembler* masm) {
   CHECK(!has_new_target());
   // The displacement is the offset of the last parameter (if any)
@@ skipping 1373 matching lines @@
                                     index_,
                                     result_,
                                     &call_runtime_);
 
   __ SmiTag(result_);
   __ bind(&exit_);
 }
 
 
 void StringCharCodeAtGenerator::GenerateSlow(
-    MacroAssembler* masm,
+    MacroAssembler* masm, EmbedMode embed_mode,
     const RuntimeCallHelper& call_helper) {
   __ Abort(kUnexpectedFallthroughToCharCodeAtSlowCase);
 
   // Index is not a smi.
   __ bind(&index_not_smi_);
   // If index is a heap number, try converting it to an integer.
   __ CheckMap(index_,
               result_,
               Heap::kHeapNumberMapRootIndex,
               index_not_number_,
               DONT_DO_SMI_CHECK);
   call_helper.BeforeCall(masm);
+  if (FLAG_vector_ics && embed_mode == PART_OF_IC_HANDLER) {
+    __ push(VectorLoadICDescriptor::VectorRegister());

[Yang, 2015/03/23 14:23:59]

If you use Push to push both registers at once, you may end up with a shorter
instruction if you use the correct order.

[mvstanton, 2015/03/23 15:54:44]

Good idea, and I went further and do one push instruction for all four registers
(and one pop for the 3 after the call).

+    __ push(VectorLoadICDescriptor::SlotRegister());
+  }
   __ push(object_);
   __ push(index_);  // Consumed by runtime conversion function.
   if (index_flags_ == STRING_INDEX_IS_NUMBER) {
     __ CallRuntime(Runtime::kNumberToIntegerMapMinusZero, 1);
   } else {
     DCHECK(index_flags_ == STRING_INDEX_IS_ARRAY_INDEX);
     // NumberToSmi discards numbers that are not exact integers.
     __ CallRuntime(Runtime::kNumberToSmi, 1);
   }
   // Save the conversion result before the pop instructions below
   // have a chance to overwrite it.
   __ Move(index_, r0);
   __ pop(object_);
+  if (FLAG_vector_ics && embed_mode == PART_OF_IC_HANDLER) {
+    __ pop(VectorLoadICDescriptor::SlotRegister());
+    __ pop(VectorLoadICDescriptor::VectorRegister());
+  }
   // Reload the instance type.
   __ ldr(result_, FieldMemOperand(object_, HeapObject::kMapOffset));
   __ ldrb(result_, FieldMemOperand(result_, Map::kInstanceTypeOffset));
   call_helper.AfterCall(masm);
   // If index is still not a smi, it must be out of range.
   __ JumpIfNotSmi(index_, index_out_of_range_);
   // Otherwise, return to the fast path.
   __ jmp(&got_smi_index_);
 
   // Call runtime. We get here when the receiver is a string and the
@@ skipping 1587 matching lines @@
 
   __ add(feedback, vector, Operand::PointerOffsetFromSmiKey(slot));
   __ ldr(feedback, FieldMemOperand(feedback, FixedArray::kHeaderSize));
 
   // Is it a weak cell?
   Label try_array;
   Label not_array, smi_key, key_okay, miss;
   __ ldr(scratch1, FieldMemOperand(feedback, HeapObject::kMapOffset));
   __ CompareRoot(scratch1, Heap::kWeakCellMapRootIndex);
   __ b(ne, &try_array);
-  __ JumpIfNotSmi(key, &miss);
   HandleMonomorphicCase(masm, receiver, key, vector, slot, feedback, scratch1,
                         &miss);
 
   __ bind(&try_array);
   // Is it a fixed array?
   __ CompareRoot(scratch1, Heap::kFixedArrayMapRootIndex);
   __ b(ne, &not_array);
+
   // We have a polymorphic element handler.
-  __ JumpIfNotSmi(key, &miss);
-
   Label polymorphic, try_poly_name;
   __ bind(&polymorphic);
   HandleArrayCases(masm, receiver, key, vector, slot, feedback, scratch1, r8,
                    r9, true, &miss);
 
   __ bind(&not_array);
   // Is it generic?
   __ CompareRoot(feedback, Heap::kmegamorphic_symbolRootIndex);
   __ b(ne, &try_poly_name);
   Handle<Code> megamorphic_stub =
@@ skipping 718 matching lines @@
                            kStackUnwindSpace, NULL,
                            MemOperand(fp, 6 * kPointerSize), NULL);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM