Slight cleanup/unification of UpdateCache code.

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 694 matching lines @@
   if (lookup->holder() != *object &&
       HasNormalObjectsInPrototypeChain(
           isolate(), lookup, object->GetPrototype())) {
     // Suppress optimization for prototype chains with slow properties objects
     // in the middle.
     return;
   }
 
   // Compute the number of arguments.
   int argc = target()->arguments_count();
-  bool had_proto_failure = false;
   Handle<Code> code;
   if (state == UNINITIALIZED) {
     // This is the first time we execute this inline cache.
     // Set the target to the pre monomorphic stub to delay
     // setting the monomorphic state.
     code = isolate()->stub_cache()->ComputeCallPreMonomorphic(
         argc, kind_, extra_ic_state);
   } else if (state == MONOMORPHIC) {
     if (kind_ == Code::CALL_IC &&
         TryUpdateExtraICState(lookup, object, &extra_ic_state)) {
       code = ComputeMonomorphicStub(lookup, state, extra_ic_state,
                                     object, name);
     } else if (kind_ == Code::CALL_IC &&
                TryRemoveInvalidPrototypeDependentStub(target(),
                                                       *object,
                                                       *name)) {
-      had_proto_failure = true;
+      state = MONOMORPHIC_PROTOTYPE_FAILURE;
       code = ComputeMonomorphicStub(lookup, state, extra_ic_state,
                                     object, name);
     } else {
       code = isolate()->stub_cache()->ComputeCallMegamorphic(
           argc, kind_, extra_ic_state);
     }
   } else {
     code = ComputeMonomorphicStub(lookup, state, extra_ic_state,
                                   object, name);
   }
 
   // If there's no appropriate stub we simply avoid updating the caches.
   if (code.is_null()) return;
 
   // Patch the call site depending on the state of the cache.
