Add MEGAMORPHIC state support for KeyedStoreIC

R=mvstanton@chromium.org

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

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

src/builtins.cc

@@ -1314,13 +1314,23 @@ static void Generate_StoreIC_Setter_ForDeopt(MacroAssembler* masm) {
 }
 
 
+static void Generate_KeyedStoreIC_Megamorphic(MacroAssembler* masm) {
+  KeyedStoreIC::GenerateGeneric(masm, SLOPPY, kMissOnMissingHandler);
+}
+
+
+static void Generate_KeyedStoreIC_Megamorphic_Strict(MacroAssembler* masm) {
+  KeyedStoreIC::GenerateGeneric(masm, STRICT, kMissOnMissingHandler);
+}
+
+
 static void Generate_KeyedStoreIC_Generic(MacroAssembler* masm) {
-  KeyedStoreIC::GenerateGeneric(masm, SLOPPY);
+  KeyedStoreIC::GenerateGeneric(masm, SLOPPY, kCallRuntimeOnMissingHandler);
 }
 
 
 static void Generate_KeyedStoreIC_Generic_Strict(MacroAssembler* masm) {
-  KeyedStoreIC::GenerateGeneric(masm, STRICT);
+  KeyedStoreIC::GenerateGeneric(masm, STRICT, kCallRuntimeOnMissingHandler);
 }
 
 

src/builtins.h

@@ -95,12 +95,15 @@ enum BuiltinExtraArguments {
   V(KeyedStoreIC_Initialize, KEYED_STORE_IC, UNINITIALIZED, kNoExtraICState)   \
   V(KeyedStoreIC_PreMonomorphic, KEYED_STORE_IC, PREMONOMORPHIC,               \
     kNoExtraICState)                                                           \
+  V(KeyedStoreIC_Megamorphic, KEYED_STORE_IC, MEGAMORPHIC, kNoExtraICState)    \
   V(KeyedStoreIC_Generic, KEYED_STORE_IC, GENERIC, kNoExtraICState)            \
                                                                                \
   V(KeyedStoreIC_Initialize_Strict, KEYED_STORE_IC, UNINITIALIZED,             \
     StoreIC::kStrictModeState)                                                 \
   V(KeyedStoreIC_PreMonomorphic_Strict, KEYED_STORE_IC, PREMONOMORPHIC,        \
     StoreIC::kStrictModeState)                                                 \
+  V(KeyedStoreIC_Megamorphic_Strict, KEYED_STORE_IC, MEGAMORPHIC,              \
+    StoreIC::kStrictModeState)                                                 \
   V(KeyedStoreIC_Generic_Strict, KEYED_STORE_IC, GENERIC,                      \
     StoreIC::kStrictModeState)                                                 \
   V(KeyedStoreIC_SloppyArguments, KEYED_STORE_IC, MONOMORPHIC,                 \

src/ic/arm/ic-arm.cc

@@ -765,8 +765,9 @@ static void KeyedStoreGenerateGenericHelper(
 }
 
 
-void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
-                                   StrictMode strict_mode) {
+void KeyedStoreIC::GenerateGeneric(
+    MacroAssembler* masm, StrictMode strict_mode,
+    KeyedStoreStubCacheRequirement handler_requirement) {
   // ---------- S t a t e --------------
   //  -- r0     : value
   //  -- r1     : key
@@ -775,7 +776,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   // -----------------------------------
   Label slow, fast_object, fast_object_grow;
   Label fast_double, fast_double_grow;
-  Label array, extra, check_if_double_array;
+  Label array, extra, check_if_double_array, maybe_name_key, miss;
 
   // Register usage.
   Register value = StoreDescriptor::ValueRegister();
@@ -790,7 +791,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   // r4 and r5 are used as general scratch registers.
 
   // Check that the key is a smi.
-  __ JumpIfNotSmi(key, &slow);
+  __ JumpIfNotSmi(key, &maybe_name_key);
   // Check that the object isn't a smi.
   __ JumpIfSmi(receiver, &slow);
   // Get the map of the object.
@@ -822,6 +823,23 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   // r1: key.
   // r2: receiver.
   PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode);
+  // Never returns to here.
+
+  __ bind(&maybe_name_key);
+  __ ldr(r4, FieldMemOperand(key, HeapObject::kMapOffset));
+  __ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset));
+  __ JumpIfNotUniqueNameInstanceType(r4, &slow);
+  Code::Flags flags = Code::RemoveTypeAndHolderFromFlags(
+      Code::ComputeHandlerFlags(Code::STORE_IC));
+  masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver,
+                                               key, r3, r4, r5, r6);
+  // Cache miss.
+  if (handler_requirement == kCallRuntimeOnMissingHandler) {
+    __ b(&slow);
+  } else {
+    DCHECK(handler_requirement == kMissOnMissingHandler);
+    __ b(&miss);
+  }
 
   // Extra capacity case: Check if there is extra capacity to
   // perform the store and update the length. Used for adding one
