RFR: 8306966: RISC-V: Support vector cast node for Vector API [v5]

[Gui Cao]

On Mon, 1 May 2023 03:06:03 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Gui Cao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Small refactoring of rvv_vsetvli
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1787:
> 
>> 1785:                                               VectorRegister src, BasicType src_bt) {
>> 1786:   assert(type2aelembytes(dst_bt) > type2aelembytes(src_bt) && type2aelembytes(dst_bt) <= 8 && type2aelembytes(src_bt) <= 4, "invalid element size");
>> 1787:   assert(dst_bt != T_FLOAT && dst_bt != T_DOUBLE && src_bt != T_FLOAT && src_bt != T_DOUBLE, "should be integer element");
> 
> Suggestion: s/"should be integer element"/"unsupported element type"/

Fixed.

> src/hotspot/cpu/riscv/riscv_v.ad line 2660:
> 
>> 2658:       __ vector_integer_extend(as_VectorRegister($dst$$reg), bt == T_FLOAT ? T_INT : T_LONG,
>> 2659:                                Matcher::vector_length(this), as_VectorRegister($src$$reg), T_BYTE);
>> 2660:       __ vfcvt_f_x_v(as_VectorRegister($dst$$reg), as_VectorRegister($dst$$reg));
> 
> The single-witdh vector conversion instructions when converting to floating-point values use the dynamic rounding mode in 'frm', so I think you should set 'frm' to the correct rounding mode first. You might also want to check the exceptional conditions possibly set by those instrucions.

Fixed.

> src/hotspot/cpu/riscv/riscv_v.ad line 2718:
> 
>> 2716: %}
>> 2717: 
>> 2718: instruct vcvtItoX(vReg dst, vReg src) %{
> 
> I think the 'TEMP_DEF dst' effect is only needed for the T_DOUBLE case. It still works if 'dst' and 'src' are allocated the same vector register for the T_FLOAT case. So you might want to further break down this into two separate ones, say 'vcvtItoF' and 'vcvtItoD'.

Fixed.

> src/hotspot/cpu/riscv/riscv_v.ad line 2804:
> 
>> 2802:   ins_encode %{
>> 2803:     __ rvv_vsetvli(T_FLOAT, Matcher::vector_length(this, $src));
>> 2804:     __ vfcvt_x_f_v(as_VectorRegister($dst$$reg), as_VectorRegister($src$$reg));
> 
> The language spec [1] specfies: "The round toward zero rounding policy applies to (i) conversion of a floating-point value to an integer value ([§5.1.3](https://docs.oracle.com/javase/specs/jls/se20/html/jls-5.html#jls-5.1.3)), and (ii) floating-point remainder ([§15.17.3](https://docs.oracle.com/javase/specs/jls/se20/html/jls-15.html#jls-15.17.3))."
> 
> So it looks to me that we should use the 'rtz' variant (vfcvt.rtz.x.f.v) here to do the conversion to integer instead here. Please also check other places where we do conversion from float-point value to integer value.
> 
> [1] https://docs.oracle.com/javase/specs/jls/se20/html/jls-15.html#jls-15.4

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13684#discussion_r1183633130
PR Review Comment: https://git.openjdk.org/jdk/pull/13684#discussion_r1183632961
PR Review Comment: https://git.openjdk.org/jdk/pull/13684#discussion_r1183632617
PR Review Comment: https://git.openjdk.org/jdk/pull/13684#discussion_r1183632804