[v5] RISC-V: Add vector psabi checking.
Checks
Commit Message
From: Yanzhang Wang <yanzhang.wang@intel.com>
This patch adds support to check function's argument or return is vector type
and throw warning if yes.
There're two exceptions,
- The vector_size attribute.
- The intrinsic functions.
gcc/ChangeLog:
* config/riscv/riscv-protos.h (riscv_init_cumulative_args): Set
warning flag if func is not builtin
* config/riscv/riscv.cc
(riscv_scalable_vector_type_p): Determine whether the type is scalable vector.
(riscv_arg_has_vector): Determine whether the arg is vector type.
(riscv_pass_in_vector_p): Check the vector type param is passed by value.
(riscv_init_cumulative_args): The same as header.
(riscv_get_arg_info): Add the checking.
(riscv_function_value): Check the func return and set warning flag
* config/riscv/riscv.h (INIT_CUMULATIVE_ARGS): Add a flag to
determine whether warning psabi or not.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/rvv.exp: Add -Wno-psabi
* gcc.target/riscv/vector-abi-1.c: New test.
* gcc.target/riscv/vector-abi-2.c: New test.
* gcc.target/riscv/vector-abi-3.c: New test.
* gcc.target/riscv/vector-abi-4.c: New test.
* gcc.target/riscv/vector-abi-5.c: New test.
* gcc.target/riscv/vector-abi-6.c: New test.
Signed-off-by: Yanzhang Wang <yanzhang.wang@intel.com>
Co-authored-by: Kito Cheng <kito.cheng@sifive.com>
---
gcc/config/riscv/riscv-protos.h | 2 +
gcc/config/riscv/riscv.cc | 112 +++++++++++++++++-
gcc/config/riscv/riscv.h | 5 +-
gcc/testsuite/gcc.target/riscv/rvv/rvv.exp | 2 +-
gcc/testsuite/gcc.target/riscv/vector-abi-1.c | 14 +++
gcc/testsuite/gcc.target/riscv/vector-abi-2.c | 15 +++
gcc/testsuite/gcc.target/riscv/vector-abi-3.c | 14 +++
gcc/testsuite/gcc.target/riscv/vector-abi-4.c | 16 +++
gcc/testsuite/gcc.target/riscv/vector-abi-5.c | 15 +++
gcc/testsuite/gcc.target/riscv/vector-abi-6.c | 20 ++++
10 files changed, 212 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-2.c
create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-3.c
create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-4.c
create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-5.c
create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-6.c
Comments
I found there're still some test cases that does not pass. I'll push
another version soon. Sorry for the inconvenience.
> -----Original Message-----
> From: Wang, Yanzhang <yanzhang.wang@intel.com>
> Sent: Monday, June 12, 2023 4:08 PM
> To: gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Li, Pan2
> <pan2.li@intel.com>; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: [PATCH v5] RISC-V: Add vector psabi checking.
>
> From: Yanzhang Wang <yanzhang.wang@intel.com>
>
> This patch adds support to check function's argument or return is vector
> type and throw warning if yes.
>
> There're two exceptions,
> - The vector_size attribute.
> - The intrinsic functions.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-protos.h (riscv_init_cumulative_args): Set
> warning flag if func is not builtin
> * config/riscv/riscv.cc
> (riscv_scalable_vector_type_p): Determine whether the type is scalable
> vector.
> (riscv_arg_has_vector): Determine whether the arg is vector type.
> (riscv_pass_in_vector_p): Check the vector type param is passed by
> value.
> (riscv_init_cumulative_args): The same as header.
> (riscv_get_arg_info): Add the checking.
> (riscv_function_value): Check the func return and set warning flag
> * config/riscv/riscv.h (INIT_CUMULATIVE_ARGS): Add a flag to
> determine whether warning psabi or not.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/rvv.exp: Add -Wno-psabi
> * gcc.target/riscv/vector-abi-1.c: New test.
> * gcc.target/riscv/vector-abi-2.c: New test.
> * gcc.target/riscv/vector-abi-3.c: New test.
> * gcc.target/riscv/vector-abi-4.c: New test.
> * gcc.target/riscv/vector-abi-5.c: New test.
> * gcc.target/riscv/vector-abi-6.c: New test.
>
> Signed-off-by: Yanzhang Wang <yanzhang.wang@intel.com>
> Co-authored-by: Kito Cheng <kito.cheng@sifive.com>
> ---
> gcc/config/riscv/riscv-protos.h | 2 +
> gcc/config/riscv/riscv.cc | 112 +++++++++++++++++-
> gcc/config/riscv/riscv.h | 5 +-
> gcc/testsuite/gcc.target/riscv/rvv/rvv.exp | 2 +-
> gcc/testsuite/gcc.target/riscv/vector-abi-1.c | 14 +++
> gcc/testsuite/gcc.target/riscv/vector-abi-2.c | 15 +++
> gcc/testsuite/gcc.target/riscv/vector-abi-3.c | 14 +++
> gcc/testsuite/gcc.target/riscv/vector-abi-4.c | 16 +++
> gcc/testsuite/gcc.target/riscv/vector-abi-5.c | 15 +++
> gcc/testsuite/gcc.target/riscv/vector-abi-6.c | 20 ++++
> 10 files changed, 212 insertions(+), 3 deletions(-) create mode 100644
> gcc/testsuite/gcc.target/riscv/vector-abi-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-2.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-3.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-4.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-5.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/vector-abi-6.c
>
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-
> protos.h index 66c1f535d60..90fde5f8be3 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -302,4 +302,6 @@ th_mempair_output_move (rtx[4], bool, machine_mode,
> RTX_CODE); #endif
>
> extern bool riscv_use_divmod_expander (void);
> +void riscv_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree,
> +int);
> +
> #endif /* ! GCC_RISCV_PROTOS_H */
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index
> de30bf4e567..dd5361c2bd2 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3795,6 +3795,99 @@ riscv_pass_fpr_pair (machine_mode mode, unsigned
> regno1,
> GEN_INT (offset2))));
> }
>
> +/* Use the TYPE_SIZE to distinguish the type with vector_size attribute
> and
> + intrinsic vector type. Because we can't get the decl for the
> +params. */
> +
> +static bool
> +riscv_scalable_vector_type_p (const_tree type) {
> + tree size = TYPE_SIZE (type);
> + if (size && TREE_CODE (size) == INTEGER_CST)
> + return false;
> +
> + /* For the data type like vint32m1_t, the size code is POLY_INT_CST.
> +*/
> + return true;
> +}
> +
> +static bool
> +riscv_arg_has_vector (const_tree type)
> +{
> + bool is_vector = false;
> +
> + switch (TREE_CODE (type))
> + {
> + case RECORD_TYPE:
> + if (!COMPLETE_TYPE_P (type))
> + break;
> +
> + for (tree f = TYPE_FIELDS (type); f; f = DECL_CHAIN (f))
> + if (TREE_CODE (f) == FIELD_DECL)
> + {
> + tree field_type = TREE_TYPE (f);
> + if (!TYPE_P (field_type))
> + break;
> +
> + /* Ignore it if it's fixed length vector. */
> + if (VECTOR_TYPE_P (field_type))
> + is_vector = riscv_scalable_vector_type_p (field_type);
> + else
> + is_vector = riscv_arg_has_vector (field_type);
> + }
> +
> + break;
> +
> + case VECTOR_TYPE:
> + is_vector = riscv_scalable_vector_type_p (type);
> + break;
> +
> + default:
> + is_vector = false;
> + break;
> + }
> +
> + return is_vector;
> +}
> +
> +/* Pass the type to check whether it's a vector type or contains vector
> type.
> + Only check the value type and no checking for vector pointer type.
> +*/
> +
> +static void
> +riscv_pass_in_vector_p (const_tree type) {
> + static int warned = 0;
> +
> + if (type && riscv_arg_has_vector (type) && !warned)
> + {
> + warning (OPT_Wpsabi, "ABI for the scalable vector type is currently
> in "
> + "experimental stage and may changes in the upcoming version of "
> + "GCC.");
> + warned = 1;
> + }
> +}
> +
> +/* Initialize a variable CUM of type CUMULATIVE_ARGS
> + for a call to a function whose data type is FNTYPE.
> + For a library call, FNTYPE is 0. */
> +
> +void
> +riscv_init_cumulative_args (CUMULATIVE_ARGS *cum,
> + tree fntype ATTRIBUTE_UNUSED,
> + rtx libname ATTRIBUTE_UNUSED,
> + tree fndecl,
> + int caller ATTRIBUTE_UNUSED)
> +{
> + memset (cum, 0, sizeof (*cum));
> +
> + if (fndecl)
> + {
> + const tree_function_decl &fn
> + = FUNCTION_DECL_CHECK (fndecl)->function_decl;
> +
> + if (fn.built_in_class == NOT_BUILT_IN)
> + cum->rvv_psabi_warning = 1;
> + }
> +}
> +
> /* Fill INFO with information about a single argument, and return an
> RTL pattern to pass or return the argument. CUM is the cumulative
> state for earlier arguments. MODE is the mode of this argument and @@
> -3816,6 +3909,12 @@ riscv_get_arg_info (struct riscv_arg_info *info, const
> CUMULATIVE_ARGS *cum,
> info->gpr_offset = cum->num_gprs;
> info->fpr_offset = cum->num_fprs;
>
> + if (cum->rvv_psabi_warning)
> + {
> + /* Only check existing of vector type. */
> + riscv_pass_in_vector_p (type);
> + }
> +
> /* TODO: Currently, it will cause an ICE for --param
> riscv-autovec-preference=fixed-vlmax. So, we just return NULL_RTX
> here
> let GCC generate loads/stores. Ideally, we should either warn the
> user not @@ -3973,7 +4072,18 @@ riscv_function_value (const_tree type,
> const_tree func, machine_mode mode)
> }
>
> memset (&args, 0, sizeof args);
> - return riscv_get_arg_info (&info, &args, mode, type, true, true);
> +
> + const_tree arg_type = type;
> + if (func && DECL_RESULT (func))
> + {
> + const tree_function_decl &fn = FUNCTION_DECL_CHECK (func)-
> >function_decl;
> + if (fn.built_in_class == NOT_BUILT_IN)
> + args.rvv_psabi_warning = 1;
> +
> + arg_type = TREE_TYPE (DECL_RESULT (func));
> + }
> +
> + return riscv_get_arg_info (&info, &args, mode, arg_type, true, true);
> }
>
> /* Implement TARGET_PASS_BY_REFERENCE. */ diff --git
> a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index
> 4541255a8ae..bfd9b7551bc 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -677,6 +677,8 @@ typedef struct {
>
> /* Number of floating-point registers used so far, likewise. */
> unsigned int num_fprs;
> +
> + int rvv_psabi_warning;
> } CUMULATIVE_ARGS;
>
> /* Initialize a variable CUM of type CUMULATIVE_ARGS @@ -684,7 +686,8 @@
> typedef struct {
> For a library call, FNTYPE is 0. */
>
> #define INIT_CUMULATIVE_ARGS(CUM, FNTYPE, LIBNAME, INDIRECT, N_NAMED_ARGS)
> \
> - memset (&(CUM), 0, sizeof (CUM))
> + riscv_init_cumulative_args (&(CUM), (FNTYPE), (LIBNAME), (INDIRECT),
> \
> + (N_NAMED_ARGS) != -1)
>
> #define EPILOGUE_USES(REGNO) riscv_epilogue_uses (REGNO)
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> index 5e69235a268..ad79d0e9a8d 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> @@ -43,7 +43,7 @@ dg-init
> # Main loop.
> set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
> - "" $CFLAGS
> + "-Wno-psabi" $CFLAGS
> gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]]
> \
> "" $CFLAGS
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
> diff --git a/gcc/testsuite/gcc.target/riscv/vector-abi-1.c
> b/gcc/testsuite/gcc.target/riscv/vector-abi-1.c
> new file mode 100644
> index 00000000000..969f14277a4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/vector-abi-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -march=rv64gcv -mabi=lp64d" } */
> +
> +#include "riscv_vector.h"
> +
> +void
> +fun (vint32m1_t a) { } /* { dg-warning "the scalable vector type" } */
> +
> +void
> +bar ()
> +{
> + vint32m1_t a;
> + fun (a);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/vector-abi-2.c
> b/gcc/testsuite/gcc.target/riscv/vector-abi-2.c
> new file mode 100644
> index 00000000000..63d97d30fc5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/vector-abi-2.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> +
> +#include "riscv_vector.h"
> +
> +vint32m1_t
> +fun (vint32m1_t* a) { return *a; } /* { dg-warning "the scalable
> +vector type" } */
> +
> +void
> +bar ()
> +{
> + vint32m1_t a;
> + fun (&a);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/vector-abi-3.c
> b/gcc/testsuite/gcc.target/riscv/vector-abi-3.c
> new file mode 100644
> index 00000000000..90ece60cc6f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/vector-abi-3.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
> +
> +#include "riscv_vector.h"
> +
> +vint32m1_t*
> +fun (vint32m1_t* a) { return a; } /* { dg-bogus "the scalable vector
> +type" } */
> +
> +void
> +bar ()
> +{
> + vint32m1_t a;
> + fun (&a);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/vector-abi-4.c
> b/gcc/testsuite/gcc.target/riscv/vector-abi-4.c
> new file mode 100644
> index 00000000000..ecf6d4cc26b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/vector-abi-4.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
> +
> +#include "riscv_vector.h"
> +
> +typedef int v4si __attribute__ ((vector_size (16)));
> +
> +v4si
> +fun (v4si a) { return a; } /* { dg-bogus "the scalable vector type" }
> +*/
> +
> +void
> +bar ()
> +{
> + v4si a;
> + fun (a);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/vector-abi-5.c
> b/gcc/testsuite/gcc.target/riscv/vector-abi-5.c
> new file mode 100644
> index 00000000000..6053e0783b6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/vector-abi-5.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
> +
> +typedef int v4si __attribute__ ((vector_size (16))); struct A { int a;
> +v4si b; };
> +
> +void
> +fun (struct A a) {} /* { dg-bogus "the scalable vector type" } */
> +
> +void
> +bar ()
> +{
> + struct A a;
> + fun (a);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/vector-abi-6.c
> b/gcc/testsuite/gcc.target/riscv/vector-abi-6.c
> new file mode 100644
> index 00000000000..63bc4a89805
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/vector-abi-6.c
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d" } */ #include
> +"riscv_vector.h"
> +
> +void
> +foo(int32_t *in1, int32_t *in2, int32_t *in3, int32_t *out,
> + size_t n, int cond) {
> + size_t vl;
> + if (cond)
> + vl = __riscv_vsetvlmax_e32m1();
> + else
> + vl = __riscv_vsetvlmax_e16mf2();
> + for (size_t i = 0; i < n; i += 1)
> + {
> + vint32m1_t a = __riscv_vle32_v_i32m1(in1, vl); /* { dg-bogus "the
> scalable vector type" } */
> + vint32m1_t b = __riscv_vle32_v_i32m1_tu(a, in2, vl);
> + vint32m1_t c = __riscv_vle32_v_i32m1_tu(b, in3, vl);
> + __riscv_vse32_v_i32m1(out, c, vl);
> + }
> +}
> --
> 2.40.1
Hi Yan-Zhang:
OK with one minor, go ahead IF the regression is clean.
Hi Pan:
Could you help to verify this patch and commit if the regression is clean?
thanks :)
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> index 5e69235a268..ad79d0e9a8d 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> @@ -43,7 +43,7 @@ dg-init
> # Main loop.
> set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
Add -Wno-psabi here rather than below, and also add it for
g++.target/riscv/rvv/rvv.exp
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
> - "" $CFLAGS
> + "-Wno-psabi" $CFLAGS
> gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]] \
> "" $CFLAGS
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
Sure thing, will commit it after all riscv.exp rvv.exp pass.
Pan
-----Original Message-----
From: Kito Cheng <kito.cheng@sifive.com>
Sent: Monday, June 12, 2023 8:43 PM
To: Wang, Yanzhang <yanzhang.wang@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>
Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
Hi Yan-Zhang:
OK with one minor, go ahead IF the regression is clean.
Hi Pan:
Could you help to verify this patch and commit if the regression is clean?
thanks :)
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> index 5e69235a268..ad79d0e9a8d 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> @@ -43,7 +43,7 @@ dg-init
> # Main loop.
> set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
Add -Wno-psabi here rather than below, and also add it for
g++.target/riscv/rvv/rvv.exp
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
> - "" $CFLAGS
> + "-Wno-psabi" $CFLAGS
> gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]] \
> "" $CFLAGS
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
I found that add the -Wno-psabi to CFLAGS will be overrode by
dg-options. It seems we can only add this option to the third
arg of dg-runtest. Attach the dg-runtest comments,
# dg-runtest -- simple main loop useful to most testsuites
#
# OPTIONS is a set of options to always pass.
# DEFAULT_EXTRA_OPTIONS is a set of options to pass if the testcase
# doesn't specify any (with dg-option).
> -----Original Message-----
> From: Kito Cheng <kito.cheng@sifive.com>
> Sent: Monday, June 12, 2023 8:43 PM
> To: Wang, Yanzhang <yanzhang.wang@intel.com>
> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Li, Pan2
> <pan2.li@intel.com>
> Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
>
> Hi Yan-Zhang:
>
> OK with one minor, go ahead IF the regression is clean.
>
> Hi Pan:
>
> Could you help to verify this patch and commit if the regression is clean?
>
> thanks :)
>
> > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > index 5e69235a268..ad79d0e9a8d 100644
> > --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > @@ -43,7 +43,7 @@ dg-init
> > # Main loop.
> > set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
>
> Add -Wno-psabi here rather than below, and also add it for
> g++.target/riscv/rvv/rvv.exp
>
> > dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
> > - "" $CFLAGS
> > + "-Wno-psabi" $CFLAGS
> > gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]]
> \
> > "" $CFLAGS
> > dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
How about appending to DEFAULT_CFLAGS?
On Mon, Jun 12, 2023 at 9:38 PM Wang, Yanzhang via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I found that add the -Wno-psabi to CFLAGS will be overrode by
> dg-options. It seems we can only add this option to the third
> arg of dg-runtest. Attach the dg-runtest comments,
>
> # dg-runtest -- simple main loop useful to most testsuites
> #
> # OPTIONS is a set of options to always pass.
> # DEFAULT_EXTRA_OPTIONS is a set of options to pass if the testcase
> # doesn't specify any (with dg-option).
>
> > -----Original Message-----
> > From: Kito Cheng <kito.cheng@sifive.com>
> > Sent: Monday, June 12, 2023 8:43 PM
> > To: Wang, Yanzhang <yanzhang.wang@intel.com>
> > Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Li, Pan2
> > <pan2.li@intel.com>
> > Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
> >
> > Hi Yan-Zhang:
> >
> > OK with one minor, go ahead IF the regression is clean.
> >
> > Hi Pan:
> >
> > Could you help to verify this patch and commit if the regression is clean?
> >
> > thanks :)
> >
> > > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > index 5e69235a268..ad79d0e9a8d 100644
> > > --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > @@ -43,7 +43,7 @@ dg-init
> > > # Main loop.
> > > set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
> >
> > Add -Wno-psabi here rather than below, and also add it for
> > g++.target/riscv/rvv/rvv.exp
> >
> > > dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
> > > - "" $CFLAGS
> > > + "-Wno-psabi" $CFLAGS
> > > gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]]
> > \
> > > "" $CFLAGS
> > > dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
It's the same behavior. Because the DEFAULT_CFLAGS will be copied to
CFLAGS and then passed as the DEFAULT_EXTRA_OPTIONS to dg-runtest.
> -----Original Message-----
> From: Kito Cheng <kito.cheng@gmail.com>
> Sent: Monday, June 12, 2023 10:08 PM
> To: Wang, Yanzhang <yanzhang.wang@intel.com>
> Cc: Kito Cheng <kito.cheng@sifive.com>; gcc-patches@gcc.gnu.org;
> juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>
> Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
>
> How about appending to DEFAULT_CFLAGS?
>
> On Mon, Jun 12, 2023 at 9:38 PM Wang, Yanzhang via Gcc-patches <gcc-
> patches@gcc.gnu.org> wrote:
> >
> > I found that add the -Wno-psabi to CFLAGS will be overrode by
> > dg-options. It seems we can only add this option to the third arg of
> > dg-runtest. Attach the dg-runtest comments,
> >
> > # dg-runtest -- simple main loop useful to most testsuites # # OPTIONS
> > is a set of options to always pass.
> > # DEFAULT_EXTRA_OPTIONS is a set of options to pass if the testcase #
> > doesn't specify any (with dg-option).
> >
> > > -----Original Message-----
> > > From: Kito Cheng <kito.cheng@sifive.com>
> > > Sent: Monday, June 12, 2023 8:43 PM
> > > To: Wang, Yanzhang <yanzhang.wang@intel.com>
> > > Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Li, Pan2
> > > <pan2.li@intel.com>
> > > Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
> > >
> > > Hi Yan-Zhang:
> > >
> > > OK with one minor, go ahead IF the regression is clean.
> > >
> > > Hi Pan:
> > >
> > > Could you help to verify this patch and commit if the regression is
> clean?
> > >
> > > thanks :)
> > >
> > > > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > > index 5e69235a268..ad79d0e9a8d 100644
> > > > --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > > +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> > > > @@ -43,7 +43,7 @@ dg-init
> > > > # Main loop.
> > > > set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
> > >
> > > Add -Wno-psabi here rather than below, and also add it for
> > > g++.target/riscv/rvv/rvv.exp
> > >
> > > > dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]]
> \
> > > > - "" $CFLAGS
> > > > + "-Wno-psabi" $CFLAGS
> > > > gcc-dg-runtest [lsort [glob -nocomplain
> > > > $srcdir/$subdir/vsetvl/*.\[cS\]]]
> > > \
> > > > "" $CFLAGS
> > > > dg-runtest [lsort [glob -nocomplain
> > > > $srcdir/$subdir/autovec/*.\[cS\]]] \
On 6/12/23 07:36, Wang, Yanzhang via Gcc-patches wrote:
> I found that add the -Wno-psabi to CFLAGS will be overrode by
> dg-options. It seems we can only add this option to the third
> arg of dg-runtest. Attach the dg-runtest comments,
I think we default to -Wno-psabi to avoid triggering diagnostics in the
common case where we aren't concerned about such issues. So not a
surprise that we'll need to work a bit harder to get it added when we do
want to check for psabi issues.
jeff
Hmmm, yeah, I think let's add it case by case...I assume we should get
it rid before GCC 14, it is mostly used for the transition period
before we settle down the ABI and for GCC 13.
On Mon, Jun 12, 2023 at 10:34 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 6/12/23 07:36, Wang, Yanzhang via Gcc-patches wrote:
> > I found that add the -Wno-psabi to CFLAGS will be overrode by
> > dg-options. It seems we can only add this option to the third
> > arg of dg-runtest. Attach the dg-runtest comments,
> I think we default to -Wno-psabi to avoid triggering diagnostics in the
> common case where we aren't concerned about such issues. So not a
> surprise that we'll need to work a bit harder to get it added when we do
> want to check for psabi issues.
>
> jeff
I think it's ok to add it to specific cases and will not affect other cases.
There's not so many cases. And we can revert it after the finalization.
> -----Original Message-----
> From: Kito Cheng <kito.cheng@sifive.com>
> Sent: Monday, June 12, 2023 10:53 PM
> To: Jeff Law <jeffreyalaw@gmail.com>
> Cc: Wang, Yanzhang <yanzhang.wang@intel.com>; gcc-patches@gcc.gnu.org;
> juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>
> Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
>
> Hmmm, yeah, I think let's add it case by case...I assume we should get it
> rid before GCC 14, it is mostly used for the transition period before we
> settle down the ABI and for GCC 13.
>
> On Mon, Jun 12, 2023 at 10:34 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> >
> >
> >
> > On 6/12/23 07:36, Wang, Yanzhang via Gcc-patches wrote:
> > > I found that add the -Wno-psabi to CFLAGS will be overrode by
> > > dg-options. It seems we can only add this option to the third arg of
> > > dg-runtest. Attach the dg-runtest comments,
> > I think we default to -Wno-psabi to avoid triggering diagnostics in
> > the common case where we aren't concerned about such issues. So not a
> > surprise that we'll need to work a bit harder to get it added when we
> > do want to check for psabi issues.
> >
> > jeff
Committed v6 with riscv.exp and rvv.exp passed, thanks Kito.
Pan
-----Original Message-----
From: Li, Pan2
Sent: Monday, June 12, 2023 8:49 PM
To: Kito Cheng <kito.cheng@sifive.com>; Wang, Yanzhang <yanzhang.wang@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai
Subject: RE: [PATCH v5] RISC-V: Add vector psabi checking.
Sure thing, will commit it after all riscv.exp rvv.exp pass.
Pan
-----Original Message-----
From: Kito Cheng <kito.cheng@sifive.com>
Sent: Monday, June 12, 2023 8:43 PM
To: Wang, Yanzhang <yanzhang.wang@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>
Subject: Re: [PATCH v5] RISC-V: Add vector psabi checking.
Hi Yan-Zhang:
OK with one minor, go ahead IF the regression is clean.
Hi Pan:
Could you help to verify this patch and commit if the regression is clean?
thanks :)
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> index 5e69235a268..ad79d0e9a8d 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/rvv.exp
> @@ -43,7 +43,7 @@ dg-init
> # Main loop.
> set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
Add -Wno-psabi here rather than below, and also add it for
g++.target/riscv/rvv/rvv.exp
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
> - "" $CFLAGS
> + "-Wno-psabi" $CFLAGS
> gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]] \
> "" $CFLAGS
> dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
@@ -302,4 +302,6 @@ th_mempair_output_move (rtx[4], bool, machine_mode, RTX_CODE);
#endif
extern bool riscv_use_divmod_expander (void);
+void riscv_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);
+
#endif /* ! GCC_RISCV_PROTOS_H */
@@ -3795,6 +3795,99 @@ riscv_pass_fpr_pair (machine_mode mode, unsigned regno1,
GEN_INT (offset2))));
}
+/* Use the TYPE_SIZE to distinguish the type with vector_size attribute and
+ intrinsic vector type. Because we can't get the decl for the params. */
+
+static bool
+riscv_scalable_vector_type_p (const_tree type)
+{
+ tree size = TYPE_SIZE (type);
+ if (size && TREE_CODE (size) == INTEGER_CST)
+ return false;
+
+ /* For the data type like vint32m1_t, the size code is POLY_INT_CST. */
+ return true;
+}
+
+static bool
+riscv_arg_has_vector (const_tree type)
+{
+ bool is_vector = false;
+
+ switch (TREE_CODE (type))
+ {
+ case RECORD_TYPE:
+ if (!COMPLETE_TYPE_P (type))
+ break;
+
+ for (tree f = TYPE_FIELDS (type); f; f = DECL_CHAIN (f))
+ if (TREE_CODE (f) == FIELD_DECL)
+ {
+ tree field_type = TREE_TYPE (f);
+ if (!TYPE_P (field_type))
+ break;
+
+ /* Ignore it if it's fixed length vector. */
+ if (VECTOR_TYPE_P (field_type))
+ is_vector = riscv_scalable_vector_type_p (field_type);
+ else
+ is_vector = riscv_arg_has_vector (field_type);
+ }
+
+ break;
+
+ case VECTOR_TYPE:
+ is_vector = riscv_scalable_vector_type_p (type);
+ break;
+
+ default:
+ is_vector = false;
+ break;
+ }
+
+ return is_vector;
+}
+
+/* Pass the type to check whether it's a vector type or contains vector type.
+ Only check the value type and no checking for vector pointer type. */
+
+static void
+riscv_pass_in_vector_p (const_tree type)
+{
+ static int warned = 0;
+
+ if (type && riscv_arg_has_vector (type) && !warned)
+ {
+ warning (OPT_Wpsabi, "ABI for the scalable vector type is currently in "
+ "experimental stage and may changes in the upcoming version of "
+ "GCC.");
+ warned = 1;
+ }
+}
+
+/* Initialize a variable CUM of type CUMULATIVE_ARGS
+ for a call to a function whose data type is FNTYPE.
+ For a library call, FNTYPE is 0. */
+
+void
+riscv_init_cumulative_args (CUMULATIVE_ARGS *cum,
+ tree fntype ATTRIBUTE_UNUSED,
+ rtx libname ATTRIBUTE_UNUSED,
+ tree fndecl,
+ int caller ATTRIBUTE_UNUSED)
+{
+ memset (cum, 0, sizeof (*cum));
+
+ if (fndecl)
+ {
+ const tree_function_decl &fn
+ = FUNCTION_DECL_CHECK (fndecl)->function_decl;
+
+ if (fn.built_in_class == NOT_BUILT_IN)
+ cum->rvv_psabi_warning = 1;
+ }
+}
+
/* Fill INFO with information about a single argument, and return an
RTL pattern to pass or return the argument. CUM is the cumulative
state for earlier arguments. MODE is the mode of this argument and
@@ -3816,6 +3909,12 @@ riscv_get_arg_info (struct riscv_arg_info *info, const CUMULATIVE_ARGS *cum,
info->gpr_offset = cum->num_gprs;
info->fpr_offset = cum->num_fprs;
+ if (cum->rvv_psabi_warning)
+ {
+ /* Only check existing of vector type. */
+ riscv_pass_in_vector_p (type);
+ }
+
/* TODO: Currently, it will cause an ICE for --param
riscv-autovec-preference=fixed-vlmax. So, we just return NULL_RTX here
let GCC generate loads/stores. Ideally, we should either warn the user not
@@ -3973,7 +4072,18 @@ riscv_function_value (const_tree type, const_tree func, machine_mode mode)
}
memset (&args, 0, sizeof args);
- return riscv_get_arg_info (&info, &args, mode, type, true, true);
+
+ const_tree arg_type = type;
+ if (func && DECL_RESULT (func))
+ {
+ const tree_function_decl &fn = FUNCTION_DECL_CHECK (func)->function_decl;
+ if (fn.built_in_class == NOT_BUILT_IN)
+ args.rvv_psabi_warning = 1;
+
+ arg_type = TREE_TYPE (DECL_RESULT (func));
+ }
+
+ return riscv_get_arg_info (&info, &args, mode, arg_type, true, true);
}
/* Implement TARGET_PASS_BY_REFERENCE. */
@@ -677,6 +677,8 @@ typedef struct {
/* Number of floating-point registers used so far, likewise. */
unsigned int num_fprs;
+
+ int rvv_psabi_warning;
} CUMULATIVE_ARGS;
/* Initialize a variable CUM of type CUMULATIVE_ARGS
@@ -684,7 +686,8 @@ typedef struct {
For a library call, FNTYPE is 0. */
#define INIT_CUMULATIVE_ARGS(CUM, FNTYPE, LIBNAME, INDIRECT, N_NAMED_ARGS) \
- memset (&(CUM), 0, sizeof (CUM))
+ riscv_init_cumulative_args (&(CUM), (FNTYPE), (LIBNAME), (INDIRECT), \
+ (N_NAMED_ARGS) != -1)
#define EPILOGUE_USES(REGNO) riscv_epilogue_uses (REGNO)
@@ -43,7 +43,7 @@ dg-init
# Main loop.
set CFLAGS "$DEFAULT_CFLAGS -march=$gcc_march -mabi=$gcc_mabi -O3"
dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/base/*.\[cS\]]] \
- "" $CFLAGS
+ "-Wno-psabi" $CFLAGS
gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/vsetvl/*.\[cS\]]] \
"" $CFLAGS
dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/autovec/*.\[cS\]]] \
new file mode 100644
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O0 -march=rv64gcv -mabi=lp64d" } */
+
+#include "riscv_vector.h"
+
+void
+fun (vint32m1_t a) { } /* { dg-warning "the scalable vector type" } */
+
+void
+bar ()
+{
+ vint32m1_t a;
+ fun (a);
+}
new file mode 100644
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
+
+#include "riscv_vector.h"
+
+vint32m1_t
+fun (vint32m1_t* a) { return *a; } /* { dg-warning "the scalable vector type" } */
+
+void
+bar ()
+{
+ vint32m1_t a;
+ fun (&a);
+}
new file mode 100644
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
+
+#include "riscv_vector.h"
+
+vint32m1_t*
+fun (vint32m1_t* a) { return a; } /* { dg-bogus "the scalable vector type" } */
+
+void
+bar ()
+{
+ vint32m1_t a;
+ fun (&a);
+}
new file mode 100644
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
+
+#include "riscv_vector.h"
+
+typedef int v4si __attribute__ ((vector_size (16)));
+
+v4si
+fun (v4si a) { return a; } /* { dg-bogus "the scalable vector type" } */
+
+void
+bar ()
+{
+ v4si a;
+ fun (a);
+}
new file mode 100644
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
+
+typedef int v4si __attribute__ ((vector_size (16)));
+struct A { int a; v4si b; };
+
+void
+fun (struct A a) {} /* { dg-bogus "the scalable vector type" } */
+
+void
+bar ()
+{
+ struct A a;
+ fun (a);
+}
new file mode 100644
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d" } */
+#include "riscv_vector.h"
+
+void
+foo(int32_t *in1, int32_t *in2, int32_t *in3, int32_t *out,
+ size_t n, int cond) {
+ size_t vl;
+ if (cond)
+ vl = __riscv_vsetvlmax_e32m1();
+ else
+ vl = __riscv_vsetvlmax_e16mf2();
+ for (size_t i = 0; i < n; i += 1)
+ {
+ vint32m1_t a = __riscv_vle32_v_i32m1(in1, vl); /* { dg-bogus "the scalable vector type" } */
+ vint32m1_t b = __riscv_vle32_v_i32m1_tu(a, in2, vl);
+ vint32m1_t c = __riscv_vle32_v_i32m1_tu(b, in3, vl);
+ __riscv_vse32_v_i32m1(out, c, vl);
+ }
+}