@@ -863,6 +881,9 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
                                   &slow, kDontCheckMap, kIncrementLength, value,
                                   key, receiver, receiver_map, elements_map,
                                   elements);
+
+  __ bind(&miss);
+  GenerateMiss(masm);
 }
 
 

src/ic/arm64/ic-arm64.cc

@@ -798,8 +798,9 @@ static void KeyedStoreGenerateGenericHelper(
 }
 
 
-void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
-                                   StrictMode strict_mode) {
+void KeyedStoreIC::GenerateGeneric(
+    MacroAssembler* masm, StrictMode strict_mode,
+    KeyedStoreStubCacheRequirement handler_requirement) {
   ASM_LOCATION("KeyedStoreIC::GenerateGeneric");
   Label slow;
   Label array;
@@ -808,6 +809,8 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   Label fast_object_grow;
   Label fast_double_grow;
   Label fast_double;
+  Label maybe_name_key;
+  Label miss;
 
   Register value = StoreDescriptor::ValueRegister();
   Register key = StoreDescriptor::NameRegister();
@@ -820,7 +823,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   Register elements = x4;
   Register elements_map = x5;
 
-  __ JumpIfNotSmi(key, &slow);
+  __ JumpIfNotSmi(key, &maybe_name_key);
   __ JumpIfSmi(receiver, &slow);
   __ Ldr(receiver_map, FieldMemOperand(receiver, HeapObject::kMapOffset));
 
@@ -853,7 +856,23 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   //  x1: key
   //  x2: receiver
   PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode);
+  // Never returns to here.
 
+  __ bind(&maybe_name_key);
+  __ Ldr(x10, FieldMemOperand(key, HeapObject::kMapOffset));
+  __ Ldrb(x10, FieldMemOperand(x10, Map::kInstanceTypeOffset));
+  __ JumpIfNotUniqueNameInstanceType(x10, &slow);
+  Code::Flags flags = Code::RemoveTypeAndHolderFromFlags(
+      Code::ComputeHandlerFlags(Code::STORE_IC));
+  masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver,
+                                               key, x3, x4, x5, x6);
+  // Cache miss.
+  if (handler_requirement == kCallRuntimeOnMissingHandler) {
+    __ B(&slow);
+  } else {
+    DCHECK(handler_requirement == kMissOnMissingHandler);
+    __ B(&miss);
+  }
 
   __ Bind(&extra);
   // Extra capacity case: Check if there is extra capacity to
@@ -895,6 +914,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
                                   &slow, kDontCheckMap, kIncrementLength, value,
                                   key, receiver, receiver_map, elements_map,
                                   elements);
+
+  if (handler_requirement == kMissOnMissingHandler) {
+    __ bind(&miss);
+    GenerateMiss(masm);
+  }
 }
 
 

src/ic/ia32/ic-ia32.cc

@@ -672,12 +672,13 @@ static void KeyedStoreGenerateGenericHelper(
 }
 
 
