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