[turbofan] Inline calls to CPP builtins

[turbofan] Inline calls to CPP builtins

BUG=

Committed: https://crrev.com/70a54d46b31696463a0dda9d56cbe130a3beedbd
Cr-Commit-Position: refs/heads/master@{#38758}

[jgruber, 2016-08-18 14:49:16]

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

[commit-bot: I haz the power, 2016-08-18 14:49:23]

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-08-18 15:57:22]

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

[commit-bot: I haz the power, 2016-08-18 15:57:22]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-08-19 06:52:15]

jgruber@chromium.org changed reviewers:
+ jarin@chromium.org

[jgruber, 2016-08-19 06:52:15]

This is the second part of CPP builtin inlining (after yesterday's builtin
descriptor CL). PTAL

[jgruber, 2016-08-19 08:07:19]

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

[commit-bot: I haz the power, 2016-08-19 08:07:24]

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-08-19 08:48:20]

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

[commit-bot: I haz the power, 2016-08-19 08:48:21]

Dry run: This issue passed the CQ dry run.

[Jarin, 2016-08-19 09:01:10]

Nice!.lgtm with nits.

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/js-graph.cc
File src/compiler/js-graph.cc (right):

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/js-graph.c...
src/compiler/js-graph.cc:41: ArgvMode argv_mode, bool builtin_exit_frame) {
The JSGraph class should only have factories that do some caching. Please, add a
TODO for caching the most common constants here (or just implement caching for
the two nodes you need).

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/linkage.cc
File src/compiler/linkage.cc (right):

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/linkage.cc...
src/compiler/linkage.cc:5: #include "src/compiler/linkage.h"
I think you should insert an empty line after this. I am surprise that the
alphabetic order checker has not complained about this.

[jgruber, 2016-08-19 11:15:53]

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

[commit-bot: I haz the power, 2016-08-19 11:16:01]

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

[jgruber, 2016-08-19 11:17:07]

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/js-graph.cc
File src/compiler/js-graph.cc (right):

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/js-graph.c...
src/compiler/js-graph.cc:41: ArgvMode argv_mode, bool builtin_exit_frame) {
On 2016/08/19 09:01:10, Jarin wrote:
> The JSGraph class should only have factories that do some caching. Please, add
a
> TODO for caching the most common constants here (or just implement caching for
> the two nodes you need).

Removed the newly added CEntryStubConstant and added caching for the two most
common cases.

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/linkage.cc
File src/compiler/linkage.cc (right):

https://codereview.chromium.org/2259883002/diff/20001/src/compiler/linkage.cc...
src/compiler/linkage.cc:5: #include "src/compiler/linkage.h"
On 2016/08/19 09:01:10, Jarin wrote:
> I think you should insert an empty line after this. I am surprise that the
> alphabetic order checker has not complained about this.

Done. This was actually an automatic change by git-cl format.

[commit-bot: I haz the power, 2016-08-19 11:43:16]

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

[commit-bot: I haz the power, 2016-08-19 11:43:17]

Dry run: This issue passed the CQ dry run.

[Jarin, 2016-08-19 12:35:32]

lgtm even more.

[jgruber, 2016-08-19 12:55:40]

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

[commit-bot: I haz the power, 2016-08-19 12:55:50]

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

[jgruber, 2016-08-19 13:00:58]

jgruber@chromium.org changed reviewers:
+ verwaest@chromium.org

[jgruber, 2016-08-19 13:00:58]

+Toon for the - now inlined - https://codereview.chromium.org/2246333003.
Related changes are only in builtins.{h,cc}.

[Toon Verwaest, 2016-08-19 13:37:05]

Much nicer, thanks!
lgtm
some final comments

https://codereview.chromium.org/2259883002/diff/60001/src/builtins/builtins.cc
File src/builtins/builtins.cc (right):

https://codereview.chromium.org/2259883002/diff/60001/src/builtins/builtins.c...
src/builtins/builtins.cc:212: return nullptr;
Shouldn't this be unreachable already?

https://codereview.chromium.org/2259883002/diff/60001/src/builtins/builtins.c...
src/builtins/builtins.cc:225: IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN)
Does it make sense to add a macro CPP_BUILTIN(V) BUILTIN_LIST(V, IGNORE_BUILTIN,
...) ? That would make this called cleaner. Equivalent below.

[commit-bot: I haz the power, 2016-08-19 13:45:55]

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

[commit-bot: I haz the power, 2016-08-19 13:45:56]

Dry run: This issue passed the CQ dry run.

[jgruber, 2016-08-19 15:11:17]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-08-19 15:11:17]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org,
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/2259883002/#ps80001
(title: "Address comments")

[commit-bot: I haz the power, 2016-08-19 15:11:22]

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

[jgruber, 2016-08-19 15:11:38]

https://codereview.chromium.org/2259883002/diff/60001/src/builtins/builtins.cc
File src/builtins/builtins.cc (right):

https://codereview.chromium.org/2259883002/diff/60001/src/builtins/builtins.c...
src/builtins/builtins.cc:212: return nullptr;
On 2016/08/19 13:37:05, Toon Verwaest wrote:
> Shouldn't this be unreachable already?

No, case statements are built only for CPP and API type builtins (see
BUILTIN_LIST_C).

https://codereview.chromium.org/2259883002/diff/60001/src/builtins/builtins.c...
src/builtins/builtins.cc:225: IGNORE_BUILTIN, IGNORE_BUILTIN, IGNORE_BUILTIN)
On 2016/08/19 13:37:05, Toon Verwaest wrote:
> Does it make sense to add a macro CPP_BUILTIN(V) BUILTIN_LIST(V,
IGNORE_BUILTIN,
> ...) ? That would make this called cleaner. Equivalent below.

Done.

[commit-bot: I haz the power, 2016-08-19 15:37:20]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-19 15:37:33]

Description was changed from

==========
[turbofan] Inline calls to CPP builtins

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

to

==========
[turbofan] Inline calls to CPP builtins

BUG=

Committed: https://crrev.com/70a54d46b31696463a0dda9d56cbe130a3beedbd
Cr-Commit-Position: refs/heads/master@{#38758}
==========

[commit-bot: I haz the power, 2016-08-19 15:37:34]

Patchset 5 (id:??) landed as
https://crrev.com/70a54d46b31696463a0dda9d56cbe130a3beedbd
Cr-Commit-Position: refs/heads/master@{#38758}