[builtins] Make sure the Math functions and constants agree.

[builtins] Make sure the Math functions and constants agree.

While the EcmaScript specification doesn't define precise values for the
Math constants or the Math functions, we should at least ensure that the
values of the constants and the functions agree, i.e. Math.E should be
exactly the same value as Math.exp(1).

Also make sure that Math.exp(1) returns the expected value; we should
revisit the fdlibm algorithm and figure out why it's wrong in the last
bit.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel
BUG=chromium:626111, v8:3266, v8:3468, v8:3493, v8:5086, v8:5108
R=yangguo@chromium.org

Committed: https://crrev.com/7877ddecdbf226c312a3008195912103c4d8a552
Cr-Commit-Position: refs/heads/master@{#37128}

[Benedikt Meurer, 2016-06-21 05:30:23]

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

[Benedikt Meurer, 2016-06-21 05:30:24]

Hey Yang,

Here's the quickfix for Math.E vs Math.exp(1).
Please take a look.

Thanks,
Benedikt

Raymond: Can you help us with the Math.exp accuracy? Looks like the fdlibm
version gets the rounding wrong in case of Math.exp(1).

[Benedikt Meurer, 2016-06-21 05:30:37]

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

[commit-bot: I haz the power, 2016-06-21 05:30:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2079233005/1

[Benedikt Meurer, 2016-06-21 05:34:39]

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

[commit-bot: I haz the power, 2016-06-21 05:34:44]

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

[Yang, 2016-06-21 05:40:30]

lgtm

https://codereview.chromium.org/2079233005/diff/20001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/src/bootstrapper.cc#new...
src/bootstrapper.cc:1712: static_cast<PropertyAttributes>(DONT_DELETE |
DONT_ENUM | READ_ONLY));
Maybe we can have a helper function to get rid of all this boilerplate.

https://codereview.chromium.org/2079233005/diff/20001/src/js/math.js
File src/js/math.js (right):

https://codereview.chromium.org/2079233005/diff/20001/src/js/math.js#newcode132
src/js/math.js:132: "SQRT2", 1.4142135623730951
What about these three? Can we move them over to the bootstrapper?

[Benedikt Meurer, 2016-06-21 05:42:55]

https://codereview.chromium.org/2079233005/diff/20001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/src/bootstrapper.cc#new...
src/bootstrapper.cc:1712: static_cast<PropertyAttributes>(DONT_DELETE |
DONT_ENUM | READ_ONLY));
Yeah, but let's do that separately. This CL is only to fix the bug.

https://codereview.chromium.org/2079233005/diff/20001/src/js/math.js
File src/js/math.js (right):

https://codereview.chromium.org/2079233005/diff/20001/src/js/math.js#newcode132
src/js/math.js:132: "SQRT2", 1.4142135623730951
Yeah, but let's do that separately. This CL is only to fix the bug.

[Benedikt Meurer, 2016-06-21 05:43:21]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2016-06-21 05:43:27]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-06-21 05:43:34]

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

[commit-bot: I haz the power, 2016-06-21 07:02:08]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-21 07:02:24]

Description was changed from

==========
[builtins] Make sure the Math functions and constants agree.

While the EcmaScript specification doesn't define precise values for the
Math constants or the Math functions, we should at least ensure that the
values of the constants and the functions agree, i.e. Math.E should be
exactly the same value as Math.exp(1).

Also make sure that Math.exp(1) returns the expected value; we should
revisit the fdlibm algorithm and figure out why it's wrong in the last
bit.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel
BUG=chromium:626111,v8:3266,v8:3468,v8:3493,v8:5086,v8:5108
R=yangguo@chromium.org
==========

to

==========
[builtins] Make sure the Math functions and constants agree.

While the EcmaScript specification doesn't define precise values for the
Math constants or the Math functions, we should at least ensure that the
values of the constants and the functions agree, i.e. Math.E should be
exactly the same value as Math.exp(1).

