Make array allocation stub shared between isolates.

Make array allocation stub shared between isolates.

This allows to make the last explicitly named stubs shared between isolates.

When sharing code stubs, we can't do patching at their entry anymore.
Therefore, I had to remove patching support of the array allocation stub.
Is this a functionality we want to keep?

The change is mostly performance-neutral because optimized code has an inlined fast
path for array allocation and only uses the stub for the slow-case.


The only isolate-specific stubs left are object allocation stubs which are
associated with their Class are per-isolate.

Since this CL removes any isolate-specific stubs from StubCode, it becomes AllStatic.

BUG=
R=koda@google.com

Committed: https://github.com/dart-lang/sdk/commit/8d38a58c18309a05e9fddfdd209dbfc5094c0738

[Florian Schneider, 2015-07-22 08:00:17]

fschneider@google.com changed reviewers:
+ iposva@google.com, johnmccutchan@google.com, koda@google.com

[Cutch, 2015-07-22 16:56:34]

As discussed in the meeting on Monday, all of our allocation stubs must be
patchable to support allocation stack tracing. In a non-precompiled world, we
want to support this.

[koda, 2015-07-23 00:58:01]

LGTM

Regarding patchability: one potential use case is to specialize the stub for
old/new space allocation (per call site). However, since we do have an inlined
"fast" version, maybe it makes more sense to recompile entire method, and we can
swallow an explicit flag check+branch in the "slow" stub case.

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

https://codereview.chromium.org/1247783002/diff/1/runtime/vm/flow_graph_compi...
runtime/vm/flow_graph_compiler_arm.cc:201: __
BranchLink(&StubCode::DeoptimizeLabel());
True already before your change, but even more obvious now: It would be nice to
have the code make it clear/enforce that this is a const pointer (why can't we
use const reference here?), to reassure the reader and protect against changes
to BrankLink/StubCode interfaces.

[Florian Schneider, 2015-07-23 10:41:51]

On 2015/07/22 16:56:34, Cutch wrote:
> As discussed in the meeting on Monday, all of our allocation stubs must be
> patchable to support allocation stack tracing. In a non-precompiled world, we
> want to support this.

The tracing is still supported. I added a runtime check in the array allocation
stub using the assembler macro MaybeTraceAllocation.

This costs a bit extra, but the stub is not used for the fast-path from
optimized code, so I don't expect any regressions.

[Florian Schneider, 2015-07-23 10:43:22]

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

https://codereview.chromium.org/1247783002/diff/1/runtime/vm/flow_graph_compi...
runtime/vm/flow_graph_compiler_arm.cc:201: __
BranchLink(&StubCode::DeoptimizeLabel());
On 2015/07/23 00:58:01, koda wrote:
> True already before your change, but even more obvious now: It would be nice
to
> have the code make it clear/enforce that this is a const pointer (why can't we
> use const reference here?), to reassure the reader and protect against changes
> to BrankLink/StubCode interfaces.

I would also prefer a const-reference. I'll see if this is easy to do as a
separate change.

[Florian Schneider, 2015-07-23 10:56:21]

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