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

[Antonios Printezis]

On Wed, 8 Nov 2023 15:29:50 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Antonios Printezis has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   variable renaming
>
> 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?

yep, fixed

> 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.

I like having the table in the comment before the code to have a high level view of which registers are used for what. This is less obvious in the code. But that's my personal opinion!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1386848362
PR Review Comment: https://git.openjdk.org/jdk/pull/16453#discussion_r1386851832