Inline monomorphic calls.

runtime/vm/flow_graph_inliner.cc

Index: runtime/vm/flow_graph_inliner.cc
diff --git a/runtime/vm/flow_graph_inliner.cc b/runtime/vm/flow_graph_inliner.cc
index 76893c0b6092bf4f30189e03c9951c00cb131230..889dadc6181c0f29b7b2ee44263ccf43b4358543 100644
--- a/runtime/vm/flow_graph_inliner.cc
+++ b/runtime/vm/flow_graph_inliner.cc
@@ -26,18 +26,21 @@ class CallSiteInliner : public FlowGraphVisitor {
         next_ssa_temp_index_(flow_graph->max_virtual_register_number()),
         inlined_(false) { }
 
-  void TryInlining(const Function& function,
+  bool TryInlining(const Function& function,
                    GrowableArray<Value*>* arguments,
-                   StaticCallInstr* call) {
-    // TODO(zerny): Generalize to all calls.
+                   Definition* call,
+                   Definition* check) {

[srdjan, 2012/09/11 14:44:07]

Maybe describe briefly what is 'check' used for in comment.

[Kevin Millikin (Google), 2012/09/11 16:29:20]

I think I can get rid of it per Florian's comment below.

+    if (FLAG_trace_inlining) {
+      OS::Print("--- %s\n", function.ToFullyQualifiedCString());
+    }
 
-    // Abort if the callee has optional parameters.
+    // Abort if the callee has named parameters.

[srdjan, 2012/09/11 14:44:07]

why change named -> optionals, as optional is used everywhere below?

[Kevin Millikin (Google), 2012/09/11 16:29:20]

Good question.  I changed it back.

     if (function.HasOptionalParameters()) {
       if (FLAG_trace_inlining) {
         OS::Print("Inline aborted %s\nReason: optional parameters\n",
                   function.ToFullyQualifiedCString());
       }
-      return;
+      return false;
     }
 
     // Assuming no optional parameters the actual/formal count should match.
@@ -55,11 +58,12 @@ class CallSiteInliner : public FlowGraphVisitor {
       // Parse the callee function.
       ParsedFunction parsed_function(function);
       Parser::ParseFunction(&parsed_function);
+      parsed_function.AllocateVariables();
       FlowGraphBuilder builder(parsed_function);
 
       // Build the callee graph.
       FlowGraph* callee_graph =
-          builder.BuildGraphForInlining(FlowGraphBuilder::kValueContext);
+          builder.BuildGraph(FlowGraphBuilder::kValueContext);
 
       // Abort if the callee graph contains control flow.
       if (callee_graph->preorder().length() != 2) {
@@ -69,7 +73,7 @@ class CallSiteInliner : public FlowGraphVisitor {
           OS::Print("Inline aborted %s\nReason: control flow\n",
                     parsed_function.function().ToFullyQualifiedCString());
         }
-        return;
+        return false;
       }
 
       if (FLAG_trace_inlining && FLAG_print_flow_graph) {
@@ -101,6 +105,19 @@ class CallSiteInliner : public FlowGraphVisitor {
       caller_graph_->InlineCall(call, callee_graph);
       next_ssa_temp_index_ = caller_graph_->max_virtual_register_number();
 
+      // Insert check if needed.
+      if (check != NULL) {
+        if (call->env() != NULL) call->env()->DeepCopyTo(check);
+        check->InsertAfter(call->previous());
+      }
+
+      // Remove (all) push arguments of the call.
+      for (intptr_t i = 0; i < call->ArgumentCount(); ++i) {
+        PushArgumentInstr* push = call->ArgumentAt(i);
+        push->ReplaceUsesWith(push->value()->definition());
+        push->RemoveFromGraph();
+      }
+
       // Replace all the formal parameters with the actuals.
       for (intptr_t i = 0; i < arguments->length(); ++i) {
         Value* val = callee_graph->graph_entry()->start_env()->ValueAt(i);
@@ -109,6 +126,10 @@ class CallSiteInliner : public FlowGraphVisitor {
         param->ReplaceUsesWith((*arguments)[i]->definition());
       }
 
+      // Replace callees null constant with callers null constant.

[srdjan, 2012/09/11 14:44:07]

s/callees/callee's/

ditto callers

[Kevin Millikin (Google), 2012/09/11 16:29:20]

Done.

+      callee_graph->graph_entry()->constant_null()->ReplaceUsesWith(
+          caller_graph_->graph_entry()->constant_null());
+
       if (FLAG_trace_inlining) {
         OS::Print("Inlined %s\n", function.ToFullyQualifiedCString());
       }
@@ -117,6 +138,7 @@ class CallSiteInliner : public FlowGraphVisitor {
       inlined_ = true;
       isolate->set_long_jump_base(base);
       isolate->set_ic_data_array(old_ic_data.raw());
+      return true;
     } else {
       Error& error = Error::Handle();
       error = isolate->object_store()->sticky_error();
@@ -128,19 +150,102 @@ class CallSiteInliner : public FlowGraphVisitor {
                   function.ToFullyQualifiedCString(),
                   error.ToErrorCString());
       }
+      return false;
+    }
+  }
+
+  void VisitClosureCall(ClosureCallInstr* call) {
+    if (FLAG_trace_inlining) OS::Print("Closure call\n");
+    // Find the closure of the callee.
+    ASSERT(call->ArgumentCount() > 0);
+    const CreateClosureInstr* closure =
+        call->ArgumentAt(0)->value()->definition()->AsCreateClosure();
+    if (closure == NULL) {
+      if (FLAG_trace_inlining) {
+        OS::Print("Inline aborted: non-closure operator.\n");
+      }
+      return;
+    }
+    GrowableArray<Value*> arguments(call->ArgumentCount() - 1);
+    for (int i = 1; i < call->ArgumentCount(); ++i) {
+      arguments.Add(call->ArgumentAt(i)->value());
+    }
+    TryInlining(closure->function(), &arguments, call, NULL);  // No check.
+  }
+
+  void VisitInstanceCall(InstanceCallInstr* call) {

[Florian Schneider, 2012/09/11 14:36:09]

I think it's not necessary to have VisitInstanceCall. All instance calls that
have IC data and a monomorphic target should have been transformed into
CheckClass + PolymorphicInstanceCall instructions previously by the optimizer.

By doing this maybe you can also avoid checking the IC data again for the same
things that are done in FlowGraphOptimizer::VisitInstanceCall.

+    TryInliningInstanceCall(call, call);
+  }
+
+  // Needed since the instance call in a polymorphic instance call is not linked
+  // in the graph.
+  void TryInliningInstanceCall(InstanceCallInstr* call, Definition* call_defn) {
+    if (FLAG_trace_inlining) OS::Print("Instance call\n");
+    if (!call->HasICData()) {
+      if (FLAG_trace_inlining) OS::Print("Inline aborted: has no IC data\n");
+      return;
+    }
+    const ICData& ic_data = *call->ic_data();
+
+    ASSERT(ic_data.num_args_tested() > 0);
+
+    if (ic_data.NumberOfChecks() == 0) {
+      if (FLAG_trace_inlining) OS::Print("Inline aborted: no IC checks\n");
+      return;
     }
+    if (ic_data.NumberOfChecks() != 1) {
+      // TODO(zerny): proceed if each check has the same cid?

[srdjan, 2012/09/11 14:44:07]

s/cid/target/ ?

+      if (FLAG_trace_inlining) OS::Print("Inline aborted: non monomorphic\n");
+      return;
+    }
+    GrowableArray<intptr_t> class_ids;
+    Function& target = Function::Handle();
+    ic_data.GetCheckAt(0, &class_ids, &target);
+    intptr_t class_id = class_ids[0];
+    if (class_id == kIllegalCid) {
+      if (FLAG_trace_inlining) OS::Print("Inline aborted: invalid receiver\n");
+      return;
+    }
+    if (class_id == kDynamicCid) {
+      if (FLAG_trace_inlining) OS::Print("Inline aborted: dynamic receiver\n");
+      return;
+    }
+
+    Definition* check;
+    // Construct a class check for the polymorphic call.
+    if (call->ic_data()->GetReceiverClassIdAt(0) == kSmiCid) {

[Florian Schneider, 2012/09/11 14:36:09]

Maybe it's cleaner for now to not handle smi receivers here.

+      check =
+          new CheckSmiInstr(call->ArgumentAt(0)->value(), call->deopt_id());
+    } else {
+      const ICData& unary_checks =
+          ICData::ZoneHandle(call->ic_data()->AsUnaryClassChecks());

[Florian Schneider, 2012/09/11 14:36:09]

I'd abort inlining if the target in not unique:

if (!HasOneTarget(unary_checks)) return;

This is also done in FlowGraphOptimizer::VisitInstanceCall.

+      check =
+          new CheckClassInstr(call->ArgumentAt(0)->value(), call, unary_checks);
+    }
+
+    GrowableArray<Value*> arguments(call->ArgumentCount());
+    for (int i = 0; i < call->ArgumentCount(); ++i) {
+      arguments.Add(call->ArgumentAt(i)->value());
+    }
+
+    TryInlining(target, &arguments, call_defn, check);
+  }
+
+  void VisitPolymorphicInstanceCall(PolymorphicInstanceCallInstr* call) {
+    if (FLAG_trace_inlining) OS::Print("Polymorphic ");
+    TryInliningInstanceCall(call->instance_call(), call);

[srdjan, 2012/09/11 14:44:07]

PolymorphicInstanceCall has a with_checks tester. If it returns false, no check
should be emitted.

[Kevin Millikin (Google), 2012/09/11 16:29:20]

You are right.  And in the case it's needed, there is already a check so no need
to insert one here.

   }
 
-  void VisitStaticCall(StaticCallInstr* instr) {
+  void VisitStaticCall(StaticCallInstr* call) {
     if (FLAG_trace_inlining) OS::Print("Static call\n");
-    GrowableArray<Value*> arguments(instr->ArgumentCount());
-    for (int i = 0; i < instr->ArgumentCount(); ++i) {
-      arguments.Add(instr->ArgumentAt(i)->value());
+    GrowableArray<Value*> arguments(call->ArgumentCount());
+    for (int i = 0; i < call->ArgumentCount(); ++i) {
+      arguments.Add(call->ArgumentAt(i)->value());
     }
-    TryInlining(instr->function(), &arguments, instr);
+    TryInlining(call->function(), &arguments, call, NULL);  // no check
   }
 
-  bool preformed_inlining() const { return inlined_; }
+  bool inlined() const { return inlined_; }
 
  private:
   FlowGraph* caller_graph_;
@@ -167,7 +272,7 @@ void FlowGraphInliner::Inline() {
   CallSiteInliner inliner(flow_graph_);
   inliner.VisitBlocks();
 
-  if (inliner.preformed_inlining()) {
+  if (inliner.inlined()) {
     if (FLAG_trace_inlining && FLAG_print_flow_graph) {
       OS::Print("After Inlining of %s\n", flow_graph_->
                 parsed_function().function().ToFullyQualifiedCString());