RE: [PATCH v10] RISC-V: Refactor requirement of ZVFH and ZVFHMIN.

["Li, Pan2" <pan2.li@intel.com>]

Committed, thanks Juzhe and Kito.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com> 
Sent: Friday, June 9, 2023 4:28 PM
To: juzhe.zhong@rivai.ai
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>; Robin Dapp <rdapp.gcc@gmail.com>; jeffreyalaw <jeffreyalaw@gmail.com>; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v10] RISC-V: Refactor requirement of ZVFH and ZVFHMIN.

lgtm too, thanks :)

On Fri, Jun 9, 2023 at 3:15 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> LGTM.
>
>
>
> juzhe.zhong@rivai.ai
>
> From: pan2.li
> Date: 2023-06-09 15:07
> To: gcc-patches
> CC: juzhe.zhong; rdapp.gcc; jeffreyalaw; pan2.li; yanzhang.wang; kito.cheng
> Subject: [PATCH v10] RISC-V: Refactor requirement of ZVFH and ZVFHMIN.
> From: Pan Li <pan2.li@intel.com>
>
> This patch would like to refactor the requirement of both the ZVFH
> and ZVFHMIN. By default, the ZVFHMIN will enable FP16 for all the
> iterators of RVV. And then the ZVFH will leverage one define attr as
> the gate for FP16 supported or not.
>
> Please note the ZVFH will cover the ZVFHMIN instructions. This patch
> add one test for this.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> Co-Authored by: Juzhe-Zhong <juzhe.zhong@rivai.ai>
> Co-Authored by: Kito Cheng <kito.cheng@sifive.com>
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.md (enabled): Move to another place, and
> add fp_vector_disabled to the cond.
> (fp_vector_disabled): New attr defined for disabling fp.
> * config/riscv/vector-iterators.md: Fix V_WHOLE and V_FRACT.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/zvfhmin-intrinsic.c: Add vle16 test
> for ZVFHMIN.
> ---
> gcc/config/riscv/riscv.md                     | 39 ++++++++++++++++---
> gcc/config/riscv/vector-iterators.md          | 23 ++++++-----
> .../riscv/rvv/base/zvfhmin-intrinsic.c        | 15 ++++++-
> 3 files changed, 59 insertions(+), 18 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 38b8fba2a53..d8e935cb934 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -239,12 +239,6 @@ (define_attr "ext_enabled" "no,yes"
> ]
> (const_string "no")))
> -;; Attribute to control enable or disable instructions.
> -(define_attr "enabled" "no,yes"
> -  (cond [(eq_attr "ext_enabled" "no")
> - (const_string "no")]
> - (const_string "yes")))
> -
> ;; Classification of each insn.
> ;; branch conditional branch
> ;; jump unconditional jump
> @@ -434,6 +428,39 @@ (define_attr "type"
> (eq_attr "move_type" "rdvlenb") (const_string "rdvlenb")]
> (const_string "unknown")))
> +;; True if the float point vector is disabled.
> +(define_attr "fp_vector_disabled" "no,yes"
> +  (cond [
> +    (and (eq_attr "type" "vfmov,vfalu,vfmul,vfdiv,
> +   vfwalu,vfwmul,vfmuladd,vfwmuladd,
> +   vfsqrt,vfrecp,vfminmax,vfsgnj,vfcmp,
> +   vfclass,vfmerge,
> +   vfncvtitof,vfwcvtftoi,vfcvtftoi,vfcvtitof,
> +   vfredo,vfredu,vfwredo,vfwredu,
> +   vfslide1up,vfslide1down")
> + (and (eq_attr "mode" "VNx1HF,VNx2HF,VNx4HF,VNx8HF,VNx16HF,VNx32HF,VNx64HF")
> +       (match_test "!TARGET_ZVFH")))
> +    (const_string "yes")
> +
> +    ;; The mode records as QI for the FP16 <=> INT8 instruction.
> +    (and (eq_attr "type" "vfncvtftoi,vfwcvtitof")
> + (and (eq_attr "mode" "VNx1QI,VNx2QI,VNx4QI,VNx8QI,VNx16QI,VNx32QI,VNx64QI")
> +       (match_test "!TARGET_ZVFH")))
> +    (const_string "yes")
> +  ]
> +  (const_string "no")))
> +
> +;; Attribute to control enable or disable instructions.
> +(define_attr "enabled" "no,yes"
> +  (cond [
> +    (eq_attr "ext_enabled" "no")
> +    (const_string "no")
> +
> +    (eq_attr "fp_vector_disabled" "yes")
> +    (const_string "no")
> +  ]
> +  (const_string "yes")))
> +
> ;; Length of instruction in bytes.
> (define_attr "length" ""
>     (cond [
> diff --git a/gcc/config/riscv/vector-iterators.md b/gcc/config/riscv/vector-iterators.md
> index f4946d84449..234b712bc9d 100644
> --- a/gcc/config/riscv/vector-iterators.md
> +++ b/gcc/config/riscv/vector-iterators.md
> @@ -453,9 +453,8 @@ (define_mode_iterator V_WHOLE [
>    (VNx1DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN < 128") (VNx2DI "TARGET_VECTOR_ELEN_64")
>    (VNx4DI "TARGET_VECTOR_ELEN_64") (VNx8DI "TARGET_VECTOR_ELEN_64") (VNx16DI "TARGET_VECTOR_ELEN_64 && TARGET_MIN_VLEN >= 128")
> -  (VNx1HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN < 128")
> -  (VNx2HF "TARGET_VECTOR_ELEN_FP_16")
> -  (VNx4HF "TARGET_VECTOR_ELEN_FP_16")
> +  (VNx2HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN == 32")
> +  (VNx4HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN == 64")
>    (VNx8HF "TARGET_VECTOR_ELEN_FP_16")
>    (VNx16HF "TARGET_VECTOR_ELEN_FP_16")
>    (VNx32HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN > 32")
> @@ -477,7 +476,11 @@ (define_mode_iterator V_WHOLE [
> (define_mode_iterator V_FRACT [
>    (VNx1QI "TARGET_MIN_VLEN < 128") VNx2QI (VNx4QI "TARGET_MIN_VLEN > 32") (VNx8QI "TARGET_MIN_VLEN >= 128")
>    (VNx1HI "TARGET_MIN_VLEN < 128") (VNx2HI "TARGET_MIN_VLEN > 32") (VNx4HI "TARGET_MIN_VLEN >= 128")
> -  (VNx1HF "TARGET_MIN_VLEN < 128") (VNx2HF "TARGET_MIN_VLEN > 32") (VNx4HF "TARGET_MIN_VLEN >= 128")
> +
> +  (VNx1HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN < 128")
> +  (VNx2HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN > 32")
> +  (VNx4HF "TARGET_VECTOR_ELEN_FP_16 && TARGET_MIN_VLEN >= 128")
> +
>    (VNx1SI "TARGET_MIN_VLEN > 32 && TARGET_MIN_VLEN < 128") (VNx2SI "TARGET_MIN_VLEN >= 128")
>    (VNx1SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN > 32 && TARGET_MIN_VLEN < 128")
>    (VNx2SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN >= 128")
> @@ -497,12 +500,12 @@ (define_mode_iterator VWEXTI [
> ])
> (define_mode_iterator VWEXTF [
> -  (VNx1SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN < 128")
> -  (VNx2SF "TARGET_VECTOR_ELEN_FP_32")
> -  (VNx4SF "TARGET_VECTOR_ELEN_FP_32")
> -  (VNx8SF "TARGET_VECTOR_ELEN_FP_32")
> -  (VNx16SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN > 32")
> -  (VNx32SF "TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN >= 128")
> +  (VNx1SF "TARGET_VECTOR_ELEN_FP_16 && TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN < 128")
> +  (VNx2SF "TARGET_VECTOR_ELEN_FP_16 && TARGET_VECTOR_ELEN_FP_32")
> +  (VNx4SF "TARGET_VECTOR_ELEN_FP_16 && TARGET_VECTOR_ELEN_FP_32")
> +  (VNx8SF "TARGET_VECTOR_ELEN_FP_16 && TARGET_VECTOR_ELEN_FP_32")
> +  (VNx16SF "TARGET_VECTOR_ELEN_FP_16 && TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN > 32")
> +  (VNx32SF "TARGET_VECTOR_ELEN_FP_16 && TARGET_VECTOR_ELEN_FP_32 && TARGET_MIN_VLEN >= 128")
>    (VNx1DF "TARGET_VECTOR_ELEN_FP_64 && TARGET_MIN_VLEN < 128")
>    (VNx2DF "TARGET_VECTOR_ELEN_FP_64")
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/zvfhmin-intrinsic.c b/gcc/testsuite/gcc.target/riscv/rvv/base/zvfhmin-intrinsic.c
> index 0923b6bc4d2..f1a29b639e0 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/base/zvfhmin-intrinsic.c
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/zvfhmin-intrinsic.c
> @@ -3,6 +3,8 @@
> #include "riscv_vector.h"
> +typedef _Float16 float16_t;
> +
> vfloat16mf4_t test_vfncvt_f_f_w_f16mf4(vfloat32mf2_t src, size_t vl) {
>    return __riscv_vfncvt_f_f_w_f16mf4(src, vl);
> }
> @@ -43,11 +45,20 @@ vfloat32m8_t test_vfwcvt_f_f_v_f32m8(vfloat16m4_t src, size_t vl) {
>    return __riscv_vfwcvt_f_f_v_f32m8(src, vl);
> }
> -/* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*mf4,\s*t[au],\s*m[au]} 2 } } */
> +vfloat16mf4_t test_vle16_v_f16mf4(const float16_t *base, size_t vl) {
> +  return __riscv_vle16_v_f16mf4(base, vl);
> +}
> +
> +vfloat16m8_t test_vle16_v_f16m8(const float16_t *base, size_t vl) {
> +  return __riscv_vle16_v_f16m8(base, vl);
> +}
> +
> +/* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*mf4,\s*t[au],\s*m[au]} 3 } } */
> /* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*mf2,\s*t[au],\s*m[au]} 2 } } */
> /* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*m1,\s*t[au],\s*m[au]} 2 } } */
> /* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*m2,\s*t[au],\s*m[au]} 2 } } */
> /* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*m4,\s*t[au],\s*m[au]} 2 } } */
> +/* { dg-final { scan-assembler-times {vsetvli\s+zero,\s*[a-x0-9]+,\s*e16,\s*m8,\s*t[au],\s*m[au]} 1 } } */
> /* { dg-final { scan-assembler-times {vfwcvt\.f\.f\.v\s+v[0-9]+,\s*v[0-9]+} 5 } } */
> /* { dg-final { scan-assembler-times {vfncvt\.f\.f\.w\s+v[0-9]+,\s*v[0-9]+} 5 } } */
> -
> +/* { dg-final { scan-assembler-times {vle16\.v\s+v[0-9]+,\s*0\([0-9ax]+\)} 4 } } */
> --
> 2.34.1
>
>