RFR: 8302908: RISC-V: Support masked vector arithmetic instructions for Vector API [v16]

[Dingli Zhang]

On Thu, 13 Apr 2023 12:12:35 GMT, Feilong Jiang <fjiang at openjdk.org> wrote:

>> Dingli Zhang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix typo
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1726:
> 
>> 1724: }
>> 1725: 
>> 1726: void C2_MacroAssembler::rvv_compare(VectorRegister vd, BasicType bt, int length_in_bytes, VectorRegister src1, VectorRegister src2, int cond, VectorMask vm) {
> 
> All RVV-related methods are named with the suffix `_v`  in C2_MacroAssembler (except `rvv_vsetvli` and `rvv_reduce_integral`, which should be renamed too, IMO), I think we should follow this naming style.

Thanks for the review! Fixed.

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1734:
> 
>> 1732:       case BoolTest::ne: vmfne_vv(vd, src1, src2, vm); break;
>> 1733:       case BoolTest::le: vmfle_vv(vd, src1, src2, vm); break;
>> 1734:       case BoolTest::ge: vmfle_vv(vd, src2, src1, vm); break;
> 
> Maybe we could add some pseudo instructions like `vmfge_vv`/`vmfgt_vv` [1] .
> 
> 1. https://github.com/riscv/riscv-v-spec/blob/b9afd6f5709fe3f91ce39bb83695bcfaa78eef94/v-spec.adoc?plain=1#L3676-L3681

Fixed.

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1747:
> 
>> 1745:       case BoolTest::ne: vmsne_vv(vd, src1, src2, vm); break;
>> 1746:       case BoolTest::le: vmsle_vv(vd, src1, src2, vm); break;
>> 1747:       case BoolTest::ge: vmsle_vv(vd, src2, src1, vm); break;
> 
> Same here, `vmsge_vv`/`vmsgt_vv` [1] would be better.
> 
> 1. https://github.com/riscv/riscv-v-spec/blob/b9afd6f5709fe3f91ce39bb83695bcfaa78eef94/v-spec.adoc?plain=1#L2724-L2729

Fixed.

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.hpp line 203:
> 
>> 201:  void rvv_vsetvli(BasicType bt, int length_in_bytes, Register tmp = t0);
>> 202: 
>> 203:  void rvv_compare(VectorRegister dst, BasicType bt, int length_in_bytes,
> 
> Suggestion:
> 
>  void compare_v(VectorRegister dst, BasicType bt, int length_in_bytes,

Fixed.

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.hpp line 230:
> 
>> 228: 
>> 229:   // Clear vector registers independent of previous vl and vtype.
>> 230:   void rvv_clear_register(VectorRegister v) {
> 
> Suggestion:
> 
>   void clear_register_v(VectorRegister v) {

Fixed.

> src/hotspot/cpu/riscv/riscv.ad line 919:
> 
>> 917: // The mask value used to control execution of a masked vector
>> 918: // instruction is always supplied by vector register v0.
>> 919: reg_class vectmask_reg_v0 (
> 
> Suggestion:
> 
> reg_class vmask_reg_v0 (

Fixed.

> src/hotspot/cpu/riscv/riscv.ad line 926:
> 
>> 924: // We need two more vmask registers to do the vector mask logical ops,
>> 925: // so define v30, v31 as mask register too.
>> 926: reg_class vectmask_reg (
> 
> Suggestion:
> 
> reg_class vmask_reg (

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166445903
PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166446278
PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166446324
PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166445963
PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166446007
PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166446060
PR Review Comment: https://git.openjdk.org/jdk/pull/12682#discussion_r1166446104