RFR: 8320999: RISC-V: C2 RotateLeftV [v4]

[Fei Yang]

On Tue, 28 May 2024 09:06:14 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review this patch?
>> More detailed description is inline in the code.
>> Thanks
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add reg version

Two minor comments remain. Otherwise looks good to me.
BTW: You didn't mention the testing performed. Are these newly-added instructs properly test covered? Thanks.

src/hotspot/cpu/riscv/riscv_v.ad line 3082:

> 3080: //
> 3081: // NOTE: for Long, its valid rotation value is 6 bits, although basic vector instruction only support 5 bit vector-immediate,
> 3082: //       in Zvbb, vror.vi support 6 bits vector-immediate, so the imm implementation of Long and other types can be unified.

Maybe simply: `As vror.vi encodes 6-bits immediate rotate amount, which is different from other vector-immediate instructions, implementation of vector rotation for long and other types can be unified.`

src/hotspot/cpu/riscv/riscv_v.ad line 3130:

> 3128: instruct vrotate_right_masked(vReg dst_src, vReg shift, vRegMask_V0 v0) %{
> 3129:   match(Set dst_src (RotateRightV (Binary dst_src shift) v0));
> 3130:   effect(TEMP_DEF dst_src);

Is the `TEMP_DEF dst_src` needed for these newly-added masked versions?

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

PR Review: https://git.openjdk.org/jdk/pull/19325#pullrequestreview-2084533121
PR Review Comment: https://git.openjdk.org/jdk/pull/19325#discussion_r1618349791
PR Review Comment: https://git.openjdk.org/jdk/pull/19325#discussion_r1618361073