LGTM, thanks :)

On Mon, Mar 25, 2024 at 3:42 PM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> This patch would like to allow the RVV intrinsic when function is
> attributed as target("arch=+v") and build with rv64gc.  For example:
>
> vint32m1_t
> __attribute__((target("arch=+v")))
> test_1 (vint32m1_t a, vint32m1_t b, size_t vl)
> {
>   return __riscv_vadd_vv_i32m1 (a, b, vl);
> }
>
> build with -march=rv64gc -mabi=lp64d -O3, we will have asm like below:
> test_1:
>   .option push
>   .option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_\
> zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0
>   vsetvli zero,a0,e32,m1,ta,ma
>   vadd.vv v8,v8,v9
>   ret
>
> The riscv_vector.h must be included when leverage intrinisc type(s) and
> API(s).  And the scope of this attribute should not excced the function
> body.  Meanwhile, to make rvv types and API(s) available for this attribute,
> include riscv_vector.h will not report error for now if v is not present
> in march.
>
> Below test are passed for this patch:
> * The riscv fully regression test.
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv-c.cc (riscv_pragma_intrinsic): Remove error
>         when V is disabled and init the RVV types and intrinic APIs.
>         * config/riscv/riscv-vector-builtins.cc (expand_builtin): Report
>         error if V ext is disabled.
>         * config/riscv/riscv.cc (riscv_return_value_is_vector_type_p):
>         Ditto.
>         (riscv_arguments_is_vector_type_p): Ditto.
>         (riscv_vector_cc_function_p): Ditto.
>         * config/riscv/riscv_vector.h: Remove error if V is disable.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/base/pragma-1.c: Remove.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-1.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-2.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-3.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-4.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-5.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-6.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-7.c: 
> New test.
>         * gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-8.c: 
> New test.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
>  gcc/config/riscv/riscv-c.cc                   | 18 +++++++----
>  gcc/config/riscv/riscv-vector-builtins.cc     |  5 ++++
>  gcc/config/riscv/riscv.cc                     | 30 ++++++++++++++++---
>  gcc/config/riscv/riscv_vector.h               |  4 ---
>  .../gcc.target/riscv/rvv/base/pragma-1.c      |  4 ---
>  .../target_attribute_v_with_intrinsic-1.c     |  5 ++++
>  .../target_attribute_v_with_intrinsic-2.c     | 18 +++++++++++
>  .../target_attribute_v_with_intrinsic-3.c     | 13 ++++++++
>  .../target_attribute_v_with_intrinsic-4.c     | 10 +++++++
>  .../target_attribute_v_with_intrinsic-5.c     | 12 ++++++++
>  .../target_attribute_v_with_intrinsic-6.c     | 12 ++++++++
>  .../target_attribute_v_with_intrinsic-7.c     |  9 ++++++
>  .../target_attribute_v_with_intrinsic-8.c     | 23 ++++++++++++++
>  13 files changed, 145 insertions(+), 18 deletions(-)
>  delete mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pragma-1.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-1.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-2.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-3.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-4.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-5.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-6.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-7.c
>  create mode 100644 
> gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-8.c
>
> diff --git a/gcc/config/riscv/riscv-c.cc b/gcc/config/riscv/riscv-c.cc
> index edb866d51e4..01314037461 100644
> --- a/gcc/config/riscv/riscv-c.cc
> +++ b/gcc/config/riscv/riscv-c.cc
> @@ -201,14 +201,20 @@ riscv_pragma_intrinsic (cpp_reader *)
>    if (strcmp (name, "vector") == 0
>        || strcmp (name, "xtheadvector") == 0)
>      {
> -      if (!TARGET_VECTOR)
> +      if (TARGET_VECTOR)
> +       riscv_vector::handle_pragma_vector ();
> +      else /* Indicates riscv_vector.h is included but v is missing in arch  
> */
>         {
> -         error ("%<#pragma riscv intrinsic%> option %qs needs 'V' or "
> -                "'XTHEADVECTOR' extension enabled",
> -                name);
> -         return;
> +         /* To make the the rvv types and intrinsic API available for the
> +            target("arch=+v") attribute,  we need to temporally enable the
> +            TARGET_VECTOR, and disable it after all initialized.  */
> +         target_flags |= MASK_VECTOR;
> +
> +         riscv_vector::init_builtins ();
> +         riscv_vector::handle_pragma_vector ();
> +
> +         target_flags &= ~MASK_VECTOR;
>         }
> -      riscv_vector::handle_pragma_vector ();
>      }
>    else
>      error ("unknown %<#pragma riscv intrinsic%> option %qs", name);
> diff --git a/gcc/config/riscv/riscv-vector-builtins.cc 
> b/gcc/config/riscv/riscv-vector-builtins.cc
> index c5881a501d1..e07373d8b57 100644
> --- a/gcc/config/riscv/riscv-vector-builtins.cc
> +++ b/gcc/config/riscv/riscv-vector-builtins.cc
> @@ -4586,6 +4586,11 @@ rtx
>  expand_builtin (unsigned int code, tree exp, rtx target)
>  {
>    registered_function &rfn = *(*registered_functions)[code];
> +
> +  if (!TARGET_VECTOR)
> +    error_at (EXPR_LOCATION (exp),
> +             "builtin function %qE requires the V ISA extension", exp);
> +
>    return function_expander (rfn.instance, rfn.decl, exp, target).expand ();
>  }
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 6d11576a8fd..fe9976bfffe 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -5467,7 +5467,15 @@ riscv_return_value_is_vector_type_p (const_tree fntype)
>  {
>    tree return_type = TREE_TYPE (fntype);
>
> -  return riscv_vector_type_p (return_type);
> +  if (riscv_vector_type_p (return_type))
> +    {
> +      if (!TARGET_VECTOR)
> +       error_at (input_location,
> +                 "return type %qT requires the V ISA extension", 
> return_type);
> +      return true;
> +    }
> +  else
> +    return false;
>  }
>
>  /* Return true if a function with type FNTYPE takes arguments in
> @@ -5481,7 +5489,13 @@ riscv_arguments_is_vector_type_p (const_tree fntype)
>      {
>        tree arg_type = TREE_VALUE (chain);
>        if (riscv_vector_type_p (arg_type))
> -       return true;
> +       {
> +         if (!TARGET_VECTOR)
> +           error_at (input_location,
> +                     "argument type %qT requires the V ISA extension",
> +                     arg_type);
> +         return true;
> +       }
>      }
>
>    return false;
> @@ -5493,8 +5507,16 @@ riscv_arguments_is_vector_type_p (const_tree fntype)
>  static bool
>  riscv_vector_cc_function_p (const_tree fntype)
>  {
> -  return lookup_attribute ("vector_cc", TYPE_ATTRIBUTES (fntype)) != 
> NULL_TREE
> -        || lookup_attribute ("riscv_vector_cc", TYPE_ATTRIBUTES (fntype)) != 
> NULL_TREE;
> +  tree attr = TYPE_ATTRIBUTES (fntype);
> +  bool vector_cc_p = lookup_attribute ("vector_cc", attr) != NULL_TREE
> +    || lookup_attribute ("riscv_vector_cc", attr) != NULL_TREE;
> +
> +  if (vector_cc_p && !TARGET_VECTOR)
> +    error_at (input_location,
> +             "function attribute %qs requires the V ISA extension",
> +             "riscv_vector_cc");
> +
> +  return vector_cc_p;
>  }
>
>  /* Implement TARGET_FNTYPE_ABI.  */
> diff --git a/gcc/config/riscv/riscv_vector.h b/gcc/config/riscv/riscv_vector.h
> index c2fc4b35242..aa0c3aa69c0 100644
> --- a/gcc/config/riscv/riscv_vector.h
> +++ b/gcc/config/riscv/riscv_vector.h
> @@ -28,9 +28,6 @@
>  #include <stdint.h>
>  #include <stddef.h>
>
> -#ifndef __riscv_vector
> -#error "Vector intrinsics require the vector extension."
> -#else
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -45,5 +42,4 @@ extern "C" {
>  #ifdef __cplusplus
>  }
>  #endif // __cplusplus
> -#endif // __riscv_vector
>  #endif // __RISCV_VECTOR_H
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pragma-1.c 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pragma-1.c
> deleted file mode 100644
> index ef329e30785..00000000000
> --- a/gcc/testsuite/gcc.target/riscv/rvv/base/pragma-1.c
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -/* { dg-do compile } */
> -/* { dg-options "-O3 -march=rv32gc -mabi=ilp32d" } */
> -
> -#pragma riscv intrinsic "vector" /* { dg-error {#pragma riscv intrinsic' 
> option 'vector' needs 'V' or 'XTHEADVECTOR' extension enabled} } */
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-1.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-1.c
> new file mode 100644
> index 00000000000..dfe8191021e
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-1.c
> @@ -0,0 +1,5 @@
> +/* Test that we do not have error when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-2.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-2.c
> new file mode 100644
> index 00000000000..9992347fe6b
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-2.c
> @@ -0,0 +1,18 @@
> +/* Test that we do not have error when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +vint32m1_t
> +__attribute__((target("arch=+v")))
> +test_1 (vint32m1_t a, vint32m1_t b, size_t vl)
> +{
> +  return __riscv_vadd_vv_i32m1 (a, b, vl);
> +}
> +
> +void
> +test_2 ()
> +{
> +  vint32m1_t a;
> +}
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-3.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-3.c
> new file mode 100644
> index 00000000000..590343ddbfd
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-3.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -Wno-implicit-int" } */
> +
> +#include "riscv_vector.h"
> +
> +vint32m1_t
> +__attribute__((target("arch=+zbb")))
> +test_1 (vint32m1_t a, vint32m1_t b, size_t vl)
> +{
> +  return __riscv_vadd_vv_i32m1 (a, b, vl);
> +}
> +
> +/* { dg-error "return type 'vint32m1_t' requires the V ISA extension" "" { 
> target { "riscv*-*-*" } } 0 } */
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-4.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-4.c
> new file mode 100644
> index 00000000000..0acece7640c
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-4.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +void
> +test_1 (vint32m1_t a) /* { dg-error {argument type 'vint32m1_t' requires the 
> V ISA extension} } */
> +{
> +  return;
> +}
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-5.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-5.c
> new file mode 100644
> index 00000000000..2dc4217ff32
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-5.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +vint32m1_t test_1 ()
> +{
> +  vint32m1_t a;
> +  return a;
> +}
> +
> +/* { dg-error "return type 'vint32m1_t' requires the V ISA extension" "" { 
> target { "riscv*-*-*" } } 0 } */
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-6.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-6.c
> new file mode 100644
> index 00000000000..562bb509e9e
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-6.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +int
> +__attribute__((riscv_vector_cc))
> +test_1 (int a)
> +{
> +  return a + 1;
> +}
> +/* { dg-error "function attribute 'riscv_vector_cc' requires the V ISA 
> extension" "" { target { "riscv*-*-*" } } 0 } */
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-7.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-7.c
> new file mode 100644
> index 00000000000..520b2e59fae
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-7.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +size_t test_1 (size_t vl)
> +{
> +  return __riscv_vsetvl_e8m4 (vl); /* { dg-error {builtin function 
> '__riscv_vsetvl_e8m4\(vl\)' requires the V ISA extension} } */
> +}
> diff --git 
> a/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-8.c
>  
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-8.c
> new file mode 100644
> index 00000000000..9032d9d0b43
> --- /dev/null
> +++ 
> b/gcc/testsuite/gcc.target/riscv/rvv/base/target_attribute_v_with_intrinsic-8.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3" } */
> +
> +#include "riscv_vector.h"
> +
> +vint32m1_t
> +__attribute__((target("arch=+v")))
> +test_1 (vint32m1_t a, vint32m1_t b, size_t vl)
> +{
> +  return __riscv_vadd_vv_i32m1 (a, b, vl);
> +}
> +
> +void
> +test_2 ()
> +{
> +  vint32m1_t a;
> +}
> +
> +size_t
> +test_3 (size_t vl)
> +{
> +  return __riscv_vsetvl_e8m4 (vl); /* { dg-error {builtin function 
> '__riscv_vsetvl_e8m4\(vl\)' requires the V ISA extension} } */
> +}
> --
> 2.34.1
>

Reply via email to