Committed, thanks Juzhe and Kito. Let's wait for a while before backport to 14.

Pan

-----Original Message-----
From: Kito Cheng <kito.ch...@gmail.com> 
Sent: Monday, May 13, 2024 10:11 PM
To: juzhe.zh...@rivai.ai
Cc: Li, Pan2 <pan2...@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH v1] RISC-V: Bugfix ICE for RVV intrinisc vfw on _Float16 
scalar

LGTM as well :)

On Sat, May 11, 2024 at 3:58 PM juzhe.zh...@rivai.ai
<juzhe.zh...@rivai.ai> wrote:
>
> LGTM from my side. Wait for kito chime in.
>
> ________________________________
> juzhe.zh...@rivai.ai
>
>
> From: pan2.li
> Date: 2024-05-11 15:54
> To: gcc-patches
> CC: juzhe.zhong; kito.cheng; Pan Li
> Subject: [PATCH v1] RISC-V: Bugfix ICE for RVV intrinisc vfw on _Float16 
> scalar
> From: Pan Li <pan2...@intel.com>
>
> For the vfw vx format RVV intrinsic, the scalar type _Float16 also
> requires the zvfh extension.  Unfortunately,  we only check the
> vector tree type and miss the scalar _Float16 type checking.  For
> example:
>
> vfloat32mf2_t test_vfwsub_wf_f32mf2(vfloat32mf2_t vs2, _Float16 rs1, size_t 
> vl)
> {
>   return __riscv_vfwsub_wf_f32mf2(vs2, rs1, vl);
> }
>
> It should report some error message like zvfh extension is required
> instead of ICE for unreg insn.
>
> This patch would like to make up such kind of validation for _Float16
> in the RVV intrinsic API.  It will report some error like below when
> there is no zvfh enabled.
>
> error: built-in function '__riscv_vfwsub_wf_f32mf2(vs2,  rs1,  vl)'
>   requires the zvfhmin or zvfh ISA extension
>
> PR target/114988
>
> Passed the rv64gcv fully regression tests, included c/c++/fortran.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vector-builtins.cc
> (validate_instance_type_required_extensions): New func impl to
> validate the intrinisc func type ops.
> (expand_builtin): Validate instance type before expand.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/pr114988-1.c: New test.
> * gcc.target/riscv/rvv/base/pr114988-2.c: New test.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/config/riscv/riscv-vector-builtins.cc     | 51 +++++++++++++++++++
> .../gcc.target/riscv/rvv/base/pr114988-1.c    |  9 ++++
> .../gcc.target/riscv/rvv/base/pr114988-2.c    |  9 ++++
> 3 files changed, 69 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-2.c
>
> diff --git a/gcc/config/riscv/riscv-vector-builtins.cc 
> b/gcc/config/riscv/riscv-vector-builtins.cc
> index 192a6c230d1..3fdb4400d70 100644
> --- a/gcc/config/riscv/riscv-vector-builtins.cc
> +++ b/gcc/config/riscv/riscv-vector-builtins.cc
> @@ -4632,6 +4632,54 @@ gimple_fold_builtin (unsigned int code, 
> gimple_stmt_iterator *gsi, gcall *stmt)
>    return gimple_folder (rfn.instance, rfn.decl, gsi, stmt).fold ();
> }
> +static bool
> +validate_instance_type_required_extensions (const rvv_type_info type,
> +     tree exp)
> +{
> +  uint64_t exts = type.required_extensions;
> +
> +  if ((exts & RVV_REQUIRE_ELEN_FP_16) &&
> +    !TARGET_VECTOR_ELEN_FP_16_P (riscv_vector_elen_flags))
> +    {
> +      error_at (EXPR_LOCATION (exp),
> + "built-in function %qE requires the "
> + "zvfhmin or zvfh ISA extension",
> + exp);
> +      return false;
> +    }
> +
> +  if ((exts & RVV_REQUIRE_ELEN_FP_32) &&
> +    !TARGET_VECTOR_ELEN_FP_32_P (riscv_vector_elen_flags))
> +    {
> +      error_at (EXPR_LOCATION (exp),
> + "built-in function %qE requires the "
> + "zve32f, zve64f, zve64d or v ISA extension",
> + exp);
> +      return false;
> +    }
> +
> +  if ((exts & RVV_REQUIRE_ELEN_FP_64) &&
> +    !TARGET_VECTOR_ELEN_FP_64_P (riscv_vector_elen_flags))
> +    {
> +      error_at (EXPR_LOCATION (exp),
> + "built-in function %qE requires the zve64d or v ISA extension",
> + exp);
> +      return false;
> +    }
> +
> +  if ((exts & RVV_REQUIRE_ELEN_64) &&
> +    !TARGET_VECTOR_ELEN_64_P (riscv_vector_elen_flags))
> +    {
> +      error_at (EXPR_LOCATION (exp),
> + "built-in function %qE requires the "
> + "zve64x, zve64f, zve64d or v ISA extension",
> + exp);
> +      return false;
> +    }
> +
> +  return true;
> +}
> +
> /* Expand a call to the RVV function with subcode CODE.  EXP is the call
>     expression and TARGET is the preferred location for the result.
>     Return the value of the lhs.  */
> @@ -4649,6 +4697,9 @@ expand_builtin (unsigned int code, tree exp, rtx target)
>        return target;
>      }
> +  if (!validate_instance_type_required_extensions (rfn.instance.type, exp))
> +    return target;
> +
>    return function_expander (rfn.instance, rfn.decl, exp, target).expand ();
> }
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-1.c 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-1.c
> new file mode 100644
> index 00000000000..b8474804c88
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-1.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +vfloat32mf2_t test_vfwsub_wf_f32mf2(vfloat32mf2_t vs2, _Float16 rs1, size_t 
> vl)
> +{
> +  return __riscv_vfwsub_wf_f32mf2(vs2, rs1, vl); /* { dg-error {built-in 
> function '__riscv_vfwsub_wf_f32mf2\(vs2,  rs1,  vl\)' requires the zvfhmin or 
> zvfh ISA extension} } */
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-2.c 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-2.c
> new file mode 100644
> index 00000000000..49aa3141af3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114988-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +vfloat32mf2_t test_vfwadd_wf_f32mf2(vfloat32mf2_t vs2, _Float16 rs1, size_t 
> vl)
> +{
> +  return __riscv_vfwadd_wf_f32mf2(vs2, rs1, vl); /* { dg-error {built-in 
> function '__riscv_vfwadd_wf_f32mf2\(vs2,  rs1,  vl\)' requires the zvfhmin or 
> zvfh ISA extension} } */
> +}
> --
> 2.34.1
>
>

Reply via email to