Allow FastCloneShallowObjectStub to use AllocationMementos.

Allow FastCloneShallowObjectStub to use AllocationMementos.
(currently turned off)

BUG=
R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=17154

[mvstanton, 2013-10-10 12:43:45]

Hi Michael, here is the CL we discussed earlier in the week, PTAL. thx!

[Michael Starzinger, 2013-10-10 13:27:39]

Just one real comment. Otherwise looking good.

https://codereview.chromium.org/26827002/diff/1/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/26827002/diff/1/src/code-stubs.h#newcode676
src/code-stubs.h:676: FastCloneShallowObjectStub(AllocationSiteMode
allocation_site_mode,
As discussed offline: I don't think it makes much sense to keep the
allocation_site as part of the minor key because it's value is statically
chosen. I don't (yet) see a use-case where we need more that one stub per
length. Let's drop that and decide whether to tack allocations when we compile
the stub.

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

https://codereview.chromium.org/26827002/diff/1/src/hydrogen.cc#newcode4341
src/hydrogen.cc:4341: Handle<Object> boilerplate;
nit: The type of "boilerplate" should be Handle<JSObject>.

[mvstanton, 2013-10-10 14:09:04]

Thx for the comments, FLAG_allocation_site_pretenuring is used instead of a new
minor key.

https://codereview.chromium.org/26827002/diff/1/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/26827002/diff/1/src/code-stubs.h#newcode676
src/code-stubs.h:676: FastCloneShallowObjectStub(AllocationSiteMode
allocation_site_mode,
On 2013/10/10 13:27:39, Michael Starzinger wrote:
> As discussed offline: I don't think it makes much sense to keep the
> allocation_site as part of the minor key because it's value is statically
> chosen. I don't (yet) see a use-case where we need more that one stub per
> length. Let's drop that and decide whether to tack allocations when we compile
> the stub.

Good comment, I added FLAG_allocation_site_pretenuring to express this,
currently turned off.

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

https://codereview.chromium.org/26827002/diff/1/src/hydrogen.cc#newcode4341
src/hydrogen.cc:4341: Handle<Object> boilerplate;
On 2013/10/10 13:27:39, Michael Starzinger wrote:
> nit: The type of "boilerplate" should be Handle<JSObject>.

Done.

[Michael Starzinger, 2013-10-11 07:49:36]

LGTM with a few minor nits.

https://codereview.chromium.org/26827002/diff/6001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/26827002/diff/6001/src/code-stubs.h#newcode694
src/code-stubs.h:694: STATIC_ASSERT(kMaximumClonedProperties < 16);
nit: Assert is obsolete now.

https://codereview.chromium.org/26827002/diff/6001/src/code-stubs.h#newcode697
src/code-stubs.h:697: int NotMissMinorKey() {
nit: Unnecessary white-space change, still fits into one line.

https://codereview.chromium.org/26827002/diff/6001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/26827002/diff/6001/src/hydrogen.cc#newcode4344
src/hydrogen.cc:4344: ASSERT(literals_cell->IsAllocationSite());
nit: Assert is not needed, the cast below will do one for you.

https://codereview.chromium.org/26827002/diff/6001/src/hydrogen.cc#newcode4351
src/hydrogen.cc:4351: IsFastLiteral(boilerplate,
nit: Second predicate should fit into one line now.

[mvstanton, 2013-10-11 09:23:47]

Right on, thanks very much.

https://codereview.chromium.org/26827002/diff/6001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/26827002/diff/6001/src/code-stubs.h#newcode694
src/code-stubs.h:694: STATIC_ASSERT(kMaximumClonedProperties < 16);
On 2013/10/11 07:49:37, Michael Starzinger wrote:
> nit: Assert is obsolete now.

Done.

https://codereview.chromium.org/26827002/diff/6001/src/code-stubs.h#newcode697
src/code-stubs.h:697: int NotMissMinorKey() {
On 2013/10/11 07:49:37, Michael Starzinger wrote:
> nit: Unnecessary white-space change, still fits into one line.

Done.

https://codereview.chromium.org/26827002/diff/6001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/26827002/diff/6001/src/hydrogen.cc#newcode4344
src/hydrogen.cc:4344: ASSERT(literals_cell->IsAllocationSite());
On 2013/10/11 07:49:37, Michael Starzinger wrote:
> nit: Assert is not needed, the cast below will do one for you.

Done.

https://codereview.chromium.org/26827002/diff/6001/src/hydrogen.cc#newcode4351
src/hydrogen.cc:4351: IsFastLiteral(boilerplate,
On 2013/10/11 07:49:37, Michael Starzinger wrote:
> nit: Second predicate should fit into one line now.

Done.

[mvstanton, 2013-10-11 09:25:21]

Committed patchset #3 manually as r17154 (presubmit successful).