[turbofan] Use FastCloneShallow[Array|Object]Stub if possible.

[turbofan] Use FastCloneShallow[Array|Object]Stub if possible.

This allows the JSIntrinsicLowering to optimize the cloning of literal
boilerplate objects using either the FastCloneShallowArrayStub or the
FastCloneShallowObjectStub when applicable.

R=mvstanton@chromium.org

Committed: https://crrev.com/d1597b7d22d4bd7790715d486b4ceb404310adad
Cr-Commit-Position: refs/heads/master@{#27974}

[Michael Starzinger, 2015-04-20 17:29:08]

mstarzinger@chromium.org changed reviewers:
+ mvstanton@chromium.org, titzer@chromium.org

[mvstanton, 2015-04-21 12:59:14]

Looks good! I just have a request to use kShallowProperties in one more place.

https://codereview.chromium.org/1097963002/diff/80001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1097963002/diff/80001/src/ast.h#newcode1511
src/ast.h:1511: flags |= kShallowProperties;
Could you also push this flag into
FullCodeGenerator::MustCreateObjectLiteralWithRuntime()?
(in src/full-codegen.cc)

There are three tests that can be eliminated with a look at this nice new
kShallowProperties flag.

https://codereview.chromium.org/1097963002/diff/80001/src/compiler/js-intrins...
File src/compiler/js-intrinsic-lowering.cc (right):

https://codereview.chromium.org/1097963002/diff/80001/src/compiler/js-intrins...
src/compiler/js-intrinsic-lowering.cc:122: node->RemoveInput(4);
nit: a comment would be nice on the RemoveInput...what's being removed?

[mvstanton, 2015-04-21 12:59:27]

And LGTM, too. :)

[Michael Starzinger, 2015-04-21 13:48:36]

https://codereview.chromium.org/1097963002/diff/80001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1097963002/diff/80001/src/ast.h#newcode1511
src/ast.h:1511: flags |= kShallowProperties;
On 2015/04/21 12:59:14, mvstanton wrote:
> Could you also push this flag into
> FullCodeGenerator::MustCreateObjectLiteralWithRuntime()?
> (in src/full-codegen.cc)
> 
> There are three tests that can be eliminated with a look at this nice new
> kShallowProperties flag.

Done.

https://codereview.chromium.org/1097963002/diff/80001/src/compiler/js-intrins...
File src/compiler/js-intrinsic-lowering.cc (right):

https://codereview.chromium.org/1097963002/diff/80001/src/compiler/js-intrins...
src/compiler/js-intrinsic-lowering.cc:122: node->RemoveInput(4);
On 2015/04/21 12:59:14, mvstanton wrote:
> nit: a comment would be nice on the RemoveInput...what's being removed?

Done. Also moved it up to before "InsertInput" so that index makes more sense.

[titzer, 2015-04-21 14:02:26]

lgtm

https://codereview.chromium.org/1097963002/diff/100001/src/compiler/js-intrin...
File src/compiler/js-intrinsic-lowering.cc (right):

https://codereview.chromium.org/1097963002/diff/100001/src/compiler/js-intrin...
src/compiler/js-intrinsic-lowering.cc:134: int length =
mconst.Value().handle()->length() / 2;
Can we have a comment that this thing takes an array of pairs (and thus the /2)?

https://codereview.chromium.org/1097963002/diff/100001/src/full-codegen.cc
File src/full-codegen.cc (right):

https://codereview.chromium.org/1097963002/diff/100001/src/full-codegen.cc#ne...
src/full-codegen.cc:427: expr->ComputeFlags() !=
ObjectLiteral::kShallowProperties ||
Is ComputeFlags() expensive enough to need a cache?

[Michael Starzinger, 2015-04-21 14:35:41]

https://codereview.chromium.org/1097963002/diff/100001/src/compiler/js-intrin...
File src/compiler/js-intrinsic-lowering.cc (right):

https://codereview.chromium.org/1097963002/diff/100001/src/compiler/js-intrin...
src/compiler/js-intrinsic-lowering.cc:134: int length =
mconst.Value().handle()->length() / 2;
On 2015/04/21 14:02:26, titzer wrote:
> Can we have a comment that this thing takes an array of pairs (and thus the
/2)?

Done.

https://codereview.chromium.org/1097963002/diff/100001/src/full-codegen.cc
File src/full-codegen.cc (right):

https://codereview.chromium.org/1097963002/diff/100001/src/full-codegen.cc#ne...
src/full-codegen.cc:427: expr->ComputeFlags() !=
ObjectLiteral::kShallowProperties ||
On 2015/04/21 14:02:26, titzer wrote:
> Is ComputeFlags() expensive enough to need a cache?

Done.

[Michael Starzinger, 2015-04-21 15:21:50]

The CQ bit was checked by mstarzinger@chromium.org

[Michael Starzinger, 2015-04-21 15:21:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvstanton@chromium.org, titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1097963002/#ps120001
(title: "Addressed comments.")

[commit-bot: I haz the power, 2015-04-21 15:22:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1097963002/120001

[commit-bot: I haz the power, 2015-04-21 15:43:35]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-22 04:10:26]

Patchset 7 (id:??) landed as
https://crrev.com/d1597b7d22d4bd7790715d486b4ceb404310adad
Cr-Commit-Position: refs/heads/master@{#27974}