Cleanup in IC patching.

src/ic.cc

Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index b8c45d1ec683906e653764e439c25b8204f6ead0..3216dfe4f4e849583fb1092cd58397bae6088b96 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -182,9 +182,7 @@ Address IC::OriginalCodeAddress() const {
 static bool TryRemoveInvalidPrototypeDependentStub(Code* target,
                                                    Object* receiver,
                                                    Object* name) {
-  if (target->is_keyed_load_stub() ||
-      target->is_keyed_call_stub() ||
-      target->is_keyed_store_stub()) {
+  if (target->is_keyed_stub()) {
     // Determine whether the failure is due to a name failure.
     if (!name->IsName()) return false;
     Name* stub_name = target->FindFirstName();
@@ -1102,46 +1100,44 @@ void IC::PatchCache(State state,
       UpdateMonomorphicIC(receiver, code, name);
       break;
     case MONOMORPHIC:
-      // Only move to megamorphic if the target changes.
-      if (target() != *code) {
-        if (target()->is_load_stub() || target()->is_store_stub()) {
-          bool is_same_handler = false;
-          {
-            DisallowHeapAllocation no_allocation;
-            Code* old_handler = target()->FindFirstHandler();
-            is_same_handler = old_handler == *code;
-          }
-          if (is_same_handler
-              && IsTransitionedMapOfMonomorphicTarget(receiver->map())) {
-            UpdateMonomorphicIC(receiver, code, name);
-            break;
-          }
-          if (UpdatePolymorphicIC(state, receiver, name, code)) {
-            break;
-          }
-
-          CopyICToMegamorphicCache(name);
+      ASSERT(target() != *code);
+      if (!target()->is_keyed_stub()) {

[ulan, 2013/09/30 07:57:46]

I don't see why (!target()->is_keyed_stub()) equivalent to
(target()->is_load_stub() || target()->is_store_stub()) at this place.

What if we have is_call_stub() for example?

[Toon Verwaest, 2013/09/30 08:21:08]

That's exactly the point. Currently this code is only used for loads and stores,
but in the future it'll be used for calls as well. And we just want to
distinguish between keyed vs non-keyed semantics. We just don't want to execute
this code if it's keyed; but rather immediately bail out to the generic case
(megamorphic is generic in the case of keyed).

On 2013/09/30 07:57:46, ulan wrote:

+        bool is_same_handler = false;
+        {
+          DisallowHeapAllocation no_allocation;
+          Code* old_handler = target()->FindFirstHandler();
+          is_same_handler = old_handler == *code;
+        }
+        if (is_same_handler
+            && IsTransitionedMapOfMonomorphicTarget(receiver->map())) {
+          UpdateMonomorphicIC(receiver, code, name);
+          break;
+        }
+        if (UpdatePolymorphicIC(state, receiver, name, code)) {
+          break;
         }
 
-        UpdateMegamorphicCache(receiver->map(), *name, *code);
-        set_target(*megamorphic_stub());
+        CopyICToMegamorphicCache(name);
       }
+
+      UpdateMegamorphicCache(receiver->map(), *name, *code);
+      set_target(*megamorphic_stub());
       break;
     case MEGAMORPHIC:
       UpdateMegamorphicCache(receiver->map(), *name, *code);
       break;
     case POLYMORPHIC:
-      if (target()->is_load_stub() || target()->is_store_stub()) {
+      if (target()->is_keyed_stub()) {
+        // When trying to patch a polymorphic keyed stub with anything other
+        // than another polymorphic stub, go generic.
+        set_target(*generic_stub());
+      } else {
         if (UpdatePolymorphicIC(state, receiver, name, code)) {
           break;
         }
         CopyICToMegamorphicCache(name);
         UpdateMegamorphicCache(receiver->map(), *name, *code);
         set_target(*megamorphic_stub());
-      } else {
-        // When trying to patch a polymorphic keyed load/store element stub
-        // with anything other than another polymorphic stub, go generic.
-        set_target(*generic_stub());
       }
       break;
     case DEBUG_STUB: