lgtm with some nits.
https://codereview.chromium.org/522723002/diff/1/test/mjsunit/es6/math-hyperbolic.js
File test/mjsunit/es6/math-hyperbolic.js (right):
https://codereview.chromium.org/522723002/diff/1/test/mjsunit/es6/math-hyperbolic.js#newcode111
test/mjsunit/es6/math-hyperbolic.js:111:
assertEqualsDelta(74.209948524787, Math.cosh(-5), 1E-12);
Is there any reason to keep the tests in 108-111? If yes, then you
should probably use the correct values and test for equality: cosh(0.5)
= 1.1276259652063807d0, and cosh(5) = 74.20994852478785d0
The same goes for the sinh tests in lines 105-106: sinh(5) =
74.20321057778875d0
If you want some further tests, you could use the properties cosh(x) +
sinh(x) = exp(x) and cosh(x)^2-sinh(x)^2 = 1.
https://codereview.chromium.org/522723002/diff/1/test/mjsunit/es6/math-hyperbolic.js#newcode173
test/mjsunit/es6/math-hyperbolic.js:173: assertEquals(1,
Math.cosh(-Math.pow(2, -55)));
Oops. This is a mistake in original test. If we want to test |x| <
2^-55, we can't use 2^-55. Replace with 2^-56.
https://codereview.chromium.org/522723002/diff/1/third_party/fdlibm/fdlibm.js
File third_party/fdlibm/fdlibm.js (right):
https://codereview.chromium.org/522723002/diff/1/third_party/fdlibm/fdlibm.js#newcode785
third_party/fdlibm/fdlibm.js:785: const KCOSH_OVERFLOW = kMath[52];
You might want to add a comment in fdlibm.cc that kMath[22] is the
overflow threshold for both sinh and cosh. The comment (from the sinh
commit) only says sinh.
https://codereview.chromium.org/522723002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
