RFR: 8348638: Performance regression in Math.tanh [v5]

[Mohamed Issa]

On Thu, 17 Apr 2025 13:21:59 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Mohamed Issa has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>> 
>>   Add new tanh micro-benchmark that covers different ranges of input values
>
> test/micro/org/openjdk/bench/java/lang/MathBench.java line 70:
> 
>> 68: 
>> 69:     @Param("0")
>> 70:     public double tanhBound1;
> 
> Suggestion:
> 
>     @Param("0", "1", "2", "3")
>     public double tanhRangeIndex;

The latest code update has these changes with the addition of extra curly braces to get everything compiling properly.

> test/micro/org/openjdk/bench/java/lang/MathBench.java line 73:
> 
>> 71: 
>> 72:     @Param("2.7755575615628914E-17")
>> 73:     public double tanhBound2;
> 
> We can declare tanBoundIndex as a Parameter and then select from [hard-coded value ranges](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/FdLibm.java#L3258), which will allow us to execute all the special ranges and NaN value.
> double tanhRangeArray [][] = { 0.0 , 0x1.0P-56}, {0x1.0P-56, 1.0}, {1.0, 22.0}, {22.0,  Double.POSITIVE_INFINITY}}
> double tanhRangeLowerBound = tanhRangeArray[tanhRangeIndex][0];
> double tanhRangeLowerBound = tanhRangeArray[tanhRangeIndex][1];

This is included in the latest code update. I used the largest double value just before +INF because the random number generator doesn't accept -INF or +INF as bounds.

> test/micro/org/openjdk/bench/java/lang/MathBench.java line 549:
> 
>> 547:         for (int i = 0; i < tanhValueCount; i++) {
>> 548:             sum += Math.tanh(tanhPosVector[i]) + Math.tanh(tanhNegVector[i]);
>> 549:         }
> 
> You can remove the noise from the benchmark by assiging the array element to a double field in Invocation level setup and then directly pass that as an argument to tanh. 
> Refer https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/util/ArraysSort.java#L109

The latest code update fixes this.

> test/micro/org/openjdk/bench/java/lang/MathBench.java line 551:
> 
>> 549:         }
>> 550:         return sum;
>> 551:     }
> 
> Please also add benchmark kernels receiving constant inputs i.e. Math.tanh(1.0).
> 
> Current handling for transidental intrinsics creates a stub call node during parsing, which leaves no room to perform constant folding Value transforms. Creating a macro IR which runs through GVN optimization and lazily expands to CallNode should fix it. We already have a similar JBS https://bugs.openjdk.org/browse/JDK-8350831 but its good to add a benchmark for now.

I added new constant inputs to the non-range _tanh_ benchmark.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23889#discussion_r2054956595
PR Review Comment: https://git.openjdk.org/jdk/pull/23889#discussion_r2054958491
PR Review Comment: https://git.openjdk.org/jdk/pull/23889#discussion_r2054954019
PR Review Comment: https://git.openjdk.org/jdk/pull/23889#discussion_r2054954661