RFR: 8319716: RISC-V: Add SHA-2 [v2]

[Hamlin Li]

On Tue, 28 Nov 2023 14:20:40 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Flag fixes
>  - Merge branch 'master' into sha256
>  - Share code
>  - SHA-2

Thanks for updating.
Please check the initial comments below.

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 1359:

> 1357:   }
> 1358: 
> 1359:   inline void vmsltu_vi(VectorRegister Vd, VectorRegister Vs2, int32_t imm, VectorMask vm = unmasked) {

Seems this function is not used in the code?
And, when `imm` == 0, seems it will output unexpected value?

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 1363:

> 1361:   }
> 1362: 
> 1363:   inline void vmsgeu_vi(VectorRegister Vd, VectorRegister Vs2, int32_t imm, VectorMask vm = unmasked) {

Same comments as `vmsltu_vi ` above.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3672:

> 3670:       Sha2Generator(MacroAssembler* masm, StubCodeGenerator* cgen) : MacroAssembler(masm->code()), _cgen(cgen) {}
> 3671:       address generate_sha256_implCompress(bool multi_block) {
> 3672:         return generate_sha2_implCompress<Assembler::e32>(multi_block);

Not sure if we should use template or just adding a parameter here, seems the latter can acheive the same effect and will not bloat the code.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3681:

> 3679:     template<Assembler::SEW T>
> 3680:     void vl1reXX_v(VectorRegister vr, Register sr) {
> 3681:       if (T == Assembler::e32) __ vl1re32_v(vr, sr);

related comments as above about the template usage, seems here it's more suitable to use a parameter.
Maybe also in some other places below.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 3895:

> 3893:       __ enter();
> 3894: 
> 3895:       __ push_reg(saved_regs, sp);

Not sure if we need to push and pop `saved_regs `, as t2 is the only register in it, or maybe I miss something?

src/hotspot/cpu/riscv/vm_version_riscv.cpp line 159:

> 157:   }
> 158: 
> 159:   if (UseZvkn) {

Maybe it's safe to move the code behind `#endif // COMPILER2` at line 291, as it depends on UseRVV.

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

PR Review: https://git.openjdk.org/jdk/pull/16562#pullrequestreview-1760417335
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1412479561
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1412479867
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1412501848
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1412502971
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1412509509
PR Review Comment: https://git.openjdk.org/jdk/pull/16562#discussion_r1412487773