Create a function call IC

src/ic.cc

Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index 2607bb3b68d53405be03865fe60538dea4e8b9e0..8a352f0b85e384549eb078b72c851180ad172ae3 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -91,9 +91,11 @@ void IC::TraceIC(const char* type,
     }
     JavaScriptFrame::PrintTop(isolate(), stdout, false, true);
     ExtraICState extra_state = new_target->extra_ic_state();
-    const char* modifier =
-        GetTransitionMarkModifier(
-            KeyedStoreIC::GetKeyedAccessStoreMode(extra_state));
+    const char* modifier = "";
+    if (new_target->kind() != Code::CALL_IC) {

[Toon Verwaest, 2014/03/10 13:50:44]

Isn't this only relevant for KEYED_STORE_IC?

[mvstanton, 2014/03/20 15:51:53]

Done.

+      modifier = GetTransitionMarkModifier(
+          KeyedStoreIC::GetKeyedAccessStoreMode(extra_state));
+    }
     PrintF(" (%c->%c%s)",
            TransitionMarkFromState(state()),
            TransitionMarkFromState(new_state),
@@ -422,6 +424,7 @@ void IC::Clear(Isolate* isolate, Address address) {
     case Code::STORE_IC: return StoreIC::Clear(isolate, address, target);
     case Code::KEYED_STORE_IC:
       return KeyedStoreIC::Clear(isolate, address, target);
+    case Code::CALL_IC: return CallIC::Clear(address, target);
     case Code::COMPARE_IC: return CompareIC::Clear(isolate, address, target);
     case Code::COMPARE_NIL_IC: return CompareNilIC::Clear(address, target);
     case Code::BINARY_OP_IC:
@@ -434,6 +437,11 @@ void IC::Clear(Isolate* isolate, Address address) {
 }
 
 
+void CallIC::Clear(Address address, Code* target) {
+  // TODO(mvstanton): What does it mean to clear this hybrid?
+}
+
+
 void KeyedLoadIC::Clear(Isolate* isolate, Address address, Code* target) {
   if (IsCleared(target)) return;
   // Make sure to also clear the map used in inline fast cases.  If we
@@ -1264,6 +1272,37 @@ MaybeObject* StoreIC::Store(Handle<Object> object,
 }
 
 
+void CallIC::State::Print(StringStream* stream) const {
+  stream->Add("(args(%d), ",
+              argc_);
+  stream->Add("%s, ",
+              call_as_method_ ? "METHOD" : "FUNCTION");
+  stream->Add("%s, ",
+              monomorphic_ ? "MONOMORPHIC" : "NOT_MONOMORPHIC");
+  stream->Add("%s, ",
+              args_match_ ? "args_match" : "args_dont_match");
+  stream->Add("%s)",
+              strict_native_ ? "strict_or_native" : "not_strict_or_native");
+}
+
+
+void CallIC::GenerateNormal(MacroAssembler* masm, int argc,
+                            bool call_as_method) {
+  CallICStub stub(State(argc, call_as_method, false, true, false));
+  stub.GetCode(masm->isolate());
+}
+
+
+Handle<Code> CallIC::initialize_stub(Isolate* isolate,
+                                     int argc,
+                                     bool call_as_method) {
+  CallICStub stub(State(argc, call_as_method, false, true, false));
+  ASSERT(stub.call_as_method() == call_as_method);
+  Handle<Code> code = stub.GetCode(isolate);
+  return code;
+}
+
+
 Handle<Code> StoreIC::initialize_stub(Isolate* isolate,
                                       StrictModeFlag strict_mode) {
   ExtraICState extra_state = ComputeExtraICState(strict_mode);
@@ -1725,6 +1764,111 @@ MaybeObject* KeyedStoreIC::Store(Handle<Object> object,
 }
 
 
+CallIC::State::State(ExtraICState extra_ic_state) {
+  argc_ = ArgBits::decode(extra_ic_state);
+  call_as_method_ = CallAsMethodBits::decode(extra_ic_state);
+  monomorphic_ = MonomorphicBits::decode(extra_ic_state);
+  args_match_ = ArgsMatchBits::decode(extra_ic_state);
+  strict_native_ = StrictNativeBits::decode(extra_ic_state);
+}
+
+
+ExtraICState CallIC::State::GetExtraICState() const {
+  ExtraICState extra_ic_state =
+      ArgBits::encode(argc_) |
+      CallAsMethodBits::encode(call_as_method_) |
+      MonomorphicBits::encode(monomorphic_) |
+      ArgsMatchBits::encode(args_match_) |
+      StrictNativeBits::encode(strict_native_);
+  return extra_ic_state;
+}
+
+
+void CallIC::PatchMegamorphic(int arg_count, bool call_as_method) {
+  CallICStub stub(State(arg_count, call_as_method));
+  set_target(*stub.GetCode(isolate()));
+  TRACE_GENERIC_IC(isolate(), "CallIC", "megamorphic");
+}
+
+
+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());
+  if (feedback->IsJSFunction()) {

[Toon Verwaest, 2014/03/10 13:50:44]

Shouldn't this be feedback->IsJSFunction() && function->IsJSFunction() ?

[mvstanton, 2014/03/20 15:51:53]

I went ahead and just returned without changing anything now if
!function->IsJSFunction().

+    // We are going megamorphic, only if we were given a real function,
+    // otherwise the slow path will be taken.

[Toon Verwaest, 2014/03/10 13:50:44]

Weird sentence

[mvstanton, 2014/03/20 15:51:53]

Removed.

+    vector->set(slot->value(),
+                *TypeFeedbackInfo::MegamorphicSentinel(isolate()));
+    // Do we need to patch? Only if we currently don't have the default stub

[Toon Verwaest, 2014/03/10 13:50:44]

We only need to patch if ...

[mvstanton, 2014/03/20 15:51:53]

Done.

+    // in place.
+    if (state.monomorphic()) {
+      PatchMegamorphic(state.arg_count(), state.call_as_method());
+    }
+  } else {
+    // If we came here feedback must be the uninitialized sentinel,
+    // and we are going monomorphic.
+    ASSERT(feedback == *TypeFeedbackInfo::UninitializedSentinel(isolate()));
+
+    // Ugh. Do nothing if the function isn't a function, the slow path is taken.

[Toon Verwaest, 2014/03/10 13:50:44]

Remove ugh.

[mvstanton, 2014/03/20 15:51:53]

Done.

+    if (!function->IsJSFunction()) {
+      // If we don't have a function, just go megamorphic.
+      vector->set(slot->value(),
+                  *TypeFeedbackInfo::MegamorphicSentinel(isolate()));
+      if (state.monomorphic()) {
+        PatchMegamorphic(state.arg_count(), state.call_as_method());
+      }
+    } else {
+      vector->set(slot->value(), *function);
+      SharedFunctionInfo* shared = Handle<JSFunction>::cast(function)->shared();
+      // Choose the right monomorphic handler
+      bool arg_count_match =
+          shared->formal_parameter_count() == state.arg_count();
+      bool strict_or_native = shared->language_mode() == STRICT_MODE ||
+          shared->native();
+
+      bool must_patch = false;
+      if (state.monomorphic()) {
+        // We are here because we are in snapshot code that went monomorphic but
+        // now our cell is cleared because we are running an application.
+        // Ideally we can just patch the cell and return.

[Toon Verwaest, 2014/03/10 13:50:44]

Are we sure that we can only get here because of snapshot code?

Why not just: Stay monomorphic if the type feedback in the vector was cleared,
but the IC wasn't (and is still monomorphic).

[mvstanton, 2014/03/20 15:51:53]

I went to objects-visiting-inl.h and make sure that monomorphic call ics are
cleared in sync with the associated feedback cells. This is the simplest thing
for now. So if our type cell is uninitialized we won't have a specialized
MONOMORPHIC CallIC in place.

If this turns out to be a performance issue I can address it with your
suggestion, which sounds good.

+        must_patch = arg_count_match != state.args_match() ||
+            strict_or_native != state.strict_native();
+      } else {
+        // We need to patch only if attributes differ from the monomorphic case
+        // built into the generic stub (with arg_count_match = true and
+        // !strict_or_native)
+        must_patch = !arg_count_match || strict_or_native;
+      }
+
+      Handle<String> name;

[Toon Verwaest, 2014/03/10 13:50:44]

Don't you want to initialize this to something like the empty_string?

[mvstanton, 2014/03/20 15:51:53]

Yes, I could simplify that even more.

+      Handle<JSFunction> js_function = Handle<JSFunction>::cast(function);
+      if (js_function->shared()->name()->IsString()) {
+        name = Handle<String>(String::cast(js_function->shared()->name()),
+                              isolate());
+      }
+
+      if (must_patch) {
+        CallICStub stub(State(state.arg_count(),
+                              state.call_as_method(),
+                              true,
+                              arg_count_match,
+                              strict_or_native));
+        set_target(*stub.GetCode(isolate()));
+#ifdef DEBUG
+        const char *str = state.monomorphic()
+            ? "CallIC (snapshot miss)"
+            : "CallIC";
+#endif
+        TRACE_IC(str, name);
+      }
+    }
+  }
+}
+
+
 #undef TRACE_IC
 
 
@@ -1733,6 +1877,19 @@ MaybeObject* KeyedStoreIC::Store(Handle<Object> object,
 //
 
 // Used from ic-<arch>.cc.
+RUNTIME_FUNCTION(MaybeObject*, CallIC_Miss) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 4);
+  CallIC ic(isolate);
+  Handle<Object> receiver = args.at<Object>(0);
+  Handle<Object> function = args.at<Object>(1);
+  Handle<FixedArray> vector = args.at<FixedArray>(2);
+  Handle<Smi> slot = args.at<Smi>(3);
+  ic.HandleMiss(receiver, function, vector, slot);
+  return *function;
+}
+
+
 // Used from ic-<arch>.cc.
 RUNTIME_FUNCTION(MaybeObject*, LoadIC_Miss) {
   HandleScope scope(isolate);