Cleanup IC heuristics.

src/ic.cc

Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index 080c7bf1b723f87b1449d4d9da58567ddf6ea9fb..024ce18326c888930810ee9827d6bf92a35341ca 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -182,13 +182,12 @@ Address IC::OriginalCodeAddress() const {
 static bool TryRemoveInvalidPrototypeDependentStub(Code* target,
                                                    Object* receiver,
                                                    Object* name) {
-  // If the code is NORMAL, it handles dictionary mode objects. Such stubs do
-  // not check maps, but do positive/negative lookups.
-  if (target->type() != Code::NORMAL) {
-    Map* map = target->FindFirstMap();
-    if (map != NULL && map->is_deprecated()) {
-      return true;
-    }
+  if (target->is_keyed_load_stub() ||

[danno, 2013/05/02 11:36:06]

// Determine whether the failure is due to a name failure or a prototype chain
failure.

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+      target->is_keyed_call_stub() ||
+      target->is_keyed_store_stub()) {
+    if (!name->IsName()) return false;
+    Name* stub_name = target->FindFirstName();
+    if (Name::cast(name) != stub_name) return false;
   }
 
   InlineCacheHolderFlag cache_holder =
@@ -217,10 +216,22 @@ static bool TryRemoveInvalidPrototypeDependentStub(Code* target,
   int index = map->IndexInCodeCache(name, target);
   if (index >= 0) {
     map->RemoveFromCodeCache(String::cast(name), target, index);
+    if (target->is_load_stub() || target->is_keyed_load_stub()) {

[danno, 2013/05/02 11:36:06]

// For loads, handlers are stored in addition to the ICs on the map. Remove
those, too.

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+      Code* handler = target->FindFirstCode();
+      index = map->IndexInCodeCache(name, handler);
+      if (index >= 0) {
+        map->RemoveFromCodeCache(String::cast(name), handler, index);
+      }
+    }
     return true;
   }
 
-  return false;
+  if (cache_holder != OWN_MAP) return false;

[danno, 2013/05/02 11:36:06]

// If the IC is shared between multiple receivers (slow dictionary mode), then
the map cannot be deprecated and the stub invalidated.

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+
+  // Also flag monomorphic prototype failure if the old map was deprecated.
+  Map* old_map = target->FindFirstMap();
+  if (old_map == NULL) return false;
+  return old_map == map || old_map->is_deprecated();

[danno, 2013/05/02 11:36:06]

// The stub is not in the cache. We've ruled out all other kinds of failure
except for proptotype chain changes, a deprecated map, or a map that's different
from the one that the stub expects. If the map hasn't changed assume it's a
prototype failure. Tread deprecated maps in the same way as prototype failures
(stay monomorphic if possible).

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

 }
 
 
@@ -230,22 +241,13 @@ IC::State IC::StateFrom(Code* target, Object* receiver, Object* name) {
   if (state != MONOMORPHIC || !name->IsString()) return state;
   if (receiver->IsUndefined() || receiver->IsNull()) return state;
 
-  // For keyed load/store/call, the most likely cause of cache failure is
-  // that the key has changed.  We do not distinguish between
-  // prototype and non-prototype failures for keyed access.
   Code::Kind kind = target->kind();
-  if (kind == Code::KEYED_LOAD_IC ||
-      kind == Code::KEYED_STORE_IC ||
-      kind == Code::KEYED_CALL_IC) {
-    return MONOMORPHIC;
-  }
-
   // Remove the target from the code cache if it became invalid
   // because of changes in the prototype chain to avoid hitting it
   // again.
   // Call stubs handle this later to allow extra IC state
   // transitions.
-  if (kind != Code::CALL_IC &&
+  if (kind != Code::CALL_IC && kind != Code::KEYED_CALL_IC &&
       TryRemoveInvalidPrototypeDependentStub(target, receiver, name)) {
     return MONOMORPHIC_PROTOTYPE_FAILURE;
   }
@@ -724,8 +726,7 @@ void CallICBase::UpdateCaches(LookupResult* lookup,
         TryUpdateExtraICState(lookup, object, &extra_ic_state)) {
       code = ComputeMonomorphicStub(lookup, state, extra_ic_state,
                                     object, name);
-    } else if (kind_ == Code::CALL_IC &&
-               TryRemoveInvalidPrototypeDependentStub(target(),
+    } else if (TryRemoveInvalidPrototypeDependentStub(target(),
                                                       *object,
                                                       *name)) {
       state = MONOMORPHIC_PROTOTYPE_FAILURE;
@@ -748,15 +749,7 @@ void CallICBase::UpdateCaches(LookupResult* lookup,
     case UNINITIALIZED:
     case MONOMORPHIC_PROTOTYPE_FAILURE:
     case PREMONOMORPHIC:
-      set_target(*code);
-      break;
     case MONOMORPHIC:
-      if (code->ic_state() != MONOMORPHIC) {
-        Map* map = target()->FindFirstMap();
-        if (map != NULL) {
-          UpdateMegamorphicCache(map, *name, target());
-        }
-      }
       set_target(*code);
       break;
     case MEGAMORPHIC: {
@@ -1007,14 +1000,25 @@ bool IC::UpdatePolymorphicIC(State state,
     target()->FindAllCode(&handlers, receiver_maps.length());
   }
 
-  if (!AddOneReceiverMapIfMissing(&receiver_maps,
-                                  Handle<Map>(receiver->map()))) {
-    return false;
+  bool overwrote = false;

[danno, 2013/05/02 11:36:06]

// If the name doesn't match, go generic. Otherwise, if one of the maps already
in the polymorphic ic is the receiver map, just overwrite the handler. This
handles both the prototype chain changes and deprecated maps without prematurely
going generic. 

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+  Handle<Map> new_receiver_map(receiver->map());
+  for (int current = 0; current < receiver_maps.length(); ++current) {
+    if (!receiver_maps.at(current).is_null() &&
+        receiver_maps.at(current).is_identical_to(new_receiver_map)) {
+      overwrote = true;
+      handlers.InsertAt(current, code);
+      break;
+    }
+  }

[danno, 2013/05/02 11:36:06]

Move this loop about the code above to get the map count right. Better yet, fold
the two loops together.

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+
+  if (!overwrote) {
+    receiver_maps.Add(new_receiver_map);
+    handlers.Add(code);
+    number_of_valid_maps++;
   }
 
-  handlers.Add(code);
   Handle<Code> ic = isolate()->stub_cache()->ComputePolymorphicIC(
-      &receiver_maps, &handlers, number_of_valid_maps + 1, name);
+      &receiver_maps, &handlers, number_of_valid_maps, name);
   set_target(*ic);
   return true;
 }
@@ -1049,6 +1053,7 @@ void IC::CopyICToMegamorphicCache(Handle<String> name) {
     target()->FindAllCode(&handlers, receiver_maps.length());
   }
   for (int i = 0; i < receiver_maps.length(); i++) {
+    if (receiver_maps.at(i)->is_deprecated()) continue;

[danno, 2013/05/02 11:36:06]

Maybe defer this to Crankshaft? Don't throw away the deprecated maps, just
replace them with their promoted version. Whatever you decide, please comment
about the reasoning.

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

     UpdateMegamorphicCache(*receiver_maps.at(i), *name, *handlers.at(i));
   }
 }
@@ -1101,38 +1106,9 @@ void IC::PatchCache(State state,
           if (UpdatePolymorphicIC(state, strict_mode, receiver, name, code)) {
             break;
           }
-        }
-        if (target()->type() != Code::NORMAL) {
-          if (target()->is_load_stub()) {
+
+          if (target()->type() != Code::NORMAL) {
             CopyICToMegamorphicCache(name);
-          } else if (target()->is_store_stub()) {
-            // Ensure that the IC stays monomorphic when replacing a monomorphic
-            // IC for a deprecated map.
-            // TODO(verwaest): Remove this code once polymorphic store ICs are
-            // implemented. Updating the polymorphic IC will keep it monomorphic
-            // by filtering deprecated maps.
-            MapHandleList maps;
-            Code* handler = target();
-            handler->FindAllMaps(&maps);
-            for (int i = 0; i < Min(1, maps.length()); i++) {
-              if (maps.at(i)->is_deprecated()) {
-                UpdateMonomorphicIC(receiver, code, name);
-                return;
-              }
-            }
-            if (maps.length() > 0) {
-              if (receiver->map() == *maps.at(0)) {
-                UpdateMonomorphicIC(receiver, code, name);
-                return;
-              }
-              UpdateMegamorphicCache(*maps.at(0), *name, handler);
-            }
-          } else {
-            Code* handler = target();
-            Map* map = handler->FindFirstMap();
-            if (map != NULL) {
-              UpdateMegamorphicCache(map, *name, handler);
-            }
           }
         }
 
@@ -1235,7 +1211,7 @@ void LoadIC::UpdateCaches(LookupResult* lookup,
 }
 
 
-void IC::UpdateMegamorphicCache(Map* map, String* name, Code* code) {
+void IC::UpdateMegamorphicCache(Map* map, Name* name, Code* code) {
   // Cache code holding map should be consistent with
   // GenerateMonomorphicCacheProbe.
   isolate()->stub_cache()->Set(name, map, code);
@@ -1497,7 +1473,8 @@ Handle<Code> KeyedLoadIC::ComputeLoadHandler(LookupResult* lookup,
 static bool LookupForWrite(Handle<JSObject> receiver,
                            Handle<String> name,
                            Handle<Object> value,
-                           LookupResult* lookup) {
+                           LookupResult* lookup,
+                           IC::State* state) {
   Handle<JSObject> holder = receiver;
   receiver->Lookup(*name, lookup);
   if (lookup->IsFound()) {
@@ -1534,7 +1511,13 @@ static bool LookupForWrite(Handle<JSObject> receiver,
   PropertyDetails target_details =
       lookup->GetTransitionDetails(receiver->map());
   if (target_details.IsReadOnly()) return false;
-  return value->FitsRepresentation(target_details.representation());
+  if (!value->FitsRepresentation(target_details.representation())) {

[danno, 2013/05/02 11:36:06]

Comment this.

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+    Handle<Map> target(lookup->GetTransitionMapFromMap(receiver->map()));
+    Map::GeneralizeRepresentation(
+        target, target->LastAdded(), value->OptimalRepresentation());
+    *state = MONOMORPHIC_PROTOTYPE_FAILURE;

[danno, 2013/05/02 11:36:06]

Check for already deprecated maps? Deprecated maps shouldn't change the target
at all?

[Toon Verwaest, 2013/05/02 12:27:20]

Done.

+  }
+  return true;
 }
 
 
@@ -1618,7 +1601,7 @@ MaybeObject* StoreIC::Store(State state,
   }
 
   LookupResult lookup(isolate());
-  if (LookupForWrite(receiver, name, value, &lookup)) {
+  if (LookupForWrite(receiver, name, value, &lookup, &state)) {
     if (FLAG_use_ic) {
       UpdateCaches(&lookup, state, strict_mode, receiver, name, value);
     }