Support inlining function containing throw in the optimizer.

Support inlining function containing throw in the optimizer.

R=kmillikin@google.com

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

[Florian Schneider, 2013-05-22 11:40:39]

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:276: } else {
The diff is confused here. This part of the if-statement is taken unchanged from
the original function.

[Kevin Millikin (Google), 2013-05-22 12:05:11]

LGTM modulo the small issue of setting a deopt id/environment on the fresh
target block.

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:248: false_block->LinkTo(call_->next());
I guess it's safe that false_block does not have a deopt target, because we know
it's unreachable code.  For hygiene I would call InheritDeoptTarget for it.  I
think the right target is the deopt target after the call.

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:260: call_->previous()->LinkTo(branch);
You can call

AppendInstruction(call_->previous(), branch);

That allows you to skip the AddInputUse calls here for left and right, then they
probably don't need to be named.

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:270: // The graph entry (if present) is not in
the graph anymore.
This is the same as the last code in the else block.  Move it out of the
if/else?

[Florian Schneider, 2013-05-24 12:51:56]

Addressed comments.

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:248: false_block->LinkTo(call_->next());
On 2013/05/22 12:05:11, kmillikin wrote:
> I guess it's safe that false_block does not have a deopt target, because we
know
> it's unreachable code.  For hygiene I would call InheritDeoptTarget for it.  I
> think the right target is the deopt target after the call.

Done.

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:260: call_->previous()->LinkTo(branch);
On 2013/05/22 12:05:11, kmillikin wrote:
> You can call
> 
> AppendInstruction(call_->previous(), branch);
> 
> That allows you to skip the AddInputUse calls here for left and right, then
they
> probably don't need to be named. 

Done. Added AppendInstruction helper to the Instruction class.

https://codereview.chromium.org/15730003/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:270: // The graph entry (if present) is not in
the graph anymore.
On 2013/05/22 12:05:11, kmillikin wrote:
> This is the same as the last code in the else block.  Move it out of the
> if/else?

Done.

[Florian Schneider, 2013-05-24 12:52:54]

Committed patchset #2 manually as r23142 (presubmit successful).