Cleanup in IC patching.

src/ic.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 164 matching lines @@
   intptr_t delta =
       original_code->instruction_start() - code->instruction_start();
   return addr + delta;
 }
 #endif
 
 
 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();
     if (Name::cast(name) != stub_name) return false;
   }
 
   InlineCacheHolderFlag cache_holder =
       Code::ExtractCacheHolderFromFlags(target->flags());
 
   Isolate* isolate = target->GetIsolate();
@@ skipping 897 matching lines @@
                     Handle<HeapObject> receiver,
                     Handle<String> name,
                     Handle<Code> code) {
   switch (state) {
     case UNINITIALIZED:
     case PREMONOMORPHIC:
     case MONOMORPHIC_PROTOTYPE_FAILURE:
       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:
       break;
     case GENERIC:
       UNREACHABLE();
       break;
   }
 }
 
@@ skipping 1911 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal