CallIC must update type feedback info correctly.

src/ic.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
 #include "src/api.h"
 #include "src/arguments.h"
 #include "src/codegen.h"
@@ skipping 358 matching lines @@
   }
 }
 
 
 void IC::PostPatching(Address address, Code* target, Code* old_target) {
   Isolate* isolate = target->GetHeap()->isolate();
   Code* host = isolate->
       inner_pointer_to_code_cache()->GetCacheEntry(address)->code;
   if (host->kind() != Code::FUNCTION) return;
 
+  // Type vector based ICs update these statistics differently.
+  if (target->kind() == Code::CALL_IC) return;
+
   if (FLAG_type_info_threshold > 0 && old_target->is_inline_cache_stub() &&
       target->is_inline_cache_stub() &&
-      // Call ICs don't have interesting state changes from this point
-      // of view.
-      target->kind() != Code::CALL_IC &&
       // Not all Code objects have TypeFeedbackInfo.
       host->type_feedback_info()->IsTypeFeedbackInfo()) {
     int polymorphic_delta = 0;  // "Polymorphic" here includes monomorphic.
     int generic_delta = 0;      // "Generic" here includes megamorphic.
     ComputeTypeInfoCountDelta(old_target->ic_state(), target->ic_state(),
                               &polymorphic_delta, &generic_delta);
     TypeFeedbackInfo* info = TypeFeedbackInfo::cast(host->type_feedback_info());
     info->change_ic_with_type_info_count(polymorphic_delta);
     info->change_ic_generic_count(generic_delta);
   }
@@ skipping 1519 matching lines @@
 
     CallIC_ArrayStub stub(isolate(), state);
     set_target(*stub.GetCode());
     Handle<String> name;
     if (array_function->shared()->name()->IsString()) {
       name = Handle<String>(String::cast(array_function->shared()->name()),
                             isolate());
     }
 
     TRACE_IC("CallIC (Array call)", name);
+    Object* new_feedback = vector->get(slot->value());
+    UpdateTypeFeedbackInfo(feedback, new_feedback);
     return true;
   }
   return false;
 }
 
 
 void CallIC::PatchMegamorphic(Handle<FixedArray> vector,
                               Handle<Smi> slot) {
   State state(target()->extra_ic_state());
 
   // We are going generic.
   vector->set(slot->value(),
               *TypeFeedbackInfo::MegamorphicSentinel(isolate()),
               SKIP_WRITE_BARRIER);
 
   CallICStub stub(isolate(), state);
   Handle<Code> code = stub.GetCode();
   set_target(*code);
 
   TRACE_GENERIC_IC(isolate(), "CallIC", "megamorphic");
 }
 
 
+IC::State CallIC::FeedbackObjectToState(Object* feedback) {
+  IC::State state = UNINITIALIZED;
+
+  if (feedback == *TypeFeedbackInfo::MegamorphicSentinel(isolate())) {
+    state = GENERIC;
+  } else if (feedback->IsAllocationSite() || feedback->IsJSFunction()) {
+    state = MONOMORPHIC;
+  } else {
+    CHECK(feedback == *TypeFeedbackInfo::UninitializedSentinel(isolate()));
+  }
+
+  return state;
+}
+
+
+void CallIC::UpdateTypeFeedbackInfo(Object* old_feedback, Object* new_feedback) {

[Jakob Kummerow, 2014/08/06 14:49:09]

nit: 80col

[mvstanton, 2014/08/06 15:32:33]

Done.

+  // Convert old_feedback to an IC_State enum:

[Jakob Kummerow, 2014/08/06 14:49:09]

This comment seems to belong to line 1971, but I don't think it adds much value,
so feel free to just drop it.

[mvstanton, 2014/08/06 15:32:33]

Done.

+  Code* host = isolate()->

[Jakob Kummerow, 2014/08/06 14:49:09]

Idea for reducing code duplication: pull out most of this method (except for
lines 1971, 1972) into a helper with signature (Address*, IC::State old_state,
IC::State new_state).

The drawback is that you'll have to compute the states before you know whether
you'll need them; but that should be fairly cheap, and at least some of the
computations can be avoided: callers of this method know the new_state in many
(all?) cases, e.g. for vector->set(...MegamorphicSentinel...).

[mvstanton, 2014/08/06 15:32:33]

Excellent idea, done. I think the cost of computing a state var is very cheap.

+      inner_pointer_to_code_cache()->GetCacheEntry(address())->code;
+  if (host->kind() != Code::FUNCTION) return;
+
+  if (FLAG_type_info_threshold > 0 &&
+      host->type_feedback_info()->IsTypeFeedbackInfo()) {
+    IC::State old_state = FeedbackObjectToState(old_feedback);
+    IC::State new_state = FeedbackObjectToState(new_feedback);
+    int polymorphic_delta = 0;  // "Polymorphic" here includes monomorphic.
+    int generic_delta = 0;      // "Generic" here includes megamorphic.
+    ComputeTypeInfoCountDelta(old_state, new_state,
+                              &polymorphic_delta, &generic_delta);
+    TypeFeedbackInfo* info = TypeFeedbackInfo::cast(host->type_feedback_info());
+    info->change_ic_with_type_info_count(polymorphic_delta);
+    info->change_ic_generic_count(generic_delta);
+  }
+  if (host->type_feedback_info()->IsTypeFeedbackInfo()) {
+    TypeFeedbackInfo* info =
+        TypeFeedbackInfo::cast(host->type_feedback_info());
+    info->change_own_type_change_checksum();
+  }
+  host->set_profiler_ticks(0);
+  isolate()->runtime_profiler()->NotifyICChanged();
+}
+
+
 void CallIC::HandleMiss(Handle<Object> receiver,
                         Handle<Object> function,
                         Handle<FixedArray> vector,
                         Handle<Smi> slot) {
   State state(target()->extra_ic_state());
   Object* feedback = vector->get(slot->value());
 
   // Hand-coded MISS handling is easier if CallIC slots don't contain smis.
   DCHECK(!feedback->IsSmi());
 
@@ skipping 18 matching lines @@
     if (FLAG_use_ic &&
         DoCustomHandler(receiver, function, vector, slot, state)) {
       return;
     }
 
     Handle<JSFunction> js_function = Handle<JSFunction>::cast(function);
     Handle<Object> name(js_function->shared()->name(), isolate());
     TRACE_IC("CallIC", name);
     vector->set(slot->value(), *function);
   }
+
+  Object* new_feedback = vector->get(slot->value());
+  UpdateTypeFeedbackInfo(feedback, new_feedback);
 }
 
 
 #undef TRACE_IC
 
 
 // ----------------------------------------------------------------------------
 // Static IC stub generators.
 //
 
@@ skipping 1098 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal