RFR: 8321021: RISC-V: C2 VectorUCastB2X [v5]

[Fei Yang]

On Tue, 12 Mar 2024 17:21:24 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review the patch to add support for some vector intrinsics?
>> Also complement various tests on riscv.
>> Thanks.
>> 
>> ## Test
>> test/hotspot/jtreg/compiler/vectorapi/
>> test/hotspot/jtreg/compiler/vectorization/
>
> Hamlin Li has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
> 
>  - merge master
>  - remove ucast from i/s/b to float
>  - revert some chnage; remove effect(TEMP_DEF dst) for non-extending intrinsics
>  - fix typo
>  - modify test config
>  - clean code
>  - add more tests
>  - rearrange tests layout
>  - merge master
>  - Initial commit

Thanks for the quick update. Two minor comments remain. Looks good otherwise.

src/hotspot/cpu/riscv/assembler_riscv.hpp line 1284:

> 1282:   INSN(vfwcvt_f_f_v,      0b1010111, 0b001, 0b01100, 0b010010);
> 1283:   INSN(vfwcvt_rtz_x_f_v,  0b1010111, 0b001, 0b01111, 0b010010);
> 1284:   INSN(vfwcvt_rtz_xu_f_v, 0b1010111, 0b001, 0b01110, 0b010010);

I see no use of these newly added assembler functions. So test coverage would be an issue. Maybe add them in the future when they are really needed?

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

> 3213: %}
> 3214: 
> 3215: instruct vcvtUBtoX_extend(vReg dst, vReg src) %{

Personally, I don't like the `_extend` suffix in the instruct name. I prefer names like `vzeroExtBtoX` which make it explicit that this will zero-extend the vector elements. Or simply `vcvtUBtoX`.

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

PR Review: https://git.openjdk.org/jdk/pull/18040#pullrequestreview-1934187709
PR Review Comment: https://git.openjdk.org/jdk/pull/18040#discussion_r1523264264
PR Review Comment: https://git.openjdk.org/jdk/pull/18040#discussion_r1523273365