-void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
-                                   StrictMode strict_mode) {
+void KeyedStoreIC::GenerateGeneric(
+    MacroAssembler* masm, StrictMode strict_mode,
+    KeyedStoreStubCacheRequirement handler_requirement) {
   // Return address is on the stack.
   Label slow, fast_object, fast_object_grow;
   Label fast_double, fast_double_grow;
-  Label array, extra, check_if_double_array;
+  Label array, extra, check_if_double_array, maybe_name_key, miss;
   Register receiver = StoreDescriptor::ReceiverRegister();
   Register key = StoreDescriptor::NameRegister();
   DCHECK(receiver.is(edx));
@@ -693,7 +694,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
             1 << Map::kIsAccessCheckNeeded | 1 << Map::kIsObserved);
   __ j(not_zero, &slow);
   // Check that the key is a smi.
-  __ JumpIfNotSmi(key, &slow);
+  __ JumpIfNotSmi(key, &maybe_name_key);
   __ CmpInstanceType(edi, JS_ARRAY_TYPE);
   __ j(equal, &array);
   // Check that the object is some kind of JSObject.
@@ -711,6 +712,23 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   // Slow case: call runtime.
   __ bind(&slow);
   PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode);
+  // Never returns to here.
+
+  __ bind(&maybe_name_key);
+  __ mov(ebx, FieldOperand(key, HeapObject::kMapOffset));
+  __ movzx_b(ebx, FieldOperand(ebx, Map::kInstanceTypeOffset));
+  __ JumpIfNotUniqueNameInstanceType(ebx, &slow);
+  Code::Flags flags = Code::RemoveTypeAndHolderFromFlags(
+      Code::ComputeHandlerFlags(Code::STORE_IC));
+  masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver,
+                                               key, ebx, no_reg);
+  // Cache miss.
+  if (handler_requirement == kCallRuntimeOnMissingHandler) {
+    __ jmp(&slow);
+  } else {
+    DCHECK(handler_requirement == kMissOnMissingHandler);
+    __ jmp(&miss);
+  }
 
   // Extra capacity case: Check if there is extra capacity to
   // perform the store and update the length. Used for adding one
@@ -753,6 +771,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
                                   kCheckMap, kDontIncrementLength);
   KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow,
                                   &slow, kDontCheckMap, kIncrementLength);
+
+  if (handler_requirement == kMissOnMissingHandler) {
+    __ bind(&miss);
+    GenerateMiss(masm);
+  }
 }
 
 

src/ic/ic.cc

@@ -801,19 +801,27 @@ void IC::PatchCache(Handle<Name> name, Handle<Code> code) {
     case POLYMORPHIC:
       if (!target()->is_keyed_stub() || state() == PROTOTYPE_FAILURE) {
         if (UpdatePolymorphicIC(name, code)) break;
+        // For keyed stubs, we can't know whether old handlers were for the
+        // same key.
         CopyICToMegamorphicCache(name);
       }
       set_target(*megamorphic_stub());
     // Fall through.
     case MEGAMORPHIC:
       UpdateMegamorphicCache(*receiver_type(), *name, *code);
+      // Indicate that we've handled this case.
+      target_set_ = true;
       break;
     case DEBUG_STUB:
       break;
     case DEFAULT:
-    case GENERIC:
       UNREACHABLE();
       break;
+    case GENERIC:
+      // The generic keyed store stub re-uses store handlers, which can miss.
+      // That's ok, no reason to do anything.
+      DCHECK(target()->kind() == Code::KEYED_STORE_IC);
+      break;
   }
 }
 
