VM: [Kernel] Implement CatchBlockEntryInstr::should_restore_closure_context on all platforms.

VM: [Kernel] Implement CatchBlockEntryInstr::should_restore_closure_context on all platforms.

This was the only Kernel specific backend change - to simplify the graph builder
we allowed it to capture exception and stack trace variables. This in turn
requires CatchBlockEntryInstr to respect that this variables can be captured
and correctly update them when exception is caught.

This was implemented on X64 and ARM before. This CL expands implementation to
all other implementations including DBC.

R=zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/7e1d584218bca5061a773f92a4a49292d005a8a4

[Vyacheslav Egorov (Google), 2017-04-05 14:59:42]

vegorov@google.com changed reviewers:
+ kustermann@google.com, zra@google.com

[Vyacheslav Egorov (Google), 2017-04-05 14:59:43]

Please take a look.

[zra, 2017-04-05 16:10:45]

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_arm64.cc (right):

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm64.cc:2630: __ ldr(CTX, Address(FP,
closure_parameter->index() * kWordSize));
Is the closure_parameter index known to be something small? If it might ever not
be, then this load should use LoadFromOffset().

Since the stores below use StoreToOffset, I'm thinking this load and the next
should just use LoadFromOffset to be consistent.

Looks like arm should be changed as well.

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_mips.cc (right):

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
runtime/vm/intermediate_language_mips.cc:2766: __ lw(CTX, Address(FP,
closure_parameter->index() * kWordSize));
ditto

[Vyacheslav Egorov (Google), 2017-04-05 16:23:30]

Addressed comments, please take another look.

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_arm64.cc (right):

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
runtime/vm/intermediate_language_arm64.cc:2630: __ ldr(CTX, Address(FP,
closure_parameter->index() * kWordSize));
On 2017/04/05 16:10:45, zra wrote:
> Is the closure_parameter index known to be something small? If it might ever
not
> be, then this load should use LoadFromOffset().
> 
> Since the stores below use StoreToOffset, I'm thinking this load and the next
> should just use LoadFromOffset to be consistent.
> 
> Looks like arm should be changed as well.

Done (both arm and arm64)

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
File runtime/vm/intermediate_language_mips.cc (right):

https://codereview.chromium.org/2796283003/diff/20001/runtime/vm/intermediate...
runtime/vm/intermediate_language_mips.cc:2766: __ lw(CTX, Address(FP,
closure_parameter->index() * kWordSize));
On 2017/04/05 16:10:45, zra wrote:
> ditto

Done.

[zra, 2017-04-05 16:34:34]

lgtm

[Vyacheslav Egorov (Google), 2017-04-05 18:32:10]

Description was changed from

==========
VM: [Kernel] Implement CatchBlockEntryInstr::should_restore_closure_context on
all platforms.

This was the only Kernel specific backend change - to simplify the graph builder
we allowed it to capture exception and stack trace variables. This in turn
requires CatchBlockEntryInstr to respect that this variables can be captured
and correctly update them when exception is caught.

This was implemented on X64 and ARM before. This CL expands implementation to
all other implementations including DBC.
==========

to

==========
VM: [Kernel] Implement CatchBlockEntryInstr::should_restore_closure_context on
all platforms.

This was the only Kernel specific backend change - to simplify the graph builder
we allowed it to capture exception and stack trace variables. This in turn
requires CatchBlockEntryInstr to respect that this variables can be captured
and correctly update them when exception is caught.

This was implemented on X64 and ARM before. This CL expands implementation to
all other implementations including DBC.

R=zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/7e1d584218bca5061a773f92a4a49292d005a8a4
==========

[Vyacheslav Egorov (Google), 2017-04-05 18:32:11]

Committed patchset #3 (id:40001) manually as
7e1d584218bca5061a773f92a4a49292d005a8a4 (presubmit successful).