-  if (state == UNINITIALIZED ||
-      state == PREMONOMORPHIC ||
-      state == MONOMORPHIC ||
-      state == MONOMORPHIC_PROTOTYPE_FAILURE) {
-    set_target(*code);
-  } else if (state == MEGAMORPHIC) {
-    // Cache code holding map should be consistent with
-    // GenerateMonomorphicCacheProbe. It is not the map which holds the stub.
-    Handle<JSObject> cache_object = object->IsJSObject()
-        ? Handle<JSObject>::cast(object)
-        : Handle<JSObject>(JSObject::cast(object->GetPrototype()));
-    // Update the stub cache.
-    isolate()->stub_cache()->Set(*name, cache_object->map(), *code);
+  switch (state) {
+    case UNINITIALIZED:
+    case MONOMORPHIC_PROTOTYPE_FAILURE:
+    case PREMONOMORPHIC:
+      set_target(*code);
+      break;
+    case MONOMORPHIC:
+      if (code->ic_state() != MONOMORPHIC) {
+        Map* map = target()->FindFirstMap();

[ulan, 2013/01/07 15:04:54]

Can we add a comment here like in LoadIC case?

Any reason why we use different checks here (code->ic_state() != MONOMORPHIC)
and in LoadIC (target() != *code) ?

[Toon Verwaest, 2013/01/07 15:28:45]

The code above calculates the target different from the other code. It may
update the monomorphic stub, thus staying monomorphic. It will already assign
code to be a megamorphic stub if it needs to become megamorphic in the code
above. Hence, we only update the cache if that code has decided to become
megamorphic.

On 2013/01/07 15:04:54, ulan wrote:

+        if (map != NULL) {
+          isolate()->stub_cache()->Set(*name, map, target());
+        }
+      }
+      set_target(*code);
+      break;
+    case MEGAMORPHIC: {
+      // Cache code holding map should be consistent with
+      // GenerateMonomorphicCacheProbe. It is not the map which holds the stub.
+      Handle<JSObject> cache_object = object->IsJSObject()
+          ? Handle<JSObject>::cast(object)
+          : Handle<JSObject>(JSObject::cast(object->GetPrototype()));
+      // Update the stub cache.
+      isolate()->stub_cache()->Set(*name, cache_object->map(), *code);
+      break;
+    }
+    case DEBUG_BREAK:
+    case DEBUG_PREPARE_STEP_IN:
+      break;
   }
 
-  if (had_proto_failure) state = MONOMORPHIC_PROTOTYPE_FAILURE;
   TRACE_IC(kind_ == Code::CALL_IC ? "CallIC" : "KeyedCallIC",
            name, state, target());
 }
 
 
 MaybeObject* KeyedCallIC::LoadFunction(State state,
                                        Handle<Object> object,
                                        Handle<Object> key) {
   if (key->IsSymbol()) {
     return CallICBase::LoadFunction(state,
@@ skipping 243 matching lines @@
         ASSERT(HasInterceptorGetter(*holder));
         code = isolate()->stub_cache()->ComputeLoadInterceptor(
             name, receiver, holder);
         break;
       default:
         return;
     }
   }
 
   // Patch the call site depending on the state of the cache.
-  if (state == UNINITIALIZED ||
-      state == PREMONOMORPHIC ||
-      state == MONOMORPHIC_PROTOTYPE_FAILURE) {
-    set_target(*code);
-  } else if (state == MONOMORPHIC) {
-    // We are transitioning from monomorphic to megamorphic case.
-    // Place the current monomorphic stub and stub compiled for
-    // the receiver into stub cache.
-    Map* map = target()->FindFirstMap();
-    if (map != NULL) {
-      isolate()->stub_cache()->Set(*name, map, target());
-    }
-    isolate()->stub_cache()->Set(*name, receiver->map(), *code);
+  switch (state) {
+    case UNINITIALIZED:
+    case PREMONOMORPHIC:
+    case MONOMORPHIC_PROTOTYPE_FAILURE:
+      set_target(*code);
+      break;
+    case MONOMORPHIC:
+      if (target() != *code) {
+        // We are transitioning from monomorphic to megamorphic case.
+        // Place the current monomorphic stub and stub compiled for
+        // the receiver into stub cache.
+        Map* map = target()->FindFirstMap();
+        if (map != NULL) {
+          isolate()->stub_cache()->Set(*name, map, target());
+        }
+        isolate()->stub_cache()->Set(*name, receiver->map(), *code);
 
-    set_target(*megamorphic_stub());
-  } else if (state == MEGAMORPHIC) {
-    // Cache code holding map should be consistent with
-    // GenerateMonomorphicCacheProbe.
-    isolate()->stub_cache()->Set(*name, receiver->map(), *code);
+        set_target(*megamorphic_stub());
+      }
+      break;
+    case MEGAMORPHIC:
+      // Cache code holding map should be consistent with
+      // GenerateMonomorphicCacheProbe.
+      isolate()->stub_cache()->Set(*name, receiver->map(), *code);

[ulan, 2013/01/07 15:04:54]

missing break.

[Toon Verwaest, 2013/01/07 15:28:45]

Done.

+    case DEBUG_BREAK:
+    case DEBUG_PREPARE_STEP_IN:
+      break;
   }
 
   TRACE_IC("LoadIC", name, state, target());
 }
 
 
 Handle<Code> KeyedLoadIC::GetElementStubWithoutMapCheck(
     bool is_js_array,
     ElementsKind elements_kind,
     KeyedAccessGrowMode grow_mode) {
@@ skipping 233 matching lines @@
             name, receiver, holder);
         break;
       default:
         // Always rewrite to the generic case so that we do not
         // repeatedly try to rewrite.
         code = generic_stub();
         break;
     }
   }
 
-  // Patch the call site depending on the state of the cache.  Make
-  // sure to always rewrite from monomorphic to megamorphic.
-  ASSERT(state != MONOMORPHIC_PROTOTYPE_FAILURE);
-  if (state == UNINITIALIZED || state == PREMONOMORPHIC) {
-    set_target(*code);
-  } else if (state == MONOMORPHIC) {
-    set_target(*megamorphic_stub());
+  // Patch the call site depending on the state of the cache.
+  switch (state) {
+    case UNINITIALIZED:
+    case PREMONOMORPHIC:
+      set_target(*code);
+      break;
+    case MONOMORPHIC:
+      // Only move to megamorphic if the target changes.
+      if (target() != *code) {
+        set_target(*megamorphic_stub());
+      }

[ulan, 2013/01/07 15:04:54]

missing break.

[Toon Verwaest, 2013/01/07 15:28:45]

Done.

+    case MEGAMORPHIC:
+    case DEBUG_BREAK:
+    case DEBUG_PREPARE_STEP_IN:
+      break;
+    case MONOMORPHIC_PROTOTYPE_FAILURE:
+      UNREACHABLE();
+      break;
   }
 
   TRACE_IC("KeyedLoadIC", name, state, target());
 }
 
 
 static bool StoreICableLookup(LookupResult* lookup) {
   // Bail out if we didn't find a result.
   if (!lookup->IsFound()) return false;
 
@@ skipping 224 matching lines @@
 
       break;
     }
     case NONEXISTENT:
     case HANDLER:
       UNREACHABLE();
       return;
   }
 
   // Patch the call site depending on the state of the cache.
-  if (state == UNINITIALIZED || state == MONOMORPHIC_PROTOTYPE_FAILURE) {
-    set_target(*code);
-  } else if (state == MONOMORPHIC) {
-    // Only move to megamorphic if the target changes.
-    if (target() != *code) {
-      set_target((strict_mode == kStrictMode)
-                   ? megamorphic_stub_strict()
-                   : megamorphic_stub());
-    }
-  } else if (state == MEGAMORPHIC) {
-    // Update the stub cache.
-    isolate()->stub_cache()->Set(*name, receiver->map(), *code);
+  switch (state) {
+    case UNINITIALIZED:
+    case PREMONOMORPHIC:
+    case MONOMORPHIC_PROTOTYPE_FAILURE:
+      set_target(*code);
+      break;
+    case MONOMORPHIC:
+      // Only move to megamorphic if the target changes.
+      if (target() != *code) {
+        Map* map = target()->FindFirstMap();
+        if (map != NULL) {
+          isolate()->stub_cache()->Set(*name, map, target());

[ulan, 2013/01/07 15:04:54]

Can we add a comment explaining this stub cache updates like in LoadIC case?

[Toon Verwaest, 2013/01/07 15:28:45]

Done.

+        }
+        isolate()->stub_cache()->Set(*name, receiver->map(), *code);
+        set_target((strict_mode == kStrictMode)
+                     ? megamorphic_stub_strict()
+                     : megamorphic_stub());
+      }
+      break;
+    case MEGAMORPHIC:
+      // Update the stub cache.
+      isolate()->stub_cache()->Set(*name, receiver->map(), *code);
+      break;
+    case DEBUG_BREAK:
+    case DEBUG_PREPARE_STEP_IN:
+      break;
   }
 
   TRACE_IC("StoreIC", name, state, target());
 }
 
 
 static bool AddOneReceiverMapIfMissing(MapHandleList* receiver_maps,
                                        Handle<Map> new_receiver_map) {
   ASSERT(!new_receiver_map.is_null());
   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)) {
       return false;
     }
   }
   receiver_maps->Add(new_receiver_map);
   return true;
 }
 
 
 void KeyedIC::GetReceiverMapsForStub(Handle<Code> stub,
                                      MapHandleList* result) {
   ASSERT(stub->is_inline_cache_stub());
   if (!string_stub().is_null() && stub.is_identical_to(string_stub())) {
     return result->Add(isolate()->factory()->string_map());
   } else if (stub->is_keyed_load_stub() || stub->is_keyed_store_stub()) {
-    if (stub->ic_state() == MONOMORPHIC) {
-      result->Add(Handle<Map>(stub->FindFirstMap()));
-    } else {
-      ASSERT(stub->ic_state() == MEGAMORPHIC);
-      AssertNoAllocation no_allocation;
-      int mask = RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT);
-      for (RelocIterator it(*stub, mask); !it.done(); it.next()) {
-        RelocInfo* info = it.rinfo();
-        Handle<Object> object(info->target_object());
-        ASSERT(object->IsMap());
-        AddOneReceiverMapIfMissing(result, Handle<Map>::cast(object));
+    switch (stub->ic_state()) {
+      case MONOMORPHIC:
+        result->Add(Handle<Map>(stub->FindFirstMap()));
+        break;
+      case MEGAMORPHIC: {
+        AssertNoAllocation no_allocation;
+        int mask = RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT);
+        for (RelocIterator it(*stub, mask); !it.done(); it.next()) {
+          RelocInfo* info = it.rinfo();
+          Handle<Object> object(info->target_object());
+          ASSERT(object->IsMap());
+          AddOneReceiverMapIfMissing(result, Handle<Map>::cast(object));
+        }
+        break;
       }
+      case UNINITIALIZED:
+      case PREMONOMORPHIC:
+      case MONOMORPHIC_PROTOTYPE_FAILURE:
+      case DEBUG_BREAK:
+      case DEBUG_PREPARE_STEP_IN:
+        UNREACHABLE();
+        break;
     }
   }
 }
 
 
 Handle<Code> KeyedIC::ComputeStub(Handle<JSObject> receiver,
                                   StubKind stub_kind,
                                   StrictModeFlag strict_mode,
                                   Handle<Code> generic_stub) {
   State ic_state = target()->ic_state();
@@ skipping 407 matching lines @@
           : generic_stub();
       break;
     case HANDLER:
     case NONEXISTENT:
       UNREACHABLE();
       return;
   }
 
   ASSERT(!code.is_null());
 
-  // Patch the call site depending on the state of the cache.  Make
-  // sure to always rewrite from monomorphic to megamorphic.
-  ASSERT(state != MONOMORPHIC_PROTOTYPE_FAILURE);
-  if (state == UNINITIALIZED || state == PREMONOMORPHIC) {
-    set_target(*code);
-  } else if (state == MONOMORPHIC) {
-    set_target((strict_mode == kStrictMode)
-                 ? *megamorphic_stub_strict()
-                 : *megamorphic_stub());
+  // Patch the call site depending on the state of the cache.
+  switch (state) {
+    case UNINITIALIZED:
+    case PREMONOMORPHIC:
+      set_target(*code);
+      break;
+    case MONOMORPHIC:
+      // Only move to megamorphic if the target changes.
+      if (target() != *code) {
+        set_target((strict_mode == kStrictMode)
+                     ? *megamorphic_stub_strict()
+                     : *megamorphic_stub());
+      }
+      break;
+    case MEGAMORPHIC:
+    case DEBUG_BREAK:
+    case DEBUG_PREPARE_STEP_IN:
+      break;
+    case MONOMORPHIC_PROTOTYPE_FAILURE:
+      UNREACHABLE();
+      break;
   }
 
   TRACE_IC("KeyedStoreIC", name, state, target());
 }
 
 
 #undef TRACE_IC
 
 
 // ----------------------------------------------------------------------------
 // Static IC stub generators.
 //
 
 // Used from ic-<arch>.cc.
 RUNTIME_FUNCTION(MaybeObject*, CallIC_Miss) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 2);
   CallIC ic(isolate);
   IC::State state = IC::StateFrom(ic.target(), args[0], args[1]);
   Code::ExtraICState extra_ic_state = ic.target()->extra_ic_state();
   MaybeObject* maybe_result = ic.LoadFunction(state,
                                               extra_ic_state,
                                               args.at<Object>(0),
                                               args.at<String>(1));
-  // Result could be a function or a failure.
-  JSFunction* raw_function = NULL;

[ulan, 2013/01/07 15:04:54]

I am not sure, might be that we need it to avoid compiler warnings on some
platform.

[Toon Verwaest, 2013/01/07 15:28:45]

I doubt it. It would be the only maybe->To(&value) site to require this... There
are many different legacy versions of this idiom lying around the codebase; I'm
just cleaning them up as I see them ;-)

On 2013/01/07 15:04:54, ulan wrote:

+  JSFunction* raw_function;
   if (!maybe_result->To(&raw_function)) return maybe_result;
 
   // The first time the inline cache is updated may be the first time the
-  // function it references gets called.  If the function is lazily compiled
-  // then the first call will trigger a compilation.  We check for this case
+  // function it references gets called. If the function is lazily compiled
+  // then the first call will trigger a compilation. We check for this case
   // and we do the compilation immediately, instead of waiting for the stub
   // currently attached to the JSFunction object to trigger compilation.
   if (raw_function->is_compiled()) return raw_function;
 
   Handle<JSFunction> function(raw_function);
   JSFunction::CompileLazy(function, CLEAR_EXCEPTION);
   return *function;
 }
 
 
@@ skipping 701 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal