Reapply "Inlining of static calls with trivial function bodies."

Reapply "Inlining of static calls with trivial function bodies."

Reapply r11579 with a bug fix.
Original review at https://chromiumcodereview.appspot.com/10919004

R=kmillikin@google.com

Committed: https://code.google.com/p/dart/source/detail?r=11586

[Kevin Millikin (Google), 2012-08-30 08:54:28]

LGTM.

[srdjan, 2012-09-05 17:40:05]

DBC.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph.h
File runtime/vm/flow_graph.h (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph....
runtime/vm/flow_graph.h:72: void ComputeSSA(intptr_t
next_virtual_register_number = 0);
Avoid using default arguments. There is only one call site where you have to
specify 0.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_builder.cc (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:2504: FlowGraph*
FlowGraphBuilder::BuildGraphForInlining(InliningContext context) {
You may want to print the AST if you have succeeded with building graph for
inlining (with appropriate preamble)

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_builder.h (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.h:28: kTestContext
Why is this needed? From code it seems that you only need to check if you are
inlining or not.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.h:62: if (InInliningContext()) {
Why not check for exits_ != NULL instead?

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.h:158: // actually added to the graph.
The comment is not clear, please revise. Maybe describe it in context of
inlining? If needed we can chat tomorrow ( I am in CET  time zone).

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_inliner.cc (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_inliner.cc:17: DEFINE_FLAG(bool, trace_inlining, false,
"Trace inlining");
I would like to trace only inlined functions. Maybe incorporate it in
FLAG_trace_compiler?

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_inliner.cc:147: bool preformed_inlining() const { return
inlined_; }
What does preformed inlining mean?

[zerny-google, 2012-09-07 14:08:43]

Hi Srdjan, below are answers to your comments and questions. I'll be gone next
week so Kevin will land the actual changes.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph.h
File runtime/vm/flow_graph.h (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph....
runtime/vm/flow_graph.h:72: void ComputeSSA(intptr_t
next_virtual_register_number = 0);
On 2012/09/05 17:40:05, srdjan wrote:
> Avoid using default arguments. There is only one call site where you have to
> specify 0.

Done. (In next CL for inlining monomorphic calls.)

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_builder.cc (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.cc:2504: FlowGraph*
FlowGraphBuilder::BuildGraphForInlining(InliningContext context) {
On 2012/09/05 17:40:05, srdjan wrote:
> You may want to print the AST if you have succeeded with building graph for
> inlining (with appropriate preamble)

That is done in flow_graph_inliner.cc after it has been built, ssa transformed
and linked in the existing graph.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_builder.h (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.h:28: kTestContext
On 2012/09/05 17:40:05, srdjan wrote:
> Why is this needed? From code it seems that you only need to check if you are
> inlining or not.

We will need it to specialize inlining calls in a test context.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.h:62: if (InInliningContext()) {
On 2012/09/05 17:40:05, srdjan wrote:
> Why not check for exits_ != NULL instead?

The exits array might make sense outside an inlining context.

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_builder.h:158: // actually added to the graph.
On 2012/09/05 17:40:05, srdjan wrote:
> The comment is not clear, please revise. Maybe describe it in context of
> inlining? If needed we can chat tomorrow ( I am in CET  time zone).

Could you clarify this Kevin?

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_inliner.cc (right):

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_inliner.cc:17: DEFINE_FLAG(bool, trace_inlining, false,
"Trace inlining");
On 2012/09/05 17:40:05, srdjan wrote:
> I would like to trace only inlined functions. Maybe incorporate it in
> FLAG_trace_compiler?

Could you elaborate. I'm not sure I understand?

https://chromiumcodereview.appspot.com/10911004/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_inliner.cc:147: bool preformed_inlining() const { return
inlined_; }
On 2012/09/05 17:40:05, srdjan wrote:
> What does preformed inlining mean?

Typo there. I meant to write performed, but have changed it to just inlined(),
as in the inliner has inlined code in the graph.