Implement Math.log2 via ported extract from fdlibm.

Implement Math.log2 via ported extract from fdlibm.

Adapted from Raymond Toy's (rtoy@chromium.org) port, extracted from fdlibm's pow implementation.

R=rtoy@chromium.org
BUG=v8:3579
LOG=N

[Raymond Toy, 2014-12-09 20:37:38]

https://codereview.chromium.org/786823003/diff/1/src/third_party/fdlibm/fdlib...
File src/third_party/fdlibm/fdlibm.js (right):

https://codereview.chromium.org/786823003/diff/1/src/third_party/fdlibm/fdlib...
src/third_party/fdlibm/fdlibm.js:931: n -= 54;
Why 54? The original code multiplied by 2^53.  I suspect this is probably ok,
though, but would prefer 53 since that's what the fdlibm pow code did.

https://codereview.chromium.org/786823003/diff/1/src/third_party/fdlibm/fdlib...
src/third_party/fdlibm/fdlibm.js:966: var t_h = %_ConstructDouble(((ix >> 1) |
0x20000000) + th_offset, 0);
If the comment on line 965 is correct, and I believe it is, then line 966 can be
replaced by

var t_h = %_ConstructDouble(%_DoubleHi(ax + bp), 0)

I ran the tests and this passes.  This is certainly a lot clearer than the
current bit-banging code, and th_offset can be removed.

[Raymond Toy, 2014-12-09 20:45:26]

Oh, in case any one has to do bisects in the future, perhaps the description
should say this is extracted from the pow() implementation in fdlibm.

[Yang, 2014-12-10 10:01:41]

I added a half-sentence to the description to say that it originates from
fdlibm's pow implementation.

I was using TWO54 because it was defined further above. But I realized that for
both 2^53 and 2^54 the double representation has an empty lower half, so loading
from an array is just as fast as having the constant inline. So I removed TWO54
from the array.

https://codereview.chromium.org/786823003/diff/1/src/third_party/fdlibm/fdlib...
File src/third_party/fdlibm/fdlibm.js (right):

https://codereview.chromium.org/786823003/diff/1/src/third_party/fdlibm/fdlib...
src/third_party/fdlibm/fdlibm.js:931: n -= 54;
On 2014/12/09 20:37:38, Raymond Toy wrote:
> Why 54? The original code multiplied by 2^53.  I suspect this is probably ok,
> though, but would prefer 53 since that's what the fdlibm pow code did.

Done.

https://codereview.chromium.org/786823003/diff/1/src/third_party/fdlibm/fdlib...
src/third_party/fdlibm/fdlibm.js:966: var t_h = %_ConstructDouble(((ix >> 1) |
0x20000000) + th_offset, 0);
On 2014/12/09 20:37:38, Raymond Toy wrote:
> If the comment on line 965 is correct, and I believe it is, then line 966 can
be
> replaced by
> 
> var t_h = %_ConstructDouble(%_DoubleHi(ax + bp), 0)
> 
> I ran the tests and this passes.  This is certainly a lot clearer than the
> current bit-banging code, and th_offset can be removed.

Done.

[Raymond Toy, 2014-12-10 17:15:53]

lgtm.

Couple of minor comments that you can ignore if you want.

Thanks for implementing this.

https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
File src/third_party/fdlibm/fdlibm.cc (right):

https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
src/third_party/fdlibm/fdlibm.cc:85: 5.99999999999994648725e-01,   // L1       
55  coefficients for log2
Nit: Certainly would be nice to add (or move) a comment to a new line before
line 85 saying these are for the log2 implementation.  Same goes for the other
tables above now that there are so many entries.

https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
File src/third_party/fdlibm/fdlibm.js (right):

https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
src/third_party/fdlibm/fdlibm.js:614: const KEXPM1_OVERFLOW = kMath[44];
I think it would be nice to have KEXPM1_TABLE_OFFSET (or something) that gives
the starting offset in the kMath table for all of the expm1 releated entries. 
Then you wouldn't have to change every one of these offsets here and below.

[Yang, 2014-12-10 20:42:46]

On 2014/12/10 17:15:53, Raymond Toy wrote:
> lgtm.
> 
> Couple of minor comments that you can ignore if you want.
> 
> Thanks for implementing this.
> 
>
https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
> File src/third_party/fdlibm/fdlibm.cc (right):
> 
>
https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
> src/third_party/fdlibm/fdlibm.cc:85: 5.99999999999994648725e-01,   // L1      

> 55  coefficients for log2
> Nit: Certainly would be nice to add (or move) a comment to a new line before
> line 85 saying these are for the log2 implementation.  Same goes for the other
> tables above now that there are so many entries.
> 
>
https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
> File src/third_party/fdlibm/fdlibm.js (right):
> 
>
https://codereview.chromium.org/786823003/diff/20001/src/third_party/fdlibm/f...
> src/third_party/fdlibm/fdlibm.js:614: const KEXPM1_OVERFLOW = kMath[44];
> I think it would be nice to have KEXPM1_TABLE_OFFSET (or something) that gives
> the starting offset in the kMath table for all of the expm1 releated entries. 
> Then you wouldn't have to change every one of these offsets here and below.

I'm out for vacation, and don't have the means to do those comment tweaks. I'll
do those after new years.

[Yang, 2014-12-10 20:42:51]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2014-12-10 20:43:27]

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

[commit-bot: I haz the power, 2014-12-10 20:43:28]

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

[commit-bot: I haz the power, 2014-12-10 20:43:29]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Yang, 2014-12-10 20:51:03]

yangguo@chromium.org changed reviewers:
+ svenpanne@chromium.org

[Yang, 2014-12-10 20:51:03]

Sven, can you please rubberstamp and check CQ? Thanks!

[Jakob Kummerow, 2014-12-11 10:53:42]

jkummerow@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Jakob Kummerow, 2014-12-11 10:53:42]

rubberstamp LGTM

[Yang, 2014-12-11 10:54:50]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2014-12-11 10:56:11]

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

[commit-bot: I haz the power, 2014-12-11 11:23:33]

Committed patchset #2 (id:20001)