@@ -894,7 +902,8 @@ void LoadIC::UpdateCaches(LookupIterator* lookup) {
 
 
 void IC::UpdateMegamorphicCache(HeapType* type, Name* name, Code* code) {
-  if (kind() == Code::KEYED_LOAD_IC || kind() == Code::KEYED_STORE_IC) return;
+  // Megamorphic state isn't implemented for keyed loads currently.
+  if (kind() == Code::KEYED_LOAD_IC) return;
   Map* map = *TypeToMap(type, isolate());
   isolate()->stub_cache()->Set(name, map, code);
 }
@@ -1365,9 +1374,9 @@ Handle<Code> StoreIC::megamorphic_stub() {
   } else {
     DCHECK(kind() == Code::KEYED_STORE_IC);
     if (strict_mode() == STRICT) {
-      return isolate()->builtins()->KeyedStoreIC_Generic_Strict();
+      return isolate()->builtins()->KeyedStoreIC_Megamorphic_Strict();
     } else {
-      return isolate()->builtins()->KeyedStoreIC_Generic();
+      return isolate()->builtins()->KeyedStoreIC_Megamorphic();
     }
   }
 }
@@ -1813,12 +1822,12 @@ MaybeHandle<Object> KeyedStoreIC::Store(Handle<Object> object,
         StoreIC::Store(object, Handle<String>::cast(key), value,
                        JSReceiver::MAY_BE_STORE_FROM_KEYED),
         Object);
-    // TODO(jkummerow): Ideally we'd wrap this in "if (!is_target_set())",
-    // but doing so causes Hydrogen crashes. Needs investigation.
-    TRACE_GENERIC_IC(isolate(), "KeyedStoreIC",
-                     "unhandled internalized string key");
-    TRACE_IC("StoreIC", key);
-    set_target(*stub);
+    if (!is_target_set()) {
+      TRACE_GENERIC_IC(isolate(), "KeyedStoreIC",
+                       "unhandled internalized string key");
+      TRACE_IC("StoreIC", key);
+      set_target(*stub);
+    }
     return store_handle;
   }
 
