RFR: 8338694: x86_64 intrinsic for tanh using libm

[Srinivas Vamsi Parasa]

On Tue, 27 Aug 2024 22:44:43 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>>> This PR doesn't include any additional tests. It is often appropriate to add more regression testing when introducing a new implementation of a method.
>> 
>> Thank You Joe for the suggestion. Will add more tests. (This PR passes the tier-1 tanh tests in the HyperbolicTests.Java)
>
>> > This PR doesn't include any additional tests. It is often appropriate to add more regression testing when introducing a new implementation of a method.
>> 
>> Thank You Joe for the suggestion. Will add more tests. (This PR passes the tier-1 tanh tests in the HyperbolicTests.Java)
> 
> Yes @vamsi-parasa ; running that test is a good backstop and it is written to be applicable to any implementation of {sinh, cosh, tanh} that meet the general quality-of-implementation criteria for java.lang.Math. To be explicit, the WorstCaseTests.java file, and for good measure all the java.lang.Math tests, should also be run too for a change like this.
> 
> For a hypothetical example, if an intrinsic used different polynomials for different ranges of the input, it would be a reasonable regression tests _for that implementation_ to probe around the boundary of the transition between the polynomials to make sure the monotonicity requirements were being met.
> 
> That kind of check could be written to be generally applicable and be suitable for a regression tests in java/lang/Math or could be suitable for a regression test in the HotSpot area. HTH

Hi Joe(@jddarcy)  and Andrew (@theRealAph) ,

Please see the updates below:

> This PR doesn't include any additional tests. It is often appropriate to add more regression testing when introducing a new implementation of a method.
>

Added 1500 regression tests in HyperbolicTests.java which compare the accuracy of the Math.tanh intrinsic by using StrictMath.tanh (which calls FdLibm.Tanh.compute) as a reference. The tests are passing within 2.5 ulps of the expected result. The tests are fairly exhaustive and also cover the boundary transitions.

> Yes @vamsi-parasa ; running that test is a good backstop and it is written to be applicable to any implementation of {sinh, cosh, tanh} that meet the general quality-of-implementation criteria for java.lang.Math. To be explicit, the WorstCaseTests.java file, and for good measure all the java.lang.Math tests, should also be run too for a change like this.
>
Ran the WorstCaseTests.java and all the tests in java.lang.Math and they're passing on my local machine. 

> For a hypothetical example, if an intrinsic used different polynomials for different ranges of the input, it would be a reasonable regression tests _for that implementation_ to probe around the boundary of the transition between the polynomials to make sure the monotonicity requirements were being met.
> 
Added new tests in HyperbolicTests.java which probe around the various boundaries of transition. 1500 testcases and they passed within 2.5ulps of the reference StrictMath.tanh

> That kind of check could be written to be generally applicable and be suitable for a regression tests in java/lang/Math or could be suitable for a regression test in the HotSpot area. HTH

Please let me know if anything more needs to be added.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20657#issuecomment-2322295827