RFR: 8334999: RISC-V: implement AES single block encryption/decryption intrinsics

Ludovic Henry luhenry at openjdk.org
Mon Jul 1 06:43:25 UTC 2024


On Sun, 30 Jun 2024 14:02:00 GMT, ArsenyBochkarev <duke at openjdk.org> wrote:

> Hello everyone! Please review this port of vector AES single block encryption/decryption intrinsics. On my QEMU with `Zvkned` extension enabled the `test/hotspot/jtreg/compiler/codegen/aes/TestAESMain.java` test is OK. I know that currently hardware implementing this extension is not available on the market but I suppose this PR can be a good starting point on supporting AES intrinsics for RISC-V in OpenJDK.

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

> 2346:     __ lwu(keylen, Address(key, arrayOopDesc::length_offset_in_bytes() - arrayOopDesc::base_offset_in_bytes(T_INT)));
> 2347: 
> 2348:     __ vsetivli(temp1, 4, Assembler::e32, Assembler::m1);

There is no use of `temp1` after, should we replace with `x0`?

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

> 2349:     __ vle32_v(res, from);
> 2350:     __ vmv_v_x(vzero, zr);
> 2351:     generate_vle32_pack4(key, vtmp1, vtmp2, vtmp3, vtmp4);

It would be great to add a quick comment mentioning the side effect on `key` of this function call. Same at https://github.com/openjdk/jdk/pull/19960/files#diff-97f199af6d1c8c17b2fa4f50eb1bbc0081858cc59a899f32792a2d31f933ccc4R2355 and https://github.com/openjdk/jdk/pull/19960/files#diff-97f199af6d1c8c17b2fa4f50eb1bbc0081858cc59a899f32792a2d31f933ccc4R2359

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

> 2360:     generate_rev8_pack2(vtmp1, vtmp2);
> 2361: 
> 2362:     __ mv(temp2, 44);

You could replace `temp2` by `t0`/`t1`/`t2`

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

> 2446:     __ lwu(keylen, Address(key, arrayOopDesc::length_offset_in_bytes() - arrayOopDesc::base_offset_in_bytes(T_INT)));
> 2447: 
> 2448:     __ vsetivli(temp1, 4, Assembler::e32, Assembler::m1);

Same as for encrypt, there is no use of `temp1`, could you replace by `x0`?

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

> 2457:     generate_aesdecrypt_round(res, vzero, vtmp1, vtmp2, vtmp3, vtmp4);
> 2458: 
> 2459:     generate_vle32_pack4(key, vtmp1, vtmp2, vtmp3, vtmp4);

Same as above, please add a comment on the side effect on `key`.

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

> 2464:     generate_rev8_pack2(vtmp1, vtmp2);
> 2465: 
> 2466:     __ mv(temp2, 44);

Same as above, could you use `t0`/`t1`/`t2` instead?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1660541398
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1660542460
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1660541850
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1660543748
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1660544520
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1660544242


More information about the hotspot-dev mailing list