Implement missing Math logarithm functions from ES6 #3617
Conversation
rerobika
added
ES2015
tests/jerry/es2015/math-log1p.js
Outdated
| assert(isNaN(Math.log1p(NaN))); | ||
| assert(isNaN(Math.log1p(-42))); | ||
| assert(isNaN(Math.log1p(-3.0))); | ||
| assert(Math.log1p(n_zero) === n_zero); |
tests/jerry/es2015/math-expm1.js
Outdated
|
|
||
| assert(isNaN(Math.expm1(NaN))); | ||
| assert(Math.expm1(p_zero) === p_zero); | ||
| assert(Math.expm1(n_zero) === n_zero); |
There was a problem hiding this comment.
Comparing to -0.0 won't work with == or ===, because 0.0 will be equal to -0.0.
Previously I found and fixed a similar bug, you can see
the proper compare in tests/jerry/es2015/math-trunc.js .
There was a problem hiding this comment.
Thanks, you are right, it helped me to find bug in source code ;)
| /* ES6 20.2.2.15 Math.expm1(-0.0) = -0.0 */ | ||
| if ((x == 0.0) && (ecma_number_is_negative (x))) | ||
| { | ||
| break; | ||
| } |
There was a problem hiding this comment.
This check should be inside #ifdef JERRY_LIBM_MATH_H guard,
because exp in libm conforms to the spec without this workaround.
There was a problem hiding this comment.
exp(x) function conforms to specification, but for (0-) "left" and (0+) "right" "0" - but in this case it should distinguish "left" and "right" "1", because we are subtracting 1 from exp(x) result.
I deleted this check, build with tools/build.py --profile=es2015-subset --debug --show-opcodes=ON --jerry-libm=OFF --link-lib=m --clean and unfortunately test fails.
There was a problem hiding this comment.
Ah, I got it. In this case we should use expm1 and log1p instead of exp and log if we build with (GNU) libm, the new tests pass with them.
There was a problem hiding this comment.
I will add native implementation (not reuse of exp()) to provide better accuracy for small numbers (both for expm1 and log1p).
| /* ES6 20.2.2.21 Math.log1p(-0.0) = -0.0 */ | ||
| if ((x == 0.0) && (ecma_number_is_negative (x))) | ||
| { | ||
| break; | ||
| } |
| * software is freely granted, provided that this notice | ||
| * is preserved. | ||
| * | ||
| * @(#)e_log2.c 1.3 95/01/18 |
There was a problem hiding this comment.
Do we know the origin of this file? I can't see e_log2.c here: https://www.netlib.org/fdlibm/
There was a problem hiding this comment.
Source: https://github.com/JuliaMath/openlibm/blob/master/src/e_log2.c
It looks exactly the same as files on netlib.org, but I couldn't find original place with all fdlibm files - something like "SunSoft repository"
|
I added native implementations of expm1() and logp1(). |
|
LGTM (informal) |
jerry-libm/expm1.c
Outdated
| } | ||
| else | ||
| { | ||
| twopk.as_int.hi = 0x3ff00000 + (k << 20); /* 2^k */ |
There was a problem hiding this comment.
cppcheck reported this warning:
jerry-libm/expm1.c:243: portability(shiftNegativeLHS): Shifting a negative value is technically undefined behaviour
k should be casted to unsigned int to make cppcheck happy (and eliminate UB):
((unsigned int)(k) << 20)
jerry-libm/log1p.c
Outdated
| else | ||
| { | ||
| /* log1p(x<-1) = NaN */ | ||
| return (x - x) / (x - x); |
There was a problem hiding this comment.
cppcheck reported this warning:
jerry-libm/log1p.c:130: style(duplicateExpression): Same expression on both sides of '/'.
You can simply use NAN instead of this expression. (defined in jerry-libm-internal.h)
Math.log2, Math.log10, Math.log1p, Math.expm1 C implementation ported from fdlibm Part of Issue jerryscript-project#3568 JerryScript-DCO-1.0-Signed-off-by: Rafal Walczyna r.walczyna@samsung.com
|
Thanks, I rebased the commit and fixed those lines. |
Math.log2, Math.log10, Math.log1p, Math.expm1
C implementation ported from fdlibm
Part of Issue #3568
JerryScript-DCO-1.0-Signed-off-by: Rafal Walczyna r.walczyna@samsung.com