Bump the C stack pointer when building Dart frames to maintain the ARM64 ABI requirements without g…

Bump the C stack pointer when building Dart frames to maintain the ARM64 ABI requirements without guessing the stack limit.

Fixes running with the profiler on ARM64.

Cf. 5d02929fa0605380712361e42df4999b7aabc3df.

BUG=http://dartbug.com/26472
R=vegorov@google.com, zra@google.com

Committed: https://github.com/dart-lang/sdk/commit/6e2c4636cddece36c87ca77f69955a8490c8a0dd

[rmacnak, 2016-05-27 04:44:50]

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

[rmacnak, 2016-05-27 04:44:51]

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

https://codereview.chromium.org/2014413003/diff/1/runtime/vm/assembler_arm64....
runtime/vm/assembler_arm64.cc:1149: const intptr_t kMaxDartFrameSize = 256;
Not sure where in the compiler to check this isn't exceeded.

[zra, 2016-05-27 14:58:39]

I'm worried that if we also don't switch to using CSP for the stack overflow
check, we'll still get flaky crashes in the signal handlers as we get close to
the limit.

[Vyacheslav Egorov (Google), 2016-05-27 15:34:52]

lgtm

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

https://codereview.chromium.org/2014413003/diff/1/runtime/vm/assembler_arm64....
runtime/vm/assembler_arm64.cc:1149: const intptr_t kMaxDartFrameSize = 256;
On 2016/05/27 04:44:51, rmacnak wrote:
> Not sure where in the compiler to check this isn't exceeded.

It's not that easy. 

FlowGraphCompiler::StackSize can give you the size of fixed portion of the
frame, but you will have to track the dynamic size (e.g. expression stack in the
unoptimized version and pushed arguments in the optimized version) manually. 

If you need a guidance of how to implement that - I am happy to help. It can be
modeled after FlowGraphCompiler::FrameStateUpdateWith() except that you need to
just track the frame height instead of tracking the representations of pushed
values. You would also need to carry information from predecessors to
successors.

[zra, 2016-05-27 15:42:58]

On 2016/05/27 15:34:52, Vyacheslav Egorov (Google) wrote:
> It's not that easy. 

I think we should consider taking the same approach as V8:

https://github.com/v8/v8/blob/master/src/arm64/macro-assembler-arm64.cc#L868

If performance isn't hurt a lot, I think it's preferable because it's definitely
simpler.

[rmacnak, 2016-05-27 17:42:04]

Increased the bump to 4k, and updated the issue for the stack overflow change
and longer term fix.

[rmacnak, 2016-05-27 22:12:00]

Fixed simulation of "and csp, tmp, ~15", PTAL.

[zra, 2016-05-31 21:20:22]

lgtm with test

There are no assembler tests that do EmitFrame, so there are no assembler tests
that cover the bug fixed in the simulator.

[rmacnak, 2016-05-31 22:35:43]

Several assembler tests do use EnterFrame (via EnterTestFrame), but added a more
specific assembler test.

[rmacnak, 2016-05-31 22:36:48]

Description was changed from

==========
Bump the C stack pointer when building Dart frames to maintain the ARM64 ABI
requirements without guessing the stack limit.

Fixes running with the profiler on ARM64.

Cf. 5d02929fa0605380712361e42df4999b7aabc3df.

BUG=http://dartbug.com/26472
==========

to

==========
Bump the C stack pointer when building Dart frames to maintain the ARM64 ABI
requirements without guessing the stack limit.

Fixes running with the profiler on ARM64.

Cf. 5d02929fa0605380712361e42df4999b7aabc3df.

BUG=http://dartbug.com/26472
R=vegorov@google.com, zra@google.com

Committed:
https://github.com/dart-lang/sdk/commit/6e2c4636cddece36c87ca77f69955a8490c8a0dd
==========

[rmacnak, 2016-05-31 22:36:50]

Committed patchset #4 (id:60001) manually as
6e2c4636cddece36c87ca77f69955a8490c8a0dd (presubmit successful).

[zra, 2016-05-31 22:40:11]

On 2016/05/31 22:35:43, rmacnak wrote:
> Several assembler tests do use EnterFrame (via EnterTestFrame), but added a
more
> specific assembler test.

Doh! You're right. I searched for the wrong string. Thanks for adding more
tests!