On Mon, 6 Feb 2023 22:03:17 GMT, Joe Darcy <da...@openjdk.org> wrote:
>> Yes, it gave me pause when doing the transliteration. >> >> Unpacking the code a bit, >> >> >> /* |x| in [log(maxdouble), overflowthresold] */ >> // lx = *( (((*(unsigned*)&one)>>29)) + (unsigned*)&x); >> lx = __LO(x); >> if (ix<0x408633CE || >> ((ix==0x408633ce)&&(Long.compareUnsigned(lx, 0x8fb9f87d) <= 0 >> ))) {...} >> >> >> Assuming the comment accurate describe the intention of the code, >> identifying values between log(maxdouble) and the overflow threshold. Those >> particular values are as decimal fp, hex hp, and long bits: >> >> 709.782712893384 0x1.62e42fefa39efp9 40862e42__fefa39ef >> 710.4758600739439 0x1.633ce8fb9f87dp9 408633ce_8fb9f87d >> >> so the conditional is checking if the high word (ix) is for something less >> than the low end of the range OR if the high word matches the high word for >> the high end of the range AND low bits are for less than the high end of the >> range. Therefore, I think taking __LO(x) is the correct semantics here. >> >> FWIW, the ExhaustingTest of running all the float values against >> sinh/cosh/tanh in the transliteration port passes when using JDK 20 baseline >> as a reference. > > PS One possible future refactoring to the code in > > src/java.base/share/classes/java/lang/FdLibm.java > > would be to replace such integer-based range checks with floating-point based > range checks, in this case something like: > > > if (absX <= 0x1.633ce8fb9f87dp9) { // 710.4758600739439 > ...} Let's discuss this C fragment. `*( (((*(unsigned*)&one)>>29)) + (unsigned*)&x)` On a big-endian platform, `*(unsigned*)&one` returns `__HI(one) = 0x3ff0_0000`. It is then shifted right by 29, giving `1`. And `(unsigned*)&x` is the _address_ of the high word of `x`. The sum with `1` thus gives the address of the low word of `x`, which is dereferenced by the leftmost `*`, resulting in `__LO(x)`. On a little-endian platform, `*(unsigned*)&one` returns `__LO(one) = 0x0`. It is then shifted right by 29, giving `0`. And `(unsigned*)&x` is the _address_ of the low word of `x`. The sum with `0` thus gives the address of the low word of `x`, which is dereferenced by the leftmost `*`, resulting in `__LO(x)` as well. In other words, this is a complicated way to say `__LO(x)`, right? I wonder why the author came up with this creative code... Would it make sense to add this discussion as a block comment? ------------- PR: https://git.openjdk.org/jdk/pull/12429