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

[ArsenyBochkarev]

On Mon, 1 Jul 2024 06:37:32 GMT, Ludovic Henry <luhenry at openjdk.org> wrote:

>> ArsenyBochkarev has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Use t2 directly instead of temp2
>>  - Rename temp1 -> x0
>>  - Left a note on a side effect of generate_vle32_pack4
>
> 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`?

Replaced, thanks!

> 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

Done

> 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`

Ok, done! I used `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`?

Replaced

> 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`.

All done!

> 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?

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1667713599
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1667713593
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1667713596
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1667713589
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1667713582
PR Review Comment: https://git.openjdk.org/jdk/pull/19960#discussion_r1667713587