On Fri, 22 Sep 2023 15:27:51 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> Joe Darcy has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix typo found in code review.
>
> test/jdk/java/lang/Math/WorstCaseTests.java line 325:
>
>> 323:
>> 324: // Worst-case observed error
>> 325: {0x1.3f9605aaeb51bp+21, -0x1.9678ee5d64935p-1},
>
> The given expected value seems to contradict the introductory comment.
> The exact value y meets `-0x1.9678ee5d64935p-1` < y <
> `-0x1.9678ee5d64934p-1`, and is closer to `-0x1.9678ee5d64935p-1`.
> Thus, the rounded value of y is `-0x1.9678ee5d64935p-1`, but its truncated
> value is `-0x1.9678ee5d64934p-1`.
> This should be the expected value, but then the test fails.
>
> I don't think that the test logic can support errors > 1 ulp, as is the case
> here.
> Perhaps, rather than a single expected value, there should be explicit lower
> and upper bounds.
For FDLIBM tan, the stated error in the Math.tan spec is 1 ulp and the FDLIBM
sources say tan is "nearly rounded," which could reasonably be interpreted as
meaning within 1 ulp. However, the reported error by the paper is 1.02 ulps.
As you note here, the current testing methodology can only really deal with at
most a 1 ulp error, assuming the expected value is at the lower endpoint of the
range.
To avoid any lurking errors in the FDLIBM port to Java, I generated the
expected numbers running jshell on JDK 20, which uses the mostly C-based
FDLIBM. For a number of cases I had to decrement the expected value for the
test to pass against Math and StrictMath. The decremented value and its
successor may or may not bracket the exact value; I didn't verify that.
In other words, there may be other bugs in one or both math libraries the test
is detecting.
So far, I've only tried running the test locally, but this would need a
cross-platform run being before pushed to cover all the intrinsics that may be
in use on the full set of supported platforms.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15879#discussion_r1334831427
