VM: [DBC] Fix lazy deoptimization after calls that return no values.

VM: [DBC] Fix lazy deoptimization after calls that return no values.

On DBC calls return value on the stack instead of a dedicate register but not all calls have a return value (e.g. CheckStack does not) and such calls don't push anything. Implementation of a lazy deopt however assumed that value is always present and tried to manually preserve it by popping and pushing it back after the frame was rewritten. This of course damaged the frame if we performed a lazy deoptimization after the call that did not push anything.

Instead of manually preserving result value value use frame translation to handle it. This allows to handle calls that return value and those that do not uniformly in the Deopt bytecode. Compiler takes care of creating the right deoptimization environment instead.

BUG=
R=zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/1fcc7384aa28aa2ca17b8ec304dd2c4e083c36cd

[Vyacheslav Egorov (Google), 2016-12-19 19:09:22]

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

[Vyacheslav Egorov (Google), 2016-12-19 19:09:22]

Please take a look.

This brings us down to 12 failures on the dbc + hotreload configuration.

[zra, 2016-12-19 19:15:18]

If it's easy to write a regression test for this, we should probably have one.

https://codereview.chromium.org/2587133002/diff/1/runtime/vm/flow_graph_compi...
File runtime/vm/flow_graph_compiler_dbc.cc (right):

https://codereview.chromium.org/2587133002/diff/1/runtime/vm/flow_graph_compi...
runtime/vm/flow_graph_compiler_dbc.cc:275: static const intptr_t kArgCount = 0;
static?

[Vyacheslav Egorov (Google), 2016-12-19 21:55:19]

Added a test

https://codereview.chromium.org/2587133002/diff/1/runtime/vm/flow_graph_compi...
File runtime/vm/flow_graph_compiler_dbc.cc (right):

https://codereview.chromium.org/2587133002/diff/1/runtime/vm/flow_graph_compi...
runtime/vm/flow_graph_compiler_dbc.cc:275: static const intptr_t kArgCount = 0;
On 2016/12/19 19:15:18, zra wrote:
> static?

Muscle memory

[zra, 2016-12-19 21:56:57]

lgtm

[Vyacheslav Egorov (Google), 2016-12-19 22:08:29]

Description was changed from

==========
VM: [DBC] Fix lazy deoptimization after calls that return no values.

On DBC calls return value on the stack instead of a dedicate register but not
all calls have a return value (e.g. CheckStack does not) and such calls don't
push anything. Implementation of a lazy deopt however assumed that value is
always present and tried to manually preserve it by popping and pushing it back
after the frame was rewritten. This of course damaged the frame if we performed
a lazy deoptimization after the call that did not push anything.

Instead of manually preserving result value value use frame translation to
handle it. This allows to handle calls that return value and those that do not
uniformly in the Deopt bytecode. Compiler takes care of creating the right
deoptimization environment instead.

BUG=
==========

to

==========
VM: [DBC] Fix lazy deoptimization after calls that return no values.

On DBC calls return value on the stack instead of a dedicate register but not
all calls have a return value (e.g. CheckStack does not) and such calls don't
push anything. Implementation of a lazy deopt however assumed that value is
always present and tried to manually preserve it by popping and pushing it back
after the frame was rewritten. This of course damaged the frame if we performed
a lazy deoptimization after the call that did not push anything.

Instead of manually preserving result value value use frame translation to
handle it. This allows to handle calls that return value and those that do not
uniformly in the Deopt bytecode. Compiler takes care of creating the right
deoptimization environment instead.

BUG=
R=zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/1fcc7384aa28aa2ca17b8ec304dd2c4e083c36cd
==========

[Vyacheslav Egorov (Google), 2016-12-19 22:08:31]

Committed patchset #4 (id:60001) manually as
1fcc7384aa28aa2ca17b8ec304dd2c4e083c36cd (presubmit successful).

[Florian Schneider, 2016-12-20 00:01:08]

Lgtm.

https://codereview.chromium.org/2587133002/diff/60001/tests/language/vm/lazy_...
File tests/language/vm/lazy_deopt_vm_test.dart (right):

https://codereview.chromium.org/2587133002/diff/60001/tests/language/vm/lazy_...
tests/language/vm/lazy_deopt_vm_test.dart:4: // Test deoptimization on an
optimistically hoisted smi check.
Comment not matching here.

https://codereview.chromium.org/2587133002/diff/60001/tests/language/vm/lazy_...
tests/language/vm/lazy_deopt_vm_test.dart:5: // VMOptions=--deoptimize_every=10
--optimization-counter-threshold=10  --no-background-compilation
--enable-inlining-annotations
Left-over flag? I don't see inlining annotations here.