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

Antonios Printezis tonyp at openjdk.org
Wed Nov 8 16:03:39 UTC 2023


On Wed, 8 Nov 2023 15:55:29 GMT, Antonios Printezis <tonyp at openjdk.org> wrote:

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

And yes it's a bit of a PITA to keep them in sync. But I think it's worth it, especially given how many registers are used for this intrinsic.

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

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


More information about the hotspot-compiler-dev mailing list