Allow invalidation and recompilation of instance allocation stubs. Requested in order to implement…

Allow invalidation and recompilation of instance allocation stubs. Requested in order to implement better pretenuring strategy.

R=zra@google.com

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

[srdjan, 2014-09-09 22:10:20]

srdjan@google.com changed reviewers:
+ zra@google.com

[koda, 2014-09-10 15:33:21]

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

[koda, 2014-09-10 15:33:22]

This is great functionality to have.

Eventually, we'll also want this for the bulit-in classes, so that specific
allocation sites for lists, doubles, etc. can be recompiled for pretenuring.

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
runtime/vm/code_generator.cc:1349: while (frame != NULL && (frame->IsStubFrame()
|| frame->IsExitFrame())) {
This explicit null-check looks strange, since null should not appear (and if it
does, the "->" deref would still crash immediately also in release mode).
Consider only having it ASSERTs (moving it into the loop+before).

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/compiler_test.cc
File runtime/vm/compiler_test.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/compiler_test...
runtime/vm/compiler_test.cc:96: owner ^= stub.owner();
Expect the owner to be cls.

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/compiler_test...
runtime/vm/compiler_test.cc:99: EXPECT_VALID(result);
Expect the result to be an A instance.

[zra, 2014-09-10 16:07:39]

lgtm

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
runtime/vm/code_generator.cc:1349: while (frame != NULL && (frame->IsStubFrame()
|| frame->IsExitFrame())) {
parens around (frame != NULL)

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_mip...
File runtime/vm/stub_code_mips.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_mip...
runtime/vm/stub_code_mips.cc:441: __ TraceSimMsg("FixCallersTarget");
"FixAllocationStubTarget"

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_x64.cc
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_x64...
runtime/vm/stub_code_x64.cc:370: 
Extra newline

[srdjan, 2014-09-11 16:22:09]

On 2014/09/10 15:33:22, koda wrote:
> This is great functionality to have.
> 
> Eventually, we'll also want this for the bulit-in classes, so that specific
> allocation sites for lists, doubles, etc. can be recompiled for pretenuring.

Most often this would mean deoptimizing an optimized function, but there are
some more stubs that should be recompiled as well (my TODO).

> 
>
https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
> File runtime/vm/code_generator.cc (right):
> 
>
https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
> runtime/vm/code_generator.cc:1349: while (frame != NULL &&
(frame->IsStubFrame()
> || frame->IsExitFrame())) {
> This explicit null-check looks strange, since null should not appear (and if
it
> does, the "->" deref would still crash immediately also in release mode).
> Consider only having it ASSERTs (moving it into the loop+before).
> 
>
https://codereview.chromium.org/557913002/diff/40001/runtime/vm/compiler_test.cc
> File runtime/vm/compiler_test.cc (right):
> 
>
https://codereview.chromium.org/557913002/diff/40001/runtime/vm/compiler_test...
> runtime/vm/compiler_test.cc:96: owner ^= stub.owner();
> Expect the owner to be cls.
> 
>
https://codereview.chromium.org/557913002/diff/40001/runtime/vm/compiler_test...
> runtime/vm/compiler_test.cc:99: EXPECT_VALID(result);
> Expect the result to be an A instance.

[srdjan, 2014-09-11 16:22:36]

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
runtime/vm/code_generator.cc:1349: while (frame != NULL && (frame->IsStubFrame()
|| frame->IsExitFrame())) {
On 2014/09/10 15:33:21, koda wrote:
> This explicit null-check looks strange, since null should not appear (and if
it
> does, the "->" deref would still crash immediately also in release mode).
> Consider only having it ASSERTs (moving it into the loop+before).

Done, also for FixCallersTarget above.

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/code_generato...
runtime/vm/code_generator.cc:1349: while (frame != NULL && (frame->IsStubFrame()
|| frame->IsExitFrame())) {
On 2014/09/10 16:07:39, zra wrote:
> parens around (frame != NULL)

Removed that test

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_mip...
File runtime/vm/stub_code_mips.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_mip...
runtime/vm/stub_code_mips.cc:441: __ TraceSimMsg("FixCallersTarget");
On 2014/09/10 16:07:39, zra wrote:
> "FixAllocationStubTarget"

Done.

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_x64.cc
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/557913002/diff/40001/runtime/vm/stub_code_x64...
runtime/vm/stub_code_x64.cc:370: 
On 2014/09/10 16:07:39, zra wrote:
> Extra newline

Done.

[srdjan, 2014-09-11 17:57:28]

Committed patchset #5 (id:80001) manually as r40168 (presubmit successful).