RFR: 8319184: RISC-V: improve MD5 intrinsic [v4]

Hamlin Li mli at openjdk.org
Wed Nov 8 15:50:59 UTC 2023


On Tue, 7 Nov 2023 16:20:47 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
>
> Antonios Printezis has updated the pull request incrementally with one additional commit since the last revision:
> 
>   variable renaming

LGTM.
Just some minor nits.

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

> 3941:     // add i'th 32-bit integer to dest
> 3942:     void add_u32(const Register dest, uint i, const Register rtmp = t0) {
> 3943:        assert(i < 2 * L, "invalid i: %u", i);

indent?

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

> 4126:     Register state1    = x20; // s4
> 4127:     Register state2    = x21; // s5
> 4128:     Register state3    = x22; // s6

Seems to me the comment before the head of this method is kind of duplicated with this part in the code.
I would recommend to merge the 2 part here, and remove the comment before the head of this method (LINE 4048-4080). By doing this, 
1. the reader is not distracted to read 2 parts of similar code to get the information;
2. when modify the code, writer can just modify one place.

But thsi is quite subjective, and it's piece of existing code, so it depends on you.

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

Marked as reviewed by mli (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16453#pullrequestreview-1720649578
PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1386806212
PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1386837774


More information about the hotspot-compiler-dev mailing list