RFR: 8307446: RISC-V: Improve performance of floating point to integer conversion

[Xiaolin Zheng]

On Thu, 4 May 2023 14:12:19 GMT, Vladimir Kempik <vkempik at openjdk.org> wrote:

>> Hi, 
>> 
>> can I have reviews for this change that improves the performance of floating point to integer conversion?
>> 
>> Currently, risc-v port converts floating point to integer using `FCVT_SAFE` in macroAssembler_riscv.cpp.
>> 
>> The main issue here is Java spec returns 0 when the floating point number is NaN [1].
>> But for RISC-V ISA, instructions converting a floating-point value to an integer value (`FCVT.W.S`/`FCVT.L.S`/`FCVT.W.D`/`FCVT.L.D`) return the largest/smallest value when the floating point number is NaN [2].
>> That requires additional logic to handle the case when the src of conversion is NaN, as the following code did:
>> 
>> 
>> #define FCVT_SAFE(FLOATCVT, FLOATEQ)                                                             \
>> void MacroAssembler:: FLOATCVT##_safe(Register dst, FloatRegister src, Register tmp) {           \
>>   Label L_Okay;                                                                                  \
>>   fscsr(zr);                                                                                     \
>>   FLOATCVT(dst, src);                                                                            \
>>   frcsr(tmp);                                                                                    \
>>   andi(tmp, tmp, 0x1E);                                                                          \
>>   beqz(tmp, L_Okay);                                                                             \
>>   FLOATEQ(tmp, src, src);                                                                        \
>>   bnez(tmp, L_Okay);                                                                             \
>>   mv(dst, zr);                                                                                   \
>>   bind(L_Okay);                                                                                  \
>> }
>> 
>> FCVT_SAFE(fcvt_w_s, feq_s)
>> FCVT_SAFE(fcvt_l_s, feq_s)
>> FCVT_SAFE(fcvt_w_d, feq_d)
>> FCVT_SAFE(fcvt_l_d, feq_d)
>> 
>> 
>> We can improve the logic of NaN checking with the `fclass` instruction just as [JDK-8297359](https://bugs.openjdk.org/browse/JDK-8297359) did.
>> 
>> Here are the JMH results, we can got an obvious improvement for `f2i`/`f2l`/`d2i`/`d2l` conversions (source: [FloatConversion.java](https://gist.github.com/feilongjiang/b59bdd8db8460242bafac4a2ee6c2e06#file-floatconversion-java), tests on HiFive Unmatched board):
>> 
>> 
>> Before:
>> 
>> Benchmark                     (size)   Mode  Cnt   Score   Error   Units
>> FloatConversion.doubleToInt     2048  thrpt   15  29.311 ± 0.063  ops/ms
>> FloatConversion.doubleToLong    2048  thrpt   15  29.914 ± 0.023  ops/ms
>> FloatConversion.floatToInt      2048  thrpt   15  30.530 ± 0.011  ops/ms
>> FloatConversion.floatToLong     2048  thrpt   15  29.657 ± 0.021  ops/ms
>> 
>> Benchmark                     (size)   Mode  Cnt   Score   Error   Units
>> FloatConversion.doubleToInt     2048  thrpt   15  29.335 ± 0.014  ops/ms
>> FloatConversion.doubleToLong    2048  thrpt   15  29.919 ± 0.022  ops/ms
>> FloatConversion.floatToInt      2048  thrpt   15  30.523 ± 0.026  ops/ms
>> FloatConversion.floatToLong     2048  thrpt   15  29.670 ± 0.011  ops/ms
>> 
>> Benchmark                     (size)   Mode  Cnt   Score   Error   Units
>> FloatConversion.doubleToInt     2048  thrpt   15  29.344 ± 0.017  ops/ms
>> FloatConversion.doubleToLong    2048  thrpt   15  29.908 ± 0.060  ops/ms
>> FloatConversion.floatToInt      2048  thrpt   15  30.539 ± 0.009  ops/ms
>> FloatConversion.floatToLong     2048  thrpt   15  29.676 ± 0.013  ops/ms
>> 
>> ---------------------------------------------------------------------------
>> 
>> After:
>> 
>> Benchmark                     (size)   Mode  Cnt   Score   Error   Units
>> FloatConversion.doubleToInt     2048  thrpt   15  65.903 ± 0.385  ops/ms
>> FloatConversion.doubleToLong    2048  thrpt   15  66.491 ± 0.057  ops/ms
>> FloatConversion.floatToInt      2048  thrpt   15  68.045 ± 0.061  ops/ms
>> FloatConversion.floatToLong     2048  thrpt   15  68.441 ± 0.077  ops/ms
>> 
>> Benchmark                     (size)   Mode  Cnt   Score   Error   Units
>> FloatConversion.doubleToInt     2048  thrpt   15  66.015 ± 0.059  ops/ms
>> FloatConversion.doubleToLong    2048  thrpt   15  66.511 ± 0.059  ops/ms
>> FloatConversion.floatToInt      2048  thrpt   15  68.077 ± 0.051  ops/ms
>> FloatConversion.floatToLong     2048  thrpt   15  68.467 ± 0.076  ops/ms
>> 
>> Benchmark                     (size)   Mode  Cnt   Score   Error   Units
>> FloatConversion.doubleToInt     2048  thrpt   15  65.999 ± 0.067  ops/ms
>> FloatConversion.doubleToLong    2048  thrpt   15  66.454 ± 0.090  ops/ms
>> FloatConversion.floatToInt      2048  thrpt   15  68.048 ± 0.055  ops/ms
>> FloatConversion.floatToLong     2048  thrpt   15  68.467 ± 0.054  ops/ms
>> 
>> 
>> 1. https://docs.oracle.com/javase/specs/jls/se20/html/jls-5.html#jls-5.1.3
>> 2. https://github.com/riscv/riscv-isa-manual/blob/63aeaada9b2fee7ca15e5c6b6a28f3b710fb7e58/src/f-st-ext.adoc?plain=1#L365-L386
>> 
>> ## Testing:
>> - [x] tier1~3 on Unmatched board (release build)
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 4075:
> 
>> 4073:   bind(do_convert);                                                                       \
>> 4074:   FLOATCVT(dst, src);                                                                     \
>> 4075:   bind(done);                                                                             \
> 
> what about reducing the branching?
> 
> e.g.
> 
> mv (dst, zr); //pretty cheap anyway
> fclass(..);
> andi(tmp, tmp, 0b1100000000);
> bnez(tmp, done);
>   FLOATCVT(dst, src);                                                                    
> bind(done);

After applying this results look better:


Benchmark                     (size)   Mode  Cnt    Score   Error   Units
FloatConversion.doubleToInt     2048  thrpt   15  286.038 ± 1.472  ops/ms
FloatConversion.doubleToLong    2048  thrpt   15  289.585 ± 1.501  ops/ms
FloatConversion.floatToInt      2048  thrpt   15  294.313 ± 1.263  ops/ms
FloatConversion.floatToLong     2048  thrpt   15  273.749 ± 2.261  ops/ms


Stable.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13800#discussion_r1185662747