RFR: 8318723: RISC-V: C2 UDivL

Ludovic Henry luhenry at openjdk.org
Wed Oct 25 07:43:37 UTC 2023


On Tue, 24 Oct 2023 15:21:08 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> Can you review the change to add intrinsic for UDivI and UDivL?
> Thanks!
> 
> 
> ## Tests
> 
> ### Functionality
> Run tests successfully found via `grep -nr test/jdk/ -we divideUnsigned` and `grep -nr test/hotspot/ -we divideUnsigned` 
> 
> ### Performance
> NOTE: there are another 2 related issues: https://bugs.openjdk.org/browse/JDK-8318225, https://bugs.openjdk.org/browse/JDK-8318226, the pr of which will be subseqently sent out after this one finished.
> 
> #### Long
> ** Before **
> 
> LongDivMod.testDivideUnsigned                    1024          mixed  avgt    2  19852.277          ns/op
> LongDivMod.testDivideUnsigned                    1024       positive  avgt    2  29155.681          ns/op
> LongDivMod.testDivideUnsigned                    1024       negative  avgt    2   6385.280          ns/op
> 
> 
> ** After **
> 
> LongDivMod.testDivideUnsigned                    1024          mixed  avgt    2  11776.806          ns/op
> LongDivMod.testDivideUnsigned                    1024       positive  avgt    2  16101.940          ns/op
> LongDivMod.testDivideUnsigned                    1024       negative  avgt    2   6433.223          ns/op
> 
> 
> #### Integer
> ** Before **
> 
> IntegerDivMod.testDivideUnsigned                    1024          mixed  avgt    2  23498.570          ns/op
> IntegerDivMod.testDivideUnsigned                    1024       positive  avgt    2  16875.614          ns/op
> IntegerDivMod.testDivideUnsigned                    1024       negative  avgt    2  30310.243          ns/op
> 
> 
> ** After **
> 
> IntegerDivMod.testDivideUnsigned                    1024          mixed  avgt    2  23327.997          ns/op
> IntegerDivMod.testDivideUnsigned                    1024       positive  avgt    2  16708.209          ns/op
> IntegerDivMod.testDivideUnsigned                    1024       negative  avgt    2  30162.153          ns/op

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 244:

> 242:   // idiv variant which deals with MINLONG as dividend and -1 as divisor
> 243:   int corrected_idivl(Register result, Register rs1, Register rs2,
> 244:                       bool want_remainder, bool is_signed = true);

Could you not set the default value of `is_signed` to `true`, to make it clear which case it is at the callsite.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16346#discussion_r1371287339


More information about the hotspot-dev mailing list