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