Emit stackmaps even in unoptimized code if slow path pushes untagged values.

Emit stackmaps even in unoptimized code if slow path pushes untagged values.

These stackmaps cover only pushed registers region, the rest of the stack (copied parameters, locals and temporaries) will be scanned as tagged values.

R=fschneider@google.com
BUG=http://dartbug.com/22621

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

[Vyacheslav Egorov (Google), 2015-03-03 15:26:51]

+koda

[Florian Schneider, 2015-03-03 15:37:35]

lgtm

[Vyacheslav Egorov (Google), 2015-03-03 15:43:47]

Committed patchset #1 (id:1) manually as 44173 (presubmit successful).

[koda, 2015-03-03 17:57:48]

koda@google.com changed reviewers:
+ koda@google.com

[koda, 2015-03-03 17:57:48]

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
runtime/vm/flow_graph_compiler.cc:872: // The unoptimizing compiler has no stack
maps.
Update comment.

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
File runtime/vm/flow_graph_compiler.h (right):

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
runtime/vm/flow_graph_compiler.h:642: stackmap_table_builder_ = new
StackmapTableBuilder();
Memory leak?

We should also assert in the destructor that the stack maps were actually
finalized (if non-NULL).

https://codereview.chromium.org/968183003/diff/1/tests/language/vm/regress_22...
File tests/language/vm/regress_22621_vm_test.dart (right):

https://codereview.chromium.org/968183003/diff/1/tests/language/vm/regress_22...
tests/language/vm/regress_22621_vm_test.dart:9: return re.firstMatch("oooo");
Perhaps I should have added a comment here: "oooo" was chosen since the low bits
of 'o' make it neither a smi nor a well-formed pointer.

We could also consider making it "oooooooo" in case we add a 64-bit version of
LoadCodeUnits.

[Vyacheslav Egorov (Google), 2015-03-03 18:01:07]

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
runtime/vm/flow_graph_compiler.cc:872: // The unoptimizing compiler has no stack
maps.
On 2015/03/03 17:57:48, koda wrote:
> Update comment.

Acknowledged.

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
File runtime/vm/flow_graph_compiler.h (right):

https://codereview.chromium.org/968183003/diff/1/runtime/vm/flow_graph_compil...
runtime/vm/flow_graph_compiler.h:642: stackmap_table_builder_ = new
StackmapTableBuilder();
On 2015/03/03 17:57:48, koda wrote:
> Memory leak?
> 
> We should also assert in the destructor that the stack maps were actually
> finalized (if non-NULL).

it's a ZoneAllocated object as most of the things in the compiler, so it can't
leak anywhere (thus neither does it have its destructor invoked)

https://codereview.chromium.org/968183003/diff/1/tests/language/vm/regress_22...
File tests/language/vm/regress_22621_vm_test.dart (right):

https://codereview.chromium.org/968183003/diff/1/tests/language/vm/regress_22...
tests/language/vm/regress_22621_vm_test.dart:9: return re.firstMatch("oooo");
On 2015/03/03 17:57:48, koda wrote:
> Perhaps I should have added a comment here: "oooo" was chosen since the low
bits
> of 'o' make it neither a smi nor a well-formed pointer.
> 
> We could also consider making it "oooooooo" in case we add a 64-bit version of
> LoadCodeUnits.

Acknowledged.