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