RFR: 8322179: RISC-V: Implement SHA-1 intrinsic [v3]

Hamlin Li mli at openjdk.org
Tue Dec 19 17:54:53 UTC 2023


On Mon, 18 Dec 2023 13:45:36 GMT, Antonios Printezis <tonyp at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   round 1 review
>
> src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 4585:
> 
>> 4583:   }
>> 4584: 
>> 4585:   void sha1_reserve_prev_abcde(Register a, Register b, Register c, Register d, Register e,
> 
> I think it's safe to just use t0 and t1 for intermediate results without passing them as args. I used to pass them as args too, but I changed that for md5.

Yes, it is.
But I found out it's not that clear in some situation, and error-prone, e.g. when the code is a bit complicated.
So, my policy here is that I only use built-in register names in generate_sha1_implCompress, in all other places, I just use the renamed ones and use them explicitly.

> src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 4627:
> 
>> 4625: 
>> 4626:     // c_rarg0 - c_rarg3: x10 - x13
>> 4627:     Register buf    = c_rarg0;
> 
> You could copy the four arguments to a different set of registers and use a0 -> a3 for some of the other values to see if you can increase the number of compressed instructions that can be used. Unclear whether it's worth it or not.

Yeh, I'm not sure if we should take this approach.
Good side might be some code size reduction, bad side might be it's a bit confusing to read and maintain the code.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17130#discussion_r1431750068
PR Review Comment: https://git.openjdk.org/jdk/pull/17130#discussion_r1431757773


More information about the hotspot-dev mailing list