On Mon, 6 Feb 2023 11:34:40 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> Joe Darcy has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Two more spacing fixes.
>> - Implement spacing improvements from code review comments.
>
> test/jdk/java/lang/StrictMath/FdlibmTranslit.java line 874:
>
>> 872: // lx = *( (((*(unsigned*)&one)>>29)) + (unsigned*)&x);
>> 873: // lx = (((*(unsigned*)&one)>>29)) + (unsigned*)&x ;
>> 874: lx = __LO(x);
>
> This aspect of the transliteration is unclear.
> The C code seems to ignore endianness of `double` values, and there's a shift
> by 29 that seems intentional for some reason.
> Are we safe with the `__LO(x)` transliteration?
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.
-------------
PR: https://git.openjdk.org/jdk/pull/12429
