RFR: 8315716: RISC-V: implement ChaCha20 intrinsic

[Robbin Ehn]

On Mon, 25 Sep 2023 11:47:40 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Only vector version is included in this patch.
> 
> ### Test
> The patch passed the jdk tests found via `find test/jdk/ -iname *ChaCha*`

Thanks, can you include what testing you have done in the PR description? (testing as in validating the code is correct)

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

> 4317:    *  c_rarg1   - key_stream, the array that will hold the result of the ChaCha20 block function
> 4318:    */
> 4319:   address generate_chacha20Block() {

I see the other ones are not static, but they all should be static.
Ignore, I missed these were in scope of "class StubGenerator".

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

> 4341:     // in java level.
> 4342:     __ li(avl, 16);
> 4343:     __ vsetvli(length, avl, Assembler::e32, Assembler::m1, Assembler::ma, Assembler::ta);

Is this really correct.
We have no uses of ma/ta before this since we need to make sure we never touch memory outside of the arrays.
I don't think ma/ta will ever be correct when working on Java heap.

I would drop the last two argument and use the default of mu/tu as we do everywhere else.

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

> 189:   if (UseRVV && FLAG_IS_DEFAULT(UseChaCha20Intrinsics)) {
> 190:     FLAG_SET_DEFAULT(UseChaCha20Intrinsics, true);
> 191:   }

Just below this we may set RVV to false.
I would put this just above "#ifdef COMPILER2" or so.

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

PR Comment: https://git.openjdk.org/jdk/pull/15899#issuecomment-1733594250
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1335871950
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1336175787
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1335802863