Also make sure that Math.exp(1) returns the expected value; we should
revisit the fdlibm algorithm and figure out why it's wrong in the last
bit.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel
BUG=chromium:626111,v8:3266,v8:3468,v8:3493,v8:5086,v8:5108
R=yangguo@chromium.org

Committed: https://crrev.com/7877ddecdbf226c312a3008195912103c4d8a552
Cr-Commit-Position: refs/heads/master@{#37128}
==========

[commit-bot: I haz the power, 2016-06-21 07:02:25]

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

[Raymond Toy, 2016-06-21 16:20:12]

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc
File src/base/ieee754.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc#new...
src/base/ieee754.cc:1290: if (x == 1.0) return E;
It's probably not possible to fix the algorithm easily to make this round
correctly.  fdlibm does not produce correctly rounded results; it's only
accurate to about 1 ulp.

I think you have implemented some of my tests to ensure that exp(x) is still
monotonic near x = 1.  This is an important property that should be maintained.

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
File test/unittests/base/ieee754-unittest.cc (left):

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
test/unittests/base/ieee754-unittest.cc:134: EXPECT_EQ(2.7182818284590455,
exp(1.0));
Is it possible to use the constant E here, like you did in the implementation of
exp(x)?

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
File test/unittests/base/ieee754-unittest.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
test/unittests/base/ieee754-unittest.cc:133: EXPECT_LE(exp(1.0),
exp(1.0000000000000002));
I remember now what the tests in 132 and 133 are doing.  0.999999... is the
largest FP number < 1.  1.000...2 is the smallest FP number > 1.  These two
tests verify that exp(x) is monotonic at 1.

[Benedikt Meurer, 2016-06-22 03:55:59]

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc
File src/base/ieee754.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc#new...
src/base/ieee754.cc:1290: if (x == 1.0) return E;
Since exp is not performance critical or anything, would it make sense to import
an algorithm that produces correctly rounded results? Is there one w/ managable
complexity?

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
File test/unittests/base/ieee754-unittest.cc (left):

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
test/unittests/base/ieee754-unittest.cc:134: EXPECT_EQ(2.7182818284590455,
exp(1.0));
On 2016/06/21 16:20:12, Raymond Toy wrote:
> Is it possible to use the constant E here, like you did in the implementation
of
> exp(x)?

Done.

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
File test/unittests/base/ieee754-unittest.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/test/unittests/base/iee...
test/unittests/base/ieee754-unittest.cc:133: EXPECT_LE(exp(1.0),
exp(1.0000000000000002));
On 2016/06/21 16:20:12, Raymond Toy wrote:
> I remember now what the tests in 132 and 133 are doing.  0.999999... is the
> largest FP number < 1.  1.000...2 is the smallest FP number > 1.  These two
> tests verify that exp(x) is monotonic at 1.

Acknowledged.

[Raymond Toy, 2016-06-22 22:11:48]

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc
File src/base/ieee754.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc#new...
src/base/ieee754.cc:1290: if (x == 1.0) return E;
On 2016/06/22 03:55:59, Benedikt Meurer wrote:
> Since exp is not performance critical or anything, would it make sense to
import
> an algorithm that produces correctly rounded results? Is there one w/
managable
> complexity?

Yes, this is possible, and something that I would really like to see.  But I
think it's a fairly large undertaking and should probably done all at "once". 
It turns out that some of the correctly rounded algorithms for some functions
are actually faster than fdlibm.

This has been a long-term goal for me and I really need to get back on it!

[Benedikt Meurer, 2016-06-23 03:30:58]

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc
File src/base/ieee754.cc (right):

https://codereview.chromium.org/2079233005/diff/20001/src/base/ieee754.cc#new...
src/base/ieee754.cc:1290: if (x == 1.0) return E;
This would be super awesome and highly appreciated if you could take over the
improvements to the Math functions (again). :-)
We'll hopefully finish the move of the remaining Math functions to this file,
and then it should be easier to experiment and change, since everything has only
a single C++ implementation.