Remove ARGUMENTS_VARIABLE and fix crankshaft to properly detect the arguments object and keep it al…

Remove ARGUMENTS_VARIABLE and fix crankshaft to properly detect the arguments object and keep it alive when inlining .apply

BUG=

Committed: https://crrev.com/7f025eb62656ba6a4c36ecc205ebc6acb2d89e04
Cr-Commit-Position: refs/heads/master@{#39670}

[Toon Verwaest, 2016-09-22 20:21:41]

verwaest@chromium.org changed reviewers:
+ adamk@chromium.org, jarin@chromium.org

[Toon Verwaest, 2016-09-22 20:21:42]

Who would've guessed, liveness analysis and arguments objects are broken. ptal
...

[Toon Verwaest, 2016-09-22 20:31:37]

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

[commit-bot: I haz the power, 2016-09-22 20:31:48]

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

[adamk, 2016-09-22 20:56:29]

lgtm for ast, of course I can't speak to the Hydrogen code.

https://codereview.chromium.org/2367483003/diff/40001/src/crankshaft/hydrogen.cc
File src/crankshaft/hydrogen.cc (left):

https://codereview.chromium.org/2367483003/diff/40001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.cc:6937: if (var->is_arguments()) return
Bailout(kAssignmentToArguments);
I think this BailoutReason can be removed as well.

https://codereview.chromium.org/2367483003/diff/40001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.cc:9317: if (current_info()->scope()->arguments() ==
NULL) return false;
Was this check just redundant?

[commit-bot: I haz the power, 2016-09-22 21:03:33]

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

[commit-bot: I haz the power, 2016-09-22 21:03:34]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-09-23 09:13:02]

https://codereview.chromium.org/2367483003/diff/40001/src/crankshaft/hydrogen.cc
File src/crankshaft/hydrogen.cc (left):

https://codereview.chromium.org/2367483003/diff/40001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.cc:6937: if (var->is_arguments()) return
Bailout(kAssignmentToArguments);
On 2016/09/22 20:56:29, adamk wrote:
> I think this BailoutReason can be removed as well.

Done.

https://codereview.chromium.org/2367483003/diff/40001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.cc:9317: if (current_info()->scope()->arguments() ==
NULL) return false;
On 2016/09/22 20:56:29, adamk wrote:
> Was this check just redundant?

Yes, it must be. scope()->arguments() exists if there's a reference; and .apply
inlining only works if the right side is arguments. There's no way to get
arguments without having a reference somewhere. You might have noticed I
replaced it with a dcheck in the case we're going to return true for "can inline
.apply"

[Jarin, 2016-09-23 10:41:37]

lgtm

[Toon Verwaest, 2016-09-23 11:00:30]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-09-23 11:00:30]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2367483003/#ps60001
(title: "Remove bailout reason")

[commit-bot: I haz the power, 2016-09-23 11:00:49]

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

[Toon Verwaest, 2016-09-23 11:18:18]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-09-23 11:18:19]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2367483003/#ps80001
(title: "Rebaseline bytecode expectations")

[commit-bot: I haz the power, 2016-09-23 11:18:31]

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

[Toon Verwaest, 2016-09-23 11:21:09]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-09-23 11:21:09]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2367483003/#ps100001
(title: "Rebaseline more")

[commit-bot: I haz the power, 2016-09-23 11:21:19]

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-09-23 13:21:35]

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

[commit-bot: I haz the power, 2016-09-23 13:21:36]

Try jobs failed on following builders:
  v8_win_compile_dbg on master.tryserver.v8 (JOB_TIMED_OUT, no build URL)

[Toon Verwaest, 2016-09-23 13:50:51]

The CQ bit was checked by verwaest@chromium.org

[commit-bot: I haz the power, 2016-09-23 13:50:57]

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-09-23 14:26:54]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-23 14:27:07]

Description was changed from

==========
Remove ARGUMENTS_VARIABLE and fix crankshaft to properly detect the arguments
object and keep it alive when inlining .apply

BUG=
==========

to

==========
Remove ARGUMENTS_VARIABLE and fix crankshaft to properly detect the arguments
object and keep it alive when inlining .apply

BUG=

Committed: https://crrev.com/7f025eb62656ba6a4c36ecc205ebc6acb2d89e04
Cr-Commit-Position: refs/heads/master@{#39670}
==========

[commit-bot: I haz the power, 2016-09-23 14:27:08]

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