RFR: 8319184: RISC-V: improve MD5 intrinsic

Fei Yang fyang at openjdk.org
Thu Nov 2 09:49:03 UTC 2023


On Wed, 1 Nov 2023 15:05:09 GMT, Antonios Printezis <tonyp at openjdk.org> wrote:

> Tweaks to the RISC-V MD5 intrinsic.
> 
> * do the loads from the buffer more incrementally instead of all in one go
> * don't mask off the top 32 bits of a register before an addw instruction, as addw will ignore them anyway
> * remove the rmask32 register, as the mask is not needed any more (only at the start / end and it's 
> * cleanup some of the register usage

Some comments from a brief look.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3942:

> 3940: 
> 3941:     // add i'th 32-bit integer to dest
> 3942:     void addw(const Register dest, uint i, const Register rtmp) {

Can we give this function another name (Maybe something like `add_u32`) to avoid possible confusion with the `addw` assember function? Or simply inline the code into the caller?

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3964:

> 3962:                                Register a, Register b, Register c, Register d,
> 3963:                                int k, int s, int t,
> 3964:                                Register rtmp1, Register rtmp2, Register rtmp3) {

Since `rtmp2` parameter is now not used, why not remove it?

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 4133:

> 4131:     Register state1    =  x6; // t1
> 4132:     Register state2    =  x7; // t2
> 4133:     Register state3    = x20; // s4

I don't think it's safe to use x5 (t0) / x6 (t1) to keep some long-lived values like `state0` / `state1`. Those two are reserved scratch registers which could be explictly / implicitly clobberred by various assembler functions like [1]. 

[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/macroAssembler_riscv.hpp#L760-L763

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

PR Review: https://git.openjdk.org/jdk/pull/16453#pullrequestreview-1709760619
PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1379800916
PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1379792194
PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1379831529


More information about the hotspot-compiler-dev mailing list