RFR: 8315716: RISC-V: implement ChaCha20 intrinsic
Ludovic Henry
luhenry at openjdk.org
Wed Sep 27 10:09:42 UTC 2023
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*`
Changes requested by luhenry (Committer).
Only need to do the `li` -> `mv` change and it's LGTM. Also, please change the PR title to `8315716: RISC-V: implement ChaCha20 intrinsic` to link it back to JBS. Thanks!
src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 2021:
> 2019: // rotate vector register left with shift bits, 32-bit version
> 2020: void MacroAssembler::vrol_vwi(VectorRegister vd, uint32_t shift, VectorRegister tmp_vr) {
> 2021: vsrl_vi(tmp_vr, vd, 32 - shift);
Nit: You could even inline that in `macroAssembler_riscv.hpp` to match the other "vector pseudo instructions".
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 4281:
> 4279:
> 4280: // rotate vector register left with shift bits, 32-bit version
> 4281: void rotate_left_imm(VectorRegister rv, uint32_t shift, VectorRegister tmp_vr) {
This should be in `macroAssembler_riscv.cpp` instead. I would also call it something like `vrolwi`. That'll more closely match the [`vrol`](https://github.com/riscv/riscv-crypto/blob/c8ddeb7e64a3444dda0438316af1238aeed72041/doc/vector/insns/vrol.adoc#L5)
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 4287:
> 4285: }
> 4286:
> 4287: void quarter_round(VectorRegister aVec, VectorRegister bVec,
Please rename that to `chacha20_quarter_round` to make it clear it belongs to the chacha20 algorithm
src/hotspot/cpu/riscv/vm_version_riscv.cpp line 255:
> 253: FLAG_SET_DEFAULT(UseChaCha20Intrinsics, true);
> 254: }
> 255: }
You can have something like that:
Suggestion:
if (UseRVV) {
if (FLAG_IS_DEFAULT(UseChaCha20Intrinsics)) {
FLAG_SET_DEFAULT(UseChaCha20Intrinsics, true);
}
} else if (UseChaCha20Intrinsics) {
if (!FLAG_IS_DEFAULT(UseChaCha20Intrinsics)) {
warning("Chacha20 Intrinsics requires RVV instructions (not available on this CPU)");
}
FLAG_SET_DEFAULT(UseChaCha20Intrinsics, false);
}
-------------
PR Review: https://git.openjdk.org/jdk/pull/15899#pullrequestreview-1642116248
Marked as reviewed by luhenry (Committer).
PR Review: https://git.openjdk.org/jdk/pull/15899#pullrequestreview-1646047413
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1336316527
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1335859588
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1335860186
PR Review Comment: https://git.openjdk.org/jdk/pull/15899#discussion_r1336315818
More information about the hotspot-dev
mailing list