RFR: 8319716: RISC-V: Add SHA-2 [v8]
Robbin Ehn
rehn at openjdk.org
Fri Dec 22 11:58:53 UTC 2023
On Fri, 22 Dec 2023 04:39:43 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
>>
>> index store state back
>
> src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3812:
>
>> 3810: if (vset_sew == Assembler::e64 && !multi_block) return "sha512_implCompress";
>> 3811: if (vset_sew == Assembler::e64 && multi_block) return "sha512_implCompressMB";
>> 3812: return "bad name lookup";
>
> Maybe place a `ShouldNotReachHere();` immediately before the last return statement?
Fixed
> src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3935:
>
>> 3933: // x0 is not written, we known the number of vector elements.
>> 3934:
>> 3935: __ vsetivli(x0, 4, vset_sew, Assembler::m1, Assembler::ma, Assembler::ta);
>
> Shouldn't we set `LMUL` to `Assembler::m2` when `vset_sew` is `e64`? A single 128-bit wide vector register won't be able to hold 4 `e64` elements. I guess you were testing against 256-bit wide vector register so that this issue won't trigger? This also means most of the code won't work for sha512 with 128-bit RVV as `Assembler::m2` means register group of two. This menifests in the corresponding openssl sha512 implementation [1].
>
> (With that obervation, should we have two independent code for each algorithm like the openssl version? That seems more readable and maintainable to me.)
>
> [1] https://github.com/openssl/openssl/blob/master/crypto/sha/asm/sha512-riscv64-zvkb-zvknhb.pl
The sha instruction don't care about lmul, except only in that lmul*vlen == 256.
So there is no code difference except just setting m2 in case of 128.
These crypto uses 'EGW' element group width which is specified.
As quad rounds depend on result from previous quad round, you can only do 4xe64(for 256 e32), no more no less.
Yes, good catch! I must remember to tests multiple vlens. Thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1434995475
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1434995108
More information about the hotspot-dev
mailing list