Compile FastCloneShallowObjectStub using Crankshaft.

Compile FastCloneShallowObjectStub using Crankshaft.

This changes FastCloneShallowObjectStub to be compiled independent of
the target architecture. It also adds tracing to the deoptimizer for
compiled stubs and contains some minor bugfixes.

R=danno@chromium.org

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

[Michael Starzinger, 2013-02-08 10:29:44]

Please note that architecture ports are still missing. But I wanted to get a
first round of comments before porting.

[danno, 2013-02-08 12:03:41]

I really like the direction this is going. Go ahead with the platform ports :-)

https://codereview.chromium.org/12220074/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/12220074/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:3625: class HInstanceSize: public
HTemplateInstruction<1> {
Could you add a TODO so that we revisit this when we fix FieldLoads and Stores?

[Jakob Kummerow, 2013-02-08 12:45:48]

You asked for DBC, you'll get a few drive-by nits :-)

Generally the approach looks good. I didn't look at everything in detail.

https://codereview.chromium.org/12220074/diff/1/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/12220074/diff/1/src/code-stubs-hydrogen.cc#ne...
src/code-stubs-hydrogen.cc:167: true, i));
nit: fits on last line

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

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.cc#newcode654
src/hydrogen.cc:654: failure_block->Goto(failure_block_);
This line looks weird. Too bad the edge split form requirement doesn't allow us
to get by without it.

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.cc#newcode678
src/hydrogen.cc:678: builder_->set_current_block(failure_block_);
I think this line is not needed if you
s/builder_->current_block()/failure_block_/ in the line below.

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.h#newcode934
src/hydrogen.h:934: Zone* zone() { return builder_->zone(); }
nit: methods before properties

[Michael Starzinger, 2013-02-11 12:13:22]

Comments addressed. Architecture ports implemented. This is ready for another
round of review.

https://codereview.chromium.org/12220074/diff/1/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/12220074/diff/1/src/code-stubs-hydrogen.cc#ne...
src/code-stubs-hydrogen.cc:167: true, i));
On 2013/02/08 12:45:49, Jakob wrote:
> nit: fits on last line

Done.

https://codereview.chromium.org/12220074/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/12220074/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:3625: class HInstanceSize: public
HTemplateInstruction<1> {
On 2013/02/08 12:03:41, danno wrote:
> Could you add a TODO so that we revisit this when we fix FieldLoads and
Stores? 

Done.

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

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.cc#newcode654
src/hydrogen.cc:654: failure_block->Goto(failure_block_);
On 2013/02/08 12:45:49, Jakob wrote:
> This line looks weird. Too bad the edge split form requirement doesn't allow
us
> to get by without it.

Yeah, I know, it also annoys me that we need this fall-through blocks in between
and that merge blocks cannot be proceeded by other control-flow instructions
besides HGoto. But fixing that is outside the scope of this CL.

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.cc#newcode678
src/hydrogen.cc:678: builder_->set_current_block(failure_block_);
On 2013/02/08 12:45:49, Jakob wrote:
> I think this line is not needed if you
> s/builder_->current_block()/failure_block_/ in the line below.

Done.

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/12220074/diff/1/src/hydrogen.h#newcode934
src/hydrogen.h:934: Zone* zone() { return builder_->zone(); }
On 2013/02/08 12:45:49, Jakob wrote:
> nit: methods before properties

Done (also for the other builders below).

[danno, 2013-02-11 14:34:27]

I really like this CL. LGTM with a couple small issues.

https://codereview.chromium.org/12220074/diff/3021/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

https://codereview.chromium.org/12220074/diff/3021/src/arm/deoptimizer-arm.cc...
src/arm/deoptimizer-arm.cc:561: PrintF("    0x%08x: [top + %d] <- 0x%08x ;
function (stub sentinel)\n",
nit: it's actually the stub failure sentinel

https://codereview.chromium.org/12220074/diff/3021/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/12220074/diff/3021/src/hydrogen.h#newcode918
src/hydrogen.h:918: BailoutId id = BailoutId::StubEntry());
I think it's a good idea to not provide a default. If this builder ever gets
used in an inlined stub in a JS function, then it will need the right ast_id,
and with the default construction it's easy to forget to thread it through.

[Michael Starzinger, 2013-02-11 16:31:11]

Comments addressed. Will land later this week or early next week after our
branch.

https://codereview.chromium.org/12220074/diff/3021/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

https://codereview.chromium.org/12220074/diff/3021/src/arm/deoptimizer-arm.cc...
src/arm/deoptimizer-arm.cc:561: PrintF("    0x%08x: [top + %d] <- 0x%08x ;
function (stub sentinel)\n",
On 2013/02/11 14:34:27, danno wrote:
> nit: it's actually the stub failure sentinel

Done.

https://codereview.chromium.org/12220074/diff/3021/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/12220074/diff/3021/src/hydrogen.h#newcode918
src/hydrogen.h:918: BailoutId id = BailoutId::StubEntry());
On 2013/02/11 14:34:27, danno wrote:
> I think it's a good idea to not provide a default. If this builder ever gets
> used in an inlined stub in a JS function, then it will need the right ast_id,
> and with the default construction it's easy to forget to thread it through. 

Done (also for the other builders below).

[Michael Starzinger, 2013-02-26 12:01:57]

Rebased. Landing.

[Michael Starzinger, 2013-02-26 13:08:24]

Committed patchset #6 manually as r13732 (presubmit successful).