@@ -2088,7 +2097,7 @@ RUNTIME_FUNCTION(StoreIC_Miss) {
   DCHECK(args.length() == 3);
   StoreIC ic(IC::NO_EXTRA_FRAME, isolate);
   Handle<Object> receiver = args.at<Object>(0);
-  Handle<String> key = args.at<String>(1);
+  Handle<Name> key = args.at<Name>(1);
   ic.UpdateState(receiver, key);
   Handle<Object> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
@@ -2103,7 +2112,7 @@ RUNTIME_FUNCTION(StoreIC_MissFromStubFailure) {
   DCHECK(args.length() == 3 || args.length() == 4);
   StoreIC ic(IC::EXTRA_CALL_FRAME, isolate);
   Handle<Object> receiver = args.at<Object>(0);
-  Handle<String> key = args.at<String>(1);
+  Handle<Name> key = args.at<Name>(1);
   ic.UpdateState(receiver, key);
   Handle<Object> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(

src/ic/ic.h

@@ -223,7 +223,6 @@ class IC {
     return target_maps_.length() > 0 ? *target_maps_.at(0) : NULL;
   }
 
- protected:
   inline void UpdateTarget();
 
  private:
@@ -526,6 +525,12 @@ enum KeyedStoreCheckMap { kDontCheckMap, kCheckMap };
 enum KeyedStoreIncrementLength { kDontIncrementLength, kIncrementLength };
 
 
+enum KeyedStoreStubCacheRequirement {
+  kCallRuntimeOnMissingHandler,
+  kMissOnMissingHandler
+};
+
+
 class KeyedStoreIC : public StoreIC {
  public:
   // ExtraICState bits (building on IC)
@@ -559,7 +564,9 @@ class KeyedStoreIC : public StoreIC {
   }
   static void GenerateMiss(MacroAssembler* masm);
   static void GenerateSlow(MacroAssembler* masm);
-  static void GenerateGeneric(MacroAssembler* masm, StrictMode strict_mode);
+  static void GenerateGeneric(
+      MacroAssembler* masm, StrictMode strict_mode,
+      KeyedStoreStubCacheRequirement handler_requirement);
   static void GenerateSloppyArguments(MacroAssembler* masm);
 
  protected:

src/ic/x64/ic-x64.cc

@@ -566,12 +566,13 @@ static void KeyedStoreGenerateGenericHelper(
 }
 
 
-void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
-                                   StrictMode strict_mode) {
+void KeyedStoreIC::GenerateGeneric(
+    MacroAssembler* masm, StrictMode strict_mode,
+    KeyedStoreStubCacheRequirement handler_requirement) {
   // Return address is on the stack.
   Label slow, slow_with_tagged_index, fast_object, fast_object_grow;
   Label fast_double, fast_double_grow;
-  Label array, extra, check_if_double_array;
+  Label array, extra, check_if_double_array, maybe_name_key, miss;
   Register receiver = StoreDescriptor::ReceiverRegister();
   Register key = StoreDescriptor::NameRegister();
   DCHECK(receiver.is(rdx));
@@ -587,7 +588,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
            Immediate(1 << Map::kIsAccessCheckNeeded | 1 << Map::kIsObserved));
   __ j(not_zero, &slow_with_tagged_index);
   // Check that the key is a smi.
-  __ JumpIfNotSmi(key, &slow_with_tagged_index);
+  __ JumpIfNotSmi(key, &maybe_name_key);
   __ SmiToInteger32(key, key);
 
   __ CmpInstanceType(r9, JS_ARRAY_TYPE);
@@ -610,6 +611,22 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
   PropertyICCompiler::GenerateRuntimeSetProperty(masm, strict_mode);
   // Never returns to here.
 
+  __ bind(&maybe_name_key);
+  __ movp(r9, FieldOperand(key, HeapObject::kMapOffset));
+  __ movzxbp(r9, FieldOperand(r9, Map::kInstanceTypeOffset));
+  __ JumpIfNotUniqueNameInstanceType(r9, &slow_with_tagged_index);
+  Code::Flags flags = Code::RemoveTypeAndHolderFromFlags(
+      Code::ComputeHandlerFlags(Code::STORE_IC));
+  masm->isolate()->stub_cache()->GenerateProbe(masm, flags, false, receiver,
+                                               key, rbx, no_reg);
+  // Cache miss.
+  if (handler_requirement == kCallRuntimeOnMissingHandler) {
+    __ jmp(&slow_with_tagged_index);
+  } else {
+    DCHECK(handler_requirement == kMissOnMissingHandler);
+    __ jmp(&miss);
+  }
+
   // Extra capacity case: Check if there is extra capacity to
   // perform the store and update the length. Used for adding one
   // element to the array by writing to array[array.length].
@@ -648,6 +665,11 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm,
                                   kCheckMap, kDontIncrementLength);
   KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow,
                                   &slow, kDontCheckMap, kIncrementLength);
+
+  if (handler_requirement == kMissOnMissingHandler) {
+    __ bind(&miss);
+    GenerateMiss(masm);
+  }
 }
 
 

src/ic/x64/stub-cache-x64.cc

@@ -41,14 +41,14 @@ static void ProbeTable(Isolate* isolate, MacroAssembler* masm,
   __ LoadAddress(kScratchRegister, key_offset);
 
   // Check that the key in the entry matches the name.
-  // Multiply entry offset by 16 to get the entry address. Since the
-  // offset register already holds the entry offset times four, multiply
-  // by a further four.
-  __ cmpl(name, Operand(kScratchRegister, offset, scale_factor, 0));
+  __ cmpp(name, Operand(kScratchRegister, offset, scale_factor, 0));
   __ j(not_equal, &miss);
 
   // Get the map entry from the cache.
   // Use key_offset + kPointerSize * 2, rather than loading map_offset.
+  DCHECK(isolate->stub_cache()->map_reference(table).address() -
+             isolate->stub_cache()->key_reference(table).address() ==
+         kPointerSize * 2);
   __ movp(kScratchRegister,
           Operand(kScratchRegister, offset, scale_factor, kPointerSize * 2));
   __ cmpp(kScratchRegister, FieldOperand(receiver, HeapObject::kMapOffset));