Ensure that all goto instructions have deoptimization target.

Ensure that all goto instructions have deoptimization target.

R=kmillikin@google.com
BUG=

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

[Vyacheslav Egorov (Google), 2013-04-05 16:07:43]

This is ready for another look.

It proved to be monster change.

The purpose of this CL is to ensure that each Goto instruction has a
deoptimization target. 

However to ensure that I had to attach a deoptimization environment to every
Branch on StrictCompare instruction, to every StrictCompare, and to every block
entry because during graph rewriting we need to find appropriate environments to
use for newly created blocks and branches.

[Vyacheslav Egorov (Google), 2013-04-05 16:08:50]

I also had to change how we encode kDeoptBefore/kDeoptAfter difference. Now
deopt_id itself corresponds to either deoptimization point "before" or "after"
so that you can target both with normal deoptimization stubs.

[Florian Schneider, 2013-04-12 09:23:55]

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/deopt_instructi...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/deopt_instructi...
runtime/vm/deopt_instructions.cc:665: static_cast<DeoptRetAddressInstr*>(instr);
check that deopt_id is a lazy-deopt:

ASSERT((ret_address_instr->deopt_id() & 1) == kDeoptIdAfterOffset)

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler.h (left):

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler.h:267: intptr_t deopt_id,
Why remove deopt_id here?

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_ia32.cc (right):

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:508: Isolate::kNoDeoptId,
Why kNoDeoptId here?

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_x64.cc (right):

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_x64.cc:507: Isolate::kNoDeoptId,
Why kNoDeoptId here?

[Florian Schneider, 2013-04-12 10:13:22]

Runtime calls need a deopt_id in case of an exception.

On 2013/04/12 09:23:55, Florian Schneider wrote:
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/deopt_instructi...
> File runtime/vm/deopt_instructions.cc (right):
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/deopt_instructi...
> runtime/vm/deopt_instructions.cc:665:
static_cast<DeoptRetAddressInstr*>(instr);
> check that deopt_id is a lazy-deopt:
> 
> ASSERT((ret_address_instr->deopt_id() & 1) == kDeoptIdAfterOffset)
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
> File runtime/vm/flow_graph_compiler.h (left):
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
> runtime/vm/flow_graph_compiler.h:267: intptr_t deopt_id,
> Why remove deopt_id here?
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
> File runtime/vm/flow_graph_compiler_ia32.cc (right):
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
> runtime/vm/flow_graph_compiler_ia32.cc:508: Isolate::kNoDeoptId,
> Why kNoDeoptId here?
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
> File runtime/vm/flow_graph_compiler_x64.cc (right):
> 
>
https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
> runtime/vm/flow_graph_compiler_x64.cc:507: Isolate::kNoDeoptId,
> Why kNoDeoptId here?

[Vyacheslav Egorov (Google), 2013-04-12 10:57:06]

Thanks Florian.

I don't think we ever throw from inside the instanceof runtime function but I
reinstated deopt id for InstanceOfInstr.

Please take another look.

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/deopt_instructi...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/deopt_instructi...
runtime/vm/deopt_instructions.cc:665: static_cast<DeoptRetAddressInstr*>(instr);
On 2013/04/12 09:23:55, Florian Schneider wrote:
> check that deopt_id is a lazy-deopt:
> 
> ASSERT((ret_address_instr->deopt_id() & 1) == kDeoptIdAfterOffset)

Done.

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler.h (left):

https://codereview.chromium.org/12457034/diff/5001/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler.h:267: intptr_t deopt_id,
On 2013/04/12 09:23:55, Florian Schneider wrote:
> Why remove deopt_id here?

Reinstated.

[Florian Schneider, 2013-04-12 11:01:52]

LGTM.

https://codereview.chromium.org/12457034/diff/14001/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/12457034/diff/14001/runtime/vm/code_generator...
runtime/vm/code_generator.cc:579: UNREACHABLE();
Accidental edit?

[Vyacheslav Egorov (Google), 2013-04-12 11:02:48]

Thanks. Landing.

https://codereview.chromium.org/12457034/diff/14001/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/12457034/diff/14001/runtime/vm/code_generator...
runtime/vm/code_generator.cc:579: UNREACHABLE();
On 2013/04/12 11:01:52, Florian Schneider wrote:
> Accidental edit?

Indeed. Reverted.

[Vyacheslav Egorov (Google), 2013-04-12 11:20:02]

Committed patchset #5 manually as r21351 (presubmit successful).