RFR: 8318158: RISC-V: implement roundD/roundF intrinsics [v9]
Hamlin Li
mli at openjdk.org
Thu Dec 21 09:00:52 UTC 2023
On Tue, 19 Dec 2023 22:45:13 GMT, Olga Mikhaltsova <omikhaltcova at openjdk.org> wrote:
>> Please, review this Implementation of the roundD/roundF intrinsics for RISC-V platform.
>>
>> In the table below it is shown that NaN argument should be processed as a special case.
>>
>> RISC-V Java
>> (FCVT.W.S) (FCVT.L.D) (long round(double a)) (int round(float a))
>> Minimum valid input (after rounding) −2^31 −2^63 Long.MIN_VALUE Integer.MIN_VALUE
>> Maximum valid input (after rounding) 2^31 − 1 2^63 − 1 Long.MAX_VALUE Integer.MAX_VALUE
>> Output for out-of-range negative input −2^31 −2^63 Long.MIN_VALUE Integer.MIN_VALUE
>> Output for −∞ −2^31 −2^63 Long.MIN_VALUE Integer.MIN_VALUE
>> Output for out-of-range positive input 2^31 − 1 2^63 - 1 Long.MAX_VALUE Integer.MAX_VALUE
>> Output for +∞ 2^31 − 1 2^63 - 1 Long.MAX_VALUE Integer.MAX_VALUE
>> Output for NaN 2^31 − 1 2^63 - 1 0 0
>>
>> The benchmark running with the 2nd fixed implementation on the T-Head RVB-ICE board shows the following performance improvement::
>>
>> **Before**
>>
>> Benchmark (TESTSIZE) Mode Cnt Score Error Units
>> FpRoundingBenchmark.test_round_double 2048 thrpt 15 59.555 0.179 ops/ms
>> FpRoundingBenchmark.test_round_float 2048 thrpt 15 49.760 0.103 ops/ms
>>
>>
>> **After**
>>
>> Benchmark (TESTSIZE) Mode Cnt Score Error Units
>> FpRoundingBenchmark.test_round_double 2048 thrpt 15 110.956 0.186 ops/ms
>> FpRoundingBenchmark.test_round_float 2048 thrpt 15 115.947 0.122 ops/ms
>
> Olga Mikhaltsova has updated the pull request incrementally with one additional commit since the last revision:
>
> Replaced li with mv
I think usage of static rounding mode here is fine for riscv.
In typical implementations, writes to the dynamic rounding mode CSR state will serialize
the pipeline.
Static rounding modes are used to implement specialized arithmetic operations that often
have to switch frequently between different rounding modes.
-- from `“F” Standard Extension`
Looks good.
Can you add some comments for java_round_float(or double)? As there were lots of discussion here, but all this information is not in the code.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16382#issuecomment-1865886113
PR Comment: https://git.openjdk.org/jdk/pull/16382#issuecomment-1865889081
More information about the hotspot-dev
mailing list