Committed, thanks kito.
Pan
-----Original Message-----
From: Kito Cheng <kito.ch...@gmail.com>
Sent: Monday, March 25, 2024 8:04 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang
<yanzhang.w...@intel.com>
Subject: Re: [PATCH v1] RISC-V: Allow RVV intrinsic when function
target("arch=+v")
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
>
