[builtins] Add support for JS builtins written in TurboFan.

[builtins] Add support for JS builtins written in TurboFan.

This CL adds support for builtins with JavaScript linkage written using
the TurboFan CodeStubAssembler, but with a JSCall descriptor (which was
already supported thanks to a previous patch by Ben Smith). As a first
example, we convert the Math.sqrt builtin and thereby get rid of the
%_MathSqrt intrinsic, which causes trouble for the representation
selection pass in the JavaScript pipeline.

R=mstarzinger@chromium.org

Committed: https://crrev.com/43fe7d6854ef4a831cdaaf83d2897ac1ca54c066
Cr-Commit-Position: refs/heads/master@{#34989}

[Benedikt Meurer, 2016-03-22 12:01:26]

Hey Michi,

Here's the CL we talked about. Mostly straight-forward.
Please take a look.

Thanks,
Benedikt

[Michael Starzinger, 2016-03-22 12:54:32]

LGTM.

https://codereview.chromium.org/1824993002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1824993002/diff/1/src/builtins.cc#newcode2059
src/builtins.cc:2059: assembler->WordEqual(x_map,
assembler->HeapNumberMapConstant()),
Note that checking like this will not cover mutable heap numbers. Would it be
possible to check the instance type instead? I think this would be consistent
with other places as well. I saw a couple of other places in code-stubs.cc where
we seem to have the same issue.

On the other hand it might not even matter in practice in these particular
cases, because mutable heap numbers should not be handed out to other functions
like that. Just some food for thought.

https://codereview.chromium.org/1824993002/diff/1/src/builtins.cc#newcode4594
src/builtins.cc:4594: #define DEFINE_BUILTIN_ACCESSOR_T(name, argc)             
                   \
All four macros seem to do the same. Is there a reason we need four different
versions of them or is this just for symmetry?

https://codereview.chromium.org/1824993002/diff/1/src/builtins.h
File src/builtins.h (right):

https://codereview.chromium.org/1824993002/diff/1/src/builtins.h#newcode309
src/builtins.h:309: // Define list of builtins implemented in TurboFan assembly.
nit: Let's drop the "assembly", I think it's confusing.

https://codereview.chromium.org/1824993002/diff/1/src/compiler/js-builtin-red...
File src/compiler/js-builtin-reducer.cc (right):

https://codereview.chromium.org/1824993002/diff/1/src/compiler/js-builtin-red...
src/compiler/js-builtin-reducer.cc:171: // ES6 section 20.2.2.28 Math.sqrt ( x )
nit: s/20.2.2.28/20.2.2.32/ ... or whatever other format we use these days to
reference sections in the spec. I list track. :)

https://codereview.chromium.org/1824993002/diff/1/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1824993002/diff/1/src/frames.cc#newcode460
src/frames.cc:460: if (marker->IsSmi()) break;
Please add a one-line comment explaining that this is due to builtins being
compiled with optimizing compiler.

[Benedikt Meurer, 2016-03-22 13:01:18]

Thanks.

https://codereview.chromium.org/1824993002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1824993002/diff/1/src/builtins.cc#newcode2059
src/builtins.cc:2059: assembler->WordEqual(x_map,
assembler->HeapNumberMapConstant()),
As discussed offline, this is in line with what we do for the other code stubs
and builtins.

https://codereview.chromium.org/1824993002/diff/1/src/builtins.cc#newcode4594
src/builtins.cc:4594: #define DEFINE_BUILTIN_ACCESSOR_T(name, argc)             
                   \
Different parameters and for symmetry.

https://codereview.chromium.org/1824993002/diff/1/src/builtins.h
File src/builtins.h (right):

https://codereview.chromium.org/1824993002/diff/1/src/builtins.h#newcode309
src/builtins.h:309: // Define list of builtins implemented in TurboFan assembly.
On 2016/03/22 12:54:32, Michael Starzinger wrote:
> nit: Let's drop the "assembly", I think it's confusing.

Done.

https://codereview.chromium.org/1824993002/diff/1/src/compiler/js-builtin-red...
File src/compiler/js-builtin-reducer.cc (right):

https://codereview.chromium.org/1824993002/diff/1/src/compiler/js-builtin-red...
src/compiler/js-builtin-reducer.cc:171: // ES6 section 20.2.2.28 Math.sqrt ( x )
On 2016/03/22 12:54:32, Michael Starzinger wrote:
> nit: s/20.2.2.28/20.2.2.32/ ... or whatever other format we use these days to
> reference sections in the spec. I list track. :)

Done.

https://codereview.chromium.org/1824993002/diff/1/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1824993002/diff/1/src/frames.cc#newcode460
src/frames.cc:460: if (marker->IsSmi()) break;
On 2016/03/22 12:54:32, Michael Starzinger wrote:
> Please add a one-line comment explaining that this is due to builtins being
> compiled with optimizing compiler.

Done.

[Benedikt Meurer, 2016-03-22 13:02:06]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2016-03-22 13:02:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1824993002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-03-22 13:02:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1824993002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1824993002/20001

[commit-bot: I haz the power, 2016-03-22 13:25:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-22 13:25:59]

Description was changed from

==========
[builtins] Add support for JS builtins written in TurboFan.

This CL adds support for builtins with JavaScript linkage written using
the TurboFan CodeStubAssembler, but with a JSCall descriptor (which was
already supported thanks to a previous patch by Ben Smith). As a first
example, we convert the Math.sqrt builtin and thereby get rid of the
%_MathSqrt intrinsic, which causes trouble for the representation
selection pass in the JavaScript pipeline.

R=mstarzinger@chromium.org
==========

to

==========
[builtins] Add support for JS builtins written in TurboFan.

This CL adds support for builtins with JavaScript linkage written using
the TurboFan CodeStubAssembler, but with a JSCall descriptor (which was
already supported thanks to a previous patch by Ben Smith). As a first
example, we convert the Math.sqrt builtin and thereby get rid of the
%_MathSqrt intrinsic, which causes trouble for the representation
selection pass in the JavaScript pipeline.

R=mstarzinger@chromium.org

Committed: https://crrev.com/43fe7d6854ef4a831cdaaf83d2897ac1ca54c066
Cr-Commit-Position: refs/heads/master@{#34989}
==========

[commit-bot: I haz the power, 2016-03-22 13:26:00]

Patchset 2 (id:??) landed as
https://crrev.com/43fe7d6854ef4a831cdaaf83d2897ac1ca54c066
Cr-Commit-Position: refs/heads/master@{#34989}