On Thu, 21 Dec 2023 14:41:06 GMT, Robbin Ehn <rehn at openjdk.org> wrote:
>> Hi, please consider.
>>
>> Main author is @luhenry, I only fixed some minor things and tested it.
>>
>> Such as:
>> test/hotspot/jtreg/compiler/intrinsics/sha/
>> test/jdk/java/security/MessageDigest/
>> test/jdk/jdk/security/
>> tier1
>>
>> And still running some test.
>
> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
>
> index store state back
Some other comments
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3790:
> 3788: // the cost of moving those vectors at the end of each quad-rounds.
> 3789: void sha2_quad_round(Assembler::SEW vset_sew, VectorRegister rot1, VectorRegister rot2, VectorRegister rot3, VectorRegister rot4,
> 3790: Register scalarconst, VectorRegister vtemp, VectorRegister vtemp2, VectorRegister vtemp3, VectorRegister vtemp4,
maybe rename `vtemp3` -> `v_abef`, `vtemp4` -> `v_cdgh`
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3792:
> 3790: Register scalarconst, VectorRegister vtemp, VectorRegister vtemp2, VectorRegister vtemp3, VectorRegister vtemp4,
> 3791: bool gen_words = true, bool step_const = true) {
> 3792: __ vl1reXX_v(vset_sew, vtemp, scalarconst);
Seems we only incr `scalarconst ` conditionally, so load into `vtemp ` here could also be conditionally?
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3803:
> 3801: }
> 3802: if (gen_words) {
> 3803: __ vsha2ms_vv(rot1, vtemp2, rot4);
when `gen_words == false` && `step_const == true`, is it necessary to call `vmerge_vvm(vtemp2, rot3, rot2);` above?
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);
Currently, when MaxVectorSize < 16 UseRVV = false, so there are conditions when MaxVectorSize == 16 && UseRVV == true, in this case, `vsetivli` will not work as expected, and neither the following codes.
And 128 bits is the common one?
-------------
PR Review: https://git.openjdk.org/jdk/pull/16562#pullrequestreview-1793619266
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1434480065
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1434484955
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1434480079
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1434480184