RISC-V: Ensure vector args and return use function stack to pass [PR110119]

  Hi,

The reason for this bug is that in the case where the vector register is set
to a fixed length (with `--param=riscv-autovec-preference=fixed-vlmax` option),
TARGET_PASS_BY_REFERENCE thinks that variables of type vint32m1 can be passed
through two scalar registers, but when GCC calls FUNCTION_VALUE (call function
riscv_get_arg_info inside) it returns NULL_RTX. These two functions are not
unified. The current treatment is to pass all vector arguments and returns
through the function stack, and a new calling convention for vector registers
will be added in the future.

Best,
Lehua

  PR target/110119

gcc/ChangeLog:

        * config/riscv/riscv.cc (riscv_get_arg_info): Return NULL_RTX for vector mode
        (riscv_pass_by_reference): Return true for vector mode

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/rvv/base/p110119-1.c: New test.
        * gcc.target/riscv/rvv/base/p110119-2.c: New test.

---
 gcc/config/riscv/riscv.cc                     | 19 +++++++++-----
 .../gcc.target/riscv/rvv/base/p110119-1.c     | 26 +++++++++++++++++++
 .../gcc.target/riscv/rvv/base/p110119-2.c     | 26 +++++++++++++++++++
 3 files changed, 65 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
  

Thanks for fixing this.

This patch let RVV type (both vector and tuple) return in memory by default when there is no vector ABI support.
It makes sens to me.

CC more RISC-V folks to comments.

Thanks.


juzhe.zhong@rivai.ai
 
From: Lehua Ding
Date: 2023-06-14 19:03
To: gcc-patches; juzhe.zhong
Subject: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]
Hi,
 
The reason for this bug is that in the case where the vector register is set
to a fixed length (with `--param=riscv-autovec-preference=fixed-vlmax` option),
TARGET_PASS_BY_REFERENCE thinks that variables of type vint32m1 can be passed
through two scalar registers, but when GCC calls FUNCTION_VALUE (call function
riscv_get_arg_info inside) it returns NULL_RTX. These two functions are not
unified. The current treatment is to pass all vector arguments and returns
through the function stack, and a new calling convention for vector registers
will be added in the future.
 
Best,
Lehua
 
  PR target/110119
 
gcc/ChangeLog:
 
        * config/riscv/riscv.cc (riscv_get_arg_info): Return NULL_RTX for vector mode
        (riscv_pass_by_reference): Return true for vector mode
 
gcc/testsuite/ChangeLog:
 
        * gcc.target/riscv/rvv/base/p110119-1.c: New test.
        * gcc.target/riscv/rvv/base/p110119-2.c: New test.
 
