On Wed, 11 Sep 2024 01:59:54 GMT, Joe Darcy <da...@openjdk.org> wrote:
>>> If the test is going to use randomness, then its jtreg tags should include >>> >>> `@key randomness` >>> >>> and it is preferable to use jdk.test.lib.RandomFactory to get and Random >>> object since that handles printing out a key so the random sequence can be >>> replicated if the test fails. >> >> Please see the test updated to use `@key randomness` and` >> jdk.test.lib.RandomFactory` to get and Random object. >> >>> The allowable worst-case error is 2.5 ulp, although at many arguments >>> FDLIBM has a smaller error. >>> For a general Math vs StrictMath test with an allowable 2.5 ulp error, >>> without knowing how accurate FDLIBM is for that function and argument, a >>> large error of approx. 2X the nominal error should be allowed (in case >>> FDLIBM errors in one direction and the Math method errors in the other >>> direction). >>> >> So far the tests haven't failed with error of 2.5ulp. Would it be better to >> make it 5ulp? Please let me know. > > So far, this will be the only intrinsic implementation of tanh. Therefore, at > the moment it is just checking the consistency of the intrinsic > implementation with StrictMath/FDLIBM tanh. If the intrinsic has a ~1 ulp > accuracy, it would be expected to often be within 2.5 ulps of FDLIBM tanh. > However, as written the regression test would not necessarily pass against > any allowable Math.tanh implementation, which is the usual criteria for > java.lang.Math tests that aren't otherwise constrained (such as by being > limited to a given subset of platforms). > > If there was a correctly rounded tanh to compare against, then this style of > testing would be valid. > > Are there any plan to intrinsify sinh or cosh? Hi Joe (@jddarcy), As suggested by Sandhya (@sviswa7), I added ~750 fixed point tests for tanh in `TanhTests.java` using the quad precision tanh implementation in libquadmath library from gcc. Please let me know if this looks good. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20657#discussion_r1759579101
