Fix bugs in generating and printing of Crankshaft stubs

Fix bugs in generating and printing of Crankshaft stubs

Committed: http://code.google.com/p/v8/source/detail?r=13716

[danno, 2013-02-21 15:01:30]

PTAL

[Michael Starzinger, 2013-02-21 19:39:23]

https://codereview.chromium.org/12317044/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/deoptimizer.cc#newcode1676
src/deoptimizer.cc:1676: return -1;
Add a "// Minus receiver." comment to the end of this line to explain this magic
-1 value.

https://codereview.chromium.org/12317044/diff/1/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/heap.cc#newcode6212
src/heap.cc:6212: bool Heap::Prepare() {
I think it makes more sense to keep this method be called SetUp() and instead
rename the latter one to CreateHeapObject().

https://codereview.chromium.org/12317044/diff/1/src/heap.cc#newcode6321
src/heap.cc:6321: store_buffer()->SetUp();
Also is it possible to move all of the steps that happen after heap object
creation (i.e. store buffer setup and relocation mutex creation) into the above
method? Thereby we could get rid of the whole create_heap_objects argument and
only call this method if no deserializer is present.

https://codereview.chromium.org/12317044/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/hydrogen.cc#newcode638
src/hydrogen.cc:638: BailoutId id)
Still fits into one line.

https://codereview.chromium.org/12317044/diff/1/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/isolate.cc#newcode1
src/isolate.cc:1: // Copyright 2013 the V8 project authors. All rights reserved.
I thought we don't change the copyright year anymore?

[danno, 2013-02-25 11:11:23]

Feedback addressed, please take another look.

https://codereview.chromium.org/12317044/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/deoptimizer.cc#newcode1676
src/deoptimizer.cc:1676: return -1;
On 2013/02/21 19:39:23, Michael Starzinger wrote:
> Add a "// Minus receiver." comment to the end of this line to explain this
magic
> -1 value.

Done.

https://codereview.chromium.org/12317044/diff/1/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/heap.cc#newcode6212
src/heap.cc:6212: bool Heap::Prepare() {
On 2013/02/21 19:39:23, Michael Starzinger wrote:
> I think it makes more sense to keep this method be called SetUp() and instead
> rename the latter one to CreateHeapObject().

Done.

https://codereview.chromium.org/12317044/diff/1/src/heap.cc#newcode6321
src/heap.cc:6321: store_buffer()->SetUp();
On 2013/02/21 19:39:23, Michael Starzinger wrote:
> Also is it possible to move all of the steps that happen after heap object
> creation (i.e. store buffer setup and relocation mutex creation) into the
above
> method? Thereby we could get rid of the whole create_heap_objects argument and
> only call this method if no deserializer is present.

Done.

https://codereview.chromium.org/12317044/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/hydrogen.cc#newcode638
src/hydrogen.cc:638: BailoutId id)
On 2013/02/21 19:39:23, Michael Starzinger wrote:
> Still fits into one line.

Done.

https://codereview.chromium.org/12317044/diff/1/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/12317044/diff/1/src/isolate.cc#newcode1
src/isolate.cc:1: // Copyright 2013 the V8 project authors. All rights reserved.
On 2013/02/21 19:39:23, Michael Starzinger wrote:
> I thought we don't change the copyright year anymore?

Done.

[Michael Starzinger, 2013-02-25 12:00:57]

LGTM with a few final nits.

https://codereview.chromium.org/12317044/diff/2010/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/12317044/diff/2010/src/heap.h#newcode484
src/heap.h:484: // without actually actually creating any objects.
Drop one of the two "actually" in the comment.

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc#newcode2043
src/isolate.cc:2043: V8::FatalProcessOutOfMemory("heap preparation");
s/heap preparation/heap setup/

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc#newcode2049
src/isolate.cc:2049: // SetUp the object heap.
Move this comment up to before the assertion in line 2041.

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc#newcode2052
src/isolate.cc:2052: V8::FatalProcessOutOfMemory("heap setup");
s/heap setup/heap object creation/

[danno, 2013-02-25 14:25:05]

Feedback addressed.

https://codereview.chromium.org/12317044/diff/2010/src/heap.h
File src/heap.h (right):

https://codereview.chromium.org/12317044/diff/2010/src/heap.h#newcode484
src/heap.h:484: // without actually actually creating any objects.
On 2013/02/25 12:00:57, Michael Starzinger wrote:
> Drop one of the two "actually" in the comment.

Done.

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc#newcode2043
src/isolate.cc:2043: V8::FatalProcessOutOfMemory("heap preparation");
On 2013/02/25 12:00:57, Michael Starzinger wrote:
> s/heap preparation/heap setup/

Done.

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc#newcode2049
src/isolate.cc:2049: // SetUp the object heap.
On 2013/02/25 12:00:57, Michael Starzinger wrote:
> Move this comment up to before the assertion in line 2041.

Done.

https://codereview.chromium.org/12317044/diff/2010/src/isolate.cc#newcode2052
src/isolate.cc:2052: V8::FatalProcessOutOfMemory("heap setup");
On 2013/02/25 12:00:57, Michael Starzinger wrote:
> s/heap setup/heap object creation/

Done.