---
gcc/config/riscv/riscv.cc                     | 19 +++++++++-----
.../gcc.target/riscv/rvv/base/p110119-1.c     | 26 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/p110119-2.c     | 26 +++++++++++++++++++
3 files changed, 65 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dd5361c2bd2a..be868c7b6127 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3915,13 +3915,13 @@ riscv_get_arg_info (struct riscv_arg_info *info, const CUMULATIVE_ARGS *cum,
       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
-     to use an RVV vector type as function argument or support the calling
-     convention directly.  */
-  if (riscv_v_ext_mode_p (mode))
+  /* All current vector arguments and return values are passed through the
+     function stack. Ideally, we should either warn the user not to use an RVV
+     vector type as function argument or support a calling convention
+     with better performance.  */
+  if (riscv_v_ext_mode_p (mode) || riscv_v_ext_tuple_mode_p (mode))
     return NULL_RTX;
+
   if (named)
     {
       riscv_aggregate_field fields[2];
@@ -4106,6 +4106,13 @@ riscv_pass_by_reference (cumulative_args_t cum_v, const function_arg_info &arg)
return false;
     }
+  /* All current vector arguments and return values are passed through the
+     function stack. Ideally, we should either warn the user not to use an RVV
+     vector type as function argument or support a calling convention
+     with better performance.  */
+  if (riscv_v_ext_mode_p (arg.mode) || riscv_v_ext_tuple_mode_p (arg.mode))
+    return true;
+
   /* Pass by reference if the data do not fit in two integer registers.  */
   return !IN_RANGE (size, 0, 2 * UNITS_PER_WORD);
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
new file mode 100644
index 000000000000..0edbb0626299
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv --param=riscv-autovec-preference=fixed-vlmax" } */
+
+#include "riscv_vector.h"
+
+typedef int8_t vnx2qi __attribute__ ((vector_size (2)));
+
+__attribute__ ((noipa)) vnx2qi
+f_vnx2qi (int8_t a, int8_t b, int8_t *out)
+{
+  vnx2qi v = {a, b};
+  return v;
+}
+
+__attribute__ ((noipa)) vnx2qi
+f_vnx2qi_2 (vnx2qi a, int8_t *out)
+{
+  return a;
+}
+
+__attribute__ ((noipa)) vint32m1_t
+f_vint32m1 (int8_t * a, int8_t *out)
+{
+  vint32m1_t v = *(vint32m1_t*)a;
+  return v;
+}
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
new file mode 100644
index 000000000000..b233ff1e9040
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gczve32x --param=riscv-autovec-preference=fixed-vlmax" } */
+
+#include <stdint.h>
+#include "riscv_vector.h"
+
+__attribute__ ((noipa)) vint32m1x3_t
+foo1 (int32_t *in, int vl)
+{
+  vint32m1x3_t v = __riscv_vlseg3e32_v_i32m1x3 (in, vl);
+  return v;
+}
+
+__attribute__ ((noipa)) void
+foo2 (vint32m1x3_t a, int32_t *out, int vl)
+{
+  __riscv_vsseg3e32_v_i32m1x3 (out, a, vl);
+}
+
+__attribute__ ((noipa)) vint32m1x3_t
+foo3 (vint32m1x3_t a, int32_t *out, int32_t *in, int vl)
+{
+  __riscv_vsseg3e32_v_i32m1x3 (out, a, vl);
+  vint32m1x3_t v = __riscv_vlseg3e32_v_i32m1x3 (in, vl);
+  return v;
+}
-- 
2.36.3
  

Oh. I see.

Change  if (riscv_v_ext_mode_p (arg.mode) || riscv_v_ext_tuple_mode_p (arg.mode))

into 

if (riscv_v_ext_mode_p (arg.mode))

since riscv_v_ext_mode_p (arg.mode) includes riscv_v_ext_vector_mode_p (arg.mode) and riscv_v_ext_tuple_mode_p (arg.mode)

no need has riscv_v_ext_tuple_mode_p


juzhe.zhong@rivai.ai
 
From: Lehua Ding
Date: 2023-06-14 19:03
To: gcc-patches; juzhe.zhong
Subject: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]
Hi,
 
The reason for this bug is that in the case where the vector register is set
to a fixed length (with `--param=riscv-autovec-preference=fixed-vlmax` option),
TARGET_PASS_BY_REFERENCE thinks that variables of type vint32m1 can be passed
through two scalar registers, but when GCC calls FUNCTION_VALUE (call function
riscv_get_arg_info inside) it returns NULL_RTX. These two functions are not
unified. The current treatment is to pass all vector arguments and returns
through the function stack, and a new calling convention for vector registers
will be added in the future.
 
Best,
Lehua
 
  PR target/110119
 
gcc/ChangeLog:
 
        * config/riscv/riscv.cc (riscv_get_arg_info): Return NULL_RTX for vector mode
        (riscv_pass_by_reference): Return true for vector mode
 
gcc/testsuite/ChangeLog:
 
        * gcc.target/riscv/rvv/base/p110119-1.c: New test.
        * gcc.target/riscv/rvv/base/p110119-2.c: New test.
 
---
gcc/config/riscv/riscv.cc                     | 19 +++++++++-----
.../gcc.target/riscv/rvv/base/p110119-1.c     | 26 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/p110119-2.c     | 26 +++++++++++++++++++
3 files changed, 65 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dd5361c2bd2a..be868c7b6127 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3915,13 +3915,13 @@ riscv_get_arg_info (struct riscv_arg_info *info, const CUMULATIVE_ARGS *cum,
       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
-     to use an RVV vector type as function argument or support the calling
-     convention directly.  */
-  if (riscv_v_ext_mode_p (mode))
+  /* All current vector arguments and return values are passed through the
+     function stack. Ideally, we should either warn the user not to use an RVV
+     vector type as function argument or support a calling convention
+     with better performance.  */
+  if (riscv_v_ext_mode_p (mode) || riscv_v_ext_tuple_mode_p (mode))
     return NULL_RTX;
+
   if (named)
     {
       riscv_aggregate_field fields[2];
@@ -4106,6 +4106,13 @@ riscv_pass_by_reference (cumulative_args_t cum_v, const function_arg_info &arg)
return false;
     }
+  /* All current vector arguments and return values are passed through the
+     function stack. Ideally, we should either warn the user not to use an RVV
+     vector type as function argument or support a calling convention
+     with better performance.  */
+  if (riscv_v_ext_mode_p (arg.mode) || riscv_v_ext_tuple_mode_p (arg.mode))
+    return true;
+
   /* Pass by reference if the data do not fit in two integer registers.  */
   return !IN_RANGE (size, 0, 2 * UNITS_PER_WORD);
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
new file mode 100644
index 000000000000..0edbb0626299
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv --param=riscv-autovec-preference=fixed-vlmax" } */
+
+#include "riscv_vector.h"
+
+typedef int8_t vnx2qi __attribute__ ((vector_size (2)));
+
+__attribute__ ((noipa)) vnx2qi
+f_vnx2qi (int8_t a, int8_t b, int8_t *out)
+{
+  vnx2qi v = {a, b};
+  return v;
+}
+
+__attribute__ ((noipa)) vnx2qi
+f_vnx2qi_2 (vnx2qi a, int8_t *out)
+{
+  return a;
+}
+
+__attribute__ ((noipa)) vint32m1_t
+f_vint32m1 (int8_t * a, int8_t *out)
+{
+  vint32m1_t v = *(vint32m1_t*)a;
+  return v;
+}
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
new file mode 100644
index 000000000000..b233ff1e9040
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gczve32x --param=riscv-autovec-preference=fixed-vlmax" } */
+
+#include <stdint.h>
+#include "riscv_vector.h"
+
+__attribute__ ((noipa)) vint32m1x3_t
+foo1 (int32_t *in, int vl)
+{
+  vint32m1x3_t v = __riscv_vlseg3e32_v_i32m1x3 (in, vl);
+  return v;
+}
+
+__attribute__ ((noipa)) void
+foo2 (vint32m1x3_t a, int32_t *out, int vl)
+{
+  __riscv_vsseg3e32_v_i32m1x3 (out, a, vl);
+}
+
+__attribute__ ((noipa)) vint32m1x3_t
+foo3 (vint32m1x3_t a, int32_t *out, int32_t *in, int vl)
+{
+  __riscv_vsseg3e32_v_i32m1x3 (out, a, vl);
+  vint32m1x3_t v = __riscv_vlseg3e32_v_i32m1x3 (in, vl);
+  return v;
+}
-- 
2.36.3
  

Hi,

> Thanks for fixing this.
> 
> This patch let RVV type (both vector and tuple) return in memory by
> default when there is no vector ABI support. It makes sens to me.
> 
> CC more RISC-V folks to comments.

so this is intended to fix the PR as well as unblock while we continue
with the preliminary ABI separately?

If so, works for me.

Regards
 Robin
  

> so this is intended to fix the PR as well as unblock while we continue
> with the preliminary ABI separately?


Yes, and I will send the new prerelease vector calling convention later.


Best,
Lehua