RFR: 8317971: RISC-V: implement copySignF/D and signumF/D intrinsics [v3]

[Fei Yang]

On Wed, 18 Oct 2023 17:35:58 GMT, Ilya Gavrilin <igavrilin at openjdk.org> wrote:

>> Hi all, please review this changes into risc-v floating point copysign and signum intrinsics.
>> CopySign - returns first argument with the sign of second. On risc-v we have `fsgnj.x` instruction, which can implement this intrinsic.
>> Signum - returns input value if it is +/- 0.0 or NaN, otherwise 1.0 with the sign of input value returned.  On risc-v we can use `fclass.x` to specify type of input value and return appropriate value.
>> 
>> Tests:
>> Performance tests on t-head board:
>> With intrinsics:
>> 
>> Benchmark                 (seed)   Mode  Cnt      Score      Error   Units
>> MathBench.copySignDouble       0  thrpt    8  34156.580 ±   76.272  ops/ms
>> MathBench.copySignFloat        0  thrpt    8  34181.731 ±   38.182  ops/ms
>> MathBench.signumDouble         0  thrpt    8  31977.258 ± 1122.327  ops/ms
>> MathBench.signumFloat          0  thrpt    8  31836.852 ±   56.013  ops/ms
>> 
>> Intrinsics turned off (`-XX:+UnlockDiagnosticVMOptions -XX:-UseCopySignIntrinsic -XX:-UseSignumIntrinsic`):
>> 
>> Benchmark                 (seed)   Mode  Cnt      Score      Error   Units
>> MathBench.copySignDouble       0  thrpt    8  31000.996 ±  943.094  ops/ms
>> MathBench.copySignFloat        0  thrpt    8  30678.016 ±   28.087  ops/ms
>> MathBench.signumDouble         0  thrpt    8  25435.010 ± 2047.085  ops/ms
>> MathBench.signumFloat          0  thrpt    8  25257.058 ±   79.175  ops/ms
>> 
>> Regression tests: tier1, hotspot:tier2 on risc-v board.
>> 
>> Also, changed name of one micro test: before we had: `sigNumDouble` and `signumFloat` tests, they does not matches to `signum` or `sigNum`. Now we have similar part: `signum`.
>> Performance tests has been changed a bit, to check intrinsics result better, diff to modify tests:
>> 
>> diff --git a/test/micro/org/openjdk/bench/java/lang/MathBench.java b/test/micro/org/openjdk/bench/java/lang/MathBench.java
>> index 6cd1353907e..0bee25366bf 100644
>> --- a/test/micro/org/openjdk/bench/java/lang/MathBench.java
>> +++ b/test/micro/org/openjdk/bench/java/lang/MathBench.java
>> @@ -143,12 +143,12 @@ public double  ceilDouble() {
>>  
>>      @Benchmark
>>      public double  copySignDouble() {
>> -        return  Math.copySign(double81, doubleNegative12);
>> +        return  Math.copySign(double81, doubleNegative12) + Math.copySign(double81, double2) + Math.copySign(double4Dot1, doubleNegative12);
>>      }
>>  
>>      @Benchmark
>>      public float  copySignFloat() {
>> -        return  Math.copySign(floatNegative99, float1);
>> +        return  ...
>
> Ilya Gavrilin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove some effects and assert

Updated change looks good. Thanks.
JMH data on hifive unmatched for reference:

Before:
Benchmark                 (seed)   Mode  Cnt      Score       Error   Units
MathBench.copySignDouble       0  thrpt    8  79728.042 ?  8211.438  ops/ms
MathBench.copySignFloat        0  thrpt    8  79516.930 ? 13163.477  ops/ms
MathBench.sigNumDouble         0  thrpt    8  58204.403 ?  6795.238  ops/ms
MathBench.signumFloat          0  thrpt    8  57882.056 ?  3635.354  ops/ms

After:
Benchmark                 (seed)   Mode  Cnt       Score       Error   Units
MathBench.copySignDouble       0  thrpt    8  104301.832 ?  7170.917  ops/ms
MathBench.copySignFloat        0  thrpt    8  103008.851 ? 11722.187  ops/ms
MathBench.signumDouble         0  thrpt    8   64465.030 ?  6849.148  ops/ms
MathBench.signumFloat          0  thrpt    8   63987.290 ?  4298.311  ops/ms

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

Marked as reviewed by fyang (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16186#pullrequestreview-1686455232