Fix usage of literal cloning for large double arrays.

Fix usage of literal cloning for large double arrays.

This fixes a corner case where the {FastCloneShallowArrayStub} was used
for literals that are backed by a double backing store and would exceed
limits for new-space allocations on 32-bit architectures. The stub in
question does not support such literals, callers must use the runtime.
Note that this fix is for Ignition as well as FullCodeGenerator.

R=rmcilroy@chromium.org
TEST=mjsunit/regress/regress-crbug-672792
BUG=chromium:672792
Review-Url: https://codereview.chromium.org/2570843002
Cr-Commit-Position: refs/heads/master@{#41713}
Committed: https://chromium.googlesource.com/v8/v8/+/6c620e531268ca6dcdb858a75cc75c68e6b2a44b

[Michael Starzinger, 2016-12-13 13:26:00]

The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 13:26:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael Starzinger, 2016-12-13 13:26:17]

mstarzinger@chromium.org changed reviewers:
+ cbruni@chromium.org

[Michael Starzinger, 2016-12-13 13:26:17]

Ross: PTAL at interpreter (and full-codegen) changes.
Camillo: PTAL at code stub interface changes.

[commit-bot: I haz the power, 2016-12-13 13:52:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 13:52:17]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-12-13 14:48:18]

The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 14:48:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael Starzinger, 2016-12-13 14:49:37]

mstarzinger@chromium.org changed reviewers:
+ bmeurer@chromium.org

[Michael Starzinger, 2016-12-13 14:49:37]

Benedikt: I extended the test coverage and the fix to the optimizing compilers.
PTAL at that part.

[Benedikt Meurer, 2016-12-13 14:54:09]

Very nice. LGTM!

[Michael Starzinger, 2016-12-13 15:02:36]

The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 15:02:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 15:25:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 15:25:56]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-12-13 16:30:26]

LGTM, thanks!

[Camillo Bruni, 2016-12-14 10:59:08]

LGTM with comment

https://codereview.chromium.org/2570843002/diff/40001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/2570843002/diff/40001/src/code-stubs.cc#newco...
src/code-stubs.cc:2730: expr->values()->length() <= kMaximumClonedElements;
How about putting this on the ArrayLiteral node instead?

[Michael Starzinger, 2016-12-14 14:01:03]

Thanks! Comments addressed. Will land soonish.

https://codereview.chromium.org/2570843002/diff/40001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/2570843002/diff/40001/src/code-stubs.cc#newco...
src/code-stubs.cc:2730: expr->values()->length() <= kMaximumClonedElements;
On 2016/12/14 10:59:08, Camillo Bruni wrote:
> How about putting this on the ArrayLiteral node instead?

Done. Good idea, applied it to ArrayLiteral as well as to ObjectLiteral.

[Camillo Bruni, 2016-12-15 08:41:25]

LGTM, thanks!

[Michael Starzinger, 2016-12-15 08:53:24]

The CQ bit was checked by mstarzinger@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-15 08:53:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 09:20:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-15 09:20:48]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-12-15 09:45:46]

The CQ bit was checked by mstarzinger@chromium.org

[Michael Starzinger, 2016-12-15 09:45:47]

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

[commit-bot: I haz the power, 2016-12-15 09:45:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 10:29:48]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481795146952330,
"parent_rev": "b02fd88d75c44c0e88fd083076512c82a5f78adb", "commit_rev":
"6c620e531268ca6dcdb858a75cc75c68e6b2a44b"}

[commit-bot: I haz the power, 2016-12-15 10:29:54]

Description was changed from

==========
Fix usage of literal cloning for large double arrays.

This fixes a corner case where the {FastCloneShallowArrayStub} was used
for literals that are backed by a double backing store and would exceed
limits for new-space allocations on 32-bit architectures. The stub in
question does not support such literals, callers must use the runtime.
Note that this fix is for Ignition as well as FullCodeGenerator.

R=rmcilroy@chromium.org
TEST=mjsunit/regress/regress-crbug-672792
BUG=chromium:672792
==========

to

==========
Fix usage of literal cloning for large double arrays.

This fixes a corner case where the {FastCloneShallowArrayStub} was used
for literals that are backed by a double backing store and would exceed
limits for new-space allocations on 32-bit architectures. The stub in
question does not support such literals, callers must use the runtime.
Note that this fix is for Ignition as well as FullCodeGenerator.

R=rmcilroy@chromium.org
TEST=mjsunit/regress/regress-crbug-672792
BUG=chromium:672792

Review-Url: https://codereview.chromium.org/2570843002
Cr-Commit-Position: refs/heads/master@{#41713}
Committed:
https://chromium.googlesource.com/v8/v8/+/6c620e531268ca6dcdb858a75cc75c68e6b...
==========

[commit-bot: I haz the power, 2016-12-15 10:29:55]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/6c620e531268ca6dcdb858a75cc75c68e6b...