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

  Also
p110119-1.c
change name of test into
pr110119-1.c


juzhe.zhong@rivai.ai
 
发件人： juzhe.zhong@rivai.ai
发送时间： 2023-06-14 19:17
收件人： 丁乐华; gcc-patches
抄送： jeffreyalaw; Robin Dapp; palmer
主题： Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]
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
 
-- 
2.36.3
  

Fix all comment from Juzhe, thanks. Below is the new patch. Please use the
attachment if there is a problem with the format of the patch below.



        PR 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/pr110119-1.c: New test.
        * gcc.target/riscv/rvv/base/pr110119-2.c: New test.



---
 gcc/config/riscv/riscv.cc                     | 17 ++++++++----
 .../gcc.target/riscv/rvv/base/pr110119-1.c    | 26 +++++++++++++++++++
 .../gcc.target/riscv/rvv/base/pr110119-2.c    | 26 +++++++++++++++++++
 3 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-2.c

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dd5361c2bd2a..e5ae4e81b7a5 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.  */
+  /* 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))
     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))
+    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/pr110119-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-1.c
new file mode 100644
index 000000000000..0edbb0626299
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-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/pr110119-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-2.c
new file mode 100644
index 000000000000..b233ff1e9040
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-2.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gczve32x --param=riscv-autovec-preference=fixed-vlmax" } */
+
+#include <stdint.h&gt;
+#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


&nbsp;
&nbsp;
------------------&nbsp;Original&nbsp;------------------
From: &nbsp;"juzhe.zhong@rivai.ai"<juzhe.zhong@rivai.ai&gt;;
Date: &nbsp;Wed, Jun 14, 2023 07:20 PM
To: &nbsp;"丁乐华"<lehua.ding@rivai.ai&gt;; "gcc-patches"<gcc-patches@gcc.gnu.org&gt;; 
Cc: &nbsp;"Jeff Law"<jeffreyalaw@gmail.com&gt;; "Robin Dapp"<rdapp.gcc@gmail.com&gt;; "palmer"<palmer@rivosinc.com&gt;; 
Subject: &nbsp;回复: Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]

&nbsp;

 Also
 p110119-1.c
change name of test into
pr110119-1.c

 juzhe.zhong@rivai.ai


 &nbsp;
发件人：&nbsp;juzhe.zhong@rivai.ai
发送时间：&nbsp;2023-06-14 19:17
收件人：&nbsp;丁乐华; gcc-patches
抄送：&nbsp;jeffreyalaw; Robin Dapp; palmer
主题：&nbsp;Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]


 Oh. I see.


Change &nbsp;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


 &nbsp;
From:&nbsp;Lehua Ding
Date:&nbsp;2023-06-14 19:03
To:&nbsp;gcc-patches; juzhe.zhong
Subject:&nbsp;[PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]


Hi,
 &nbsp;
 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.
 &nbsp;
 Best,
 Lehua
 &nbsp;
 &nbsp; PR target/110119
 &nbsp;
 gcc/ChangeLog:
 &nbsp;
 &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; * config/riscv/riscv.cc (riscv_get_arg_info): Return NULL_RTX for vector mode
 &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; (riscv_pass_by_reference): Return true for vector mode
 &nbsp;
 gcc/testsuite/ChangeLog:
 &nbsp;
 &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; * gcc.target/riscv/rvv/base/p110119-1.c: New test.
 &nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; * gcc.target/riscv/rvv/base/p110119-2.c: New test.
 &nbsp;
 ---
  gcc/config/riscv/riscv.cc&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; &nbsp; | 19 +++++++++-----
  .../gcc.target/riscv/rvv/base/p110119-1.c&nbsp;&nbsp; &nbsp; | 26 +++++++++++++++++++
  .../gcc.target/riscv/rvv/base/p110119-2.c&nbsp;&nbsp; &nbsp; | 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
 &nbsp;
 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,
 &nbsp;&nbsp;&nbsp;&nbsp; &nbsp; riscv_pass_in_vector_p (type);
 &nbsp;&nbsp; &nbsp; }
  
 -&nbsp; /* TODO: Currently, it will cause an ICE for --param
 -&nbsp;&nbsp; &nbsp; riscv-autovec-preference=fixed-vlmax. So, we just return NULL_RTX here
 -&nbsp;&nbsp; &nbsp; let GCC generate loads/stores. Ideally, we should either warn the user not
 -&nbsp;&nbsp; &nbsp; to use an RVV vector type as function argument or support the calling
 -&nbsp;&nbsp; &nbsp; convention directly.&nbsp; */
 -&nbsp; if (riscv_v_ext_mode_p (mode))
 +&nbsp; /* All current vector arguments and return values are passed through the
 +&nbsp;&nbsp; &nbsp; function stack. Ideally, we should either warn the user not to use an RVV
 +&nbsp;&nbsp; &nbsp; vector type as function argument or support a calling convention
 +&nbsp;&nbsp; &nbsp; with better performance.&nbsp; */
 +&nbsp; if (riscv_v_ext_mode_p (mode) || riscv_v_ext_tuple_mode_p (mode))
 &nbsp;&nbsp; &nbsp; return NULL_RTX;
 +
 &nbsp;&nbsp; if (named)
 &nbsp;&nbsp; &nbsp; {
 &nbsp;&nbsp;&nbsp;&nbsp; &nbsp; riscv_aggregate_field fields[2];
 @@ -4106,6 +4106,13 @@ riscv_pass_by_reference (cumulative_args_t cum_v, const function_arg_info &amp;arg)
  	return false;
 &nbsp;&nbsp; &nbsp; }
  
 +&nbsp; /* All current vector arguments and return values are passed through the
 +&nbsp;&nbsp; &nbsp; function stack. Ideally, we should either warn the user not to use an RVV
 +&nbsp;&nbsp; &nbsp; vector type as function argument or support a calling convention
 +&nbsp;&nbsp; &nbsp; with better performance.&nbsp; */
 +&nbsp; if (riscv_v_ext_mode_p (arg.mode) || riscv_v_ext_tuple_mode_p (arg.mode))
 +&nbsp; &nbsp; return true;
 +
 &nbsp;&nbsp; /* Pass by reference if the data do not fit in two integer registers.&nbsp; */
 &nbsp;&nbsp; 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)
 +{
 +&nbsp; vnx2qi v = {a, b};
 +&nbsp; return v;
 +}
 +
 +__attribute__ ((noipa)) vnx2qi
 +f_vnx2qi_2 (vnx2qi a, int8_t *out)
 +{
 +&nbsp; return a;
 +}
 +
 +__attribute__ ((noipa)) vint32m1_t
 +f_vint32m1 (int8_t * a, int8_t *out)
 +{
 +&nbsp; vint32m1_t v = *(vint32m1_t*)a;
 +&nbsp; 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&gt;
 +#include "riscv_vector.h"
 +
 +__attribute__ ((noipa)) vint32m1x3_t
 +foo1 (int32_t *in, int vl)
 +{
 +&nbsp; vint32m1x3_t v = __riscv_vlseg3e32_v_i32m1x3 (in, vl);
 +&nbsp; return v;
 +}
 +
 +__attribute__ ((noipa)) void
 +foo2 (vint32m1x3_t a, int32_t *out, int vl)
 +{
 +&nbsp; __riscv_vsseg3e32_v_i32m1x3 (out, a, vl);
 +}
 +
 +__attribute__ ((noipa)) vint32m1x3_t
 +foo3 (vint32m1x3_t a, int32_t *out, int32_t *in, int vl)
 +{
 +&nbsp; __riscv_vsseg3e32_v_i32m1x3 (out, a, vl);
 +&nbsp; vint32m1x3_t v = __riscv_vlseg3e32_v_i32m1x3 (in, vl);
 +&nbsp; return v;
 +}
 -- 
 2.36.3
 &nbsp;
  

\ No newline at end of file

Add newline for each test.


juzhe.zhong@rivai.ai
 
From: Lehua Ding
Date: 2023-06-14 19:33
To: 钟居哲; gcc-patches
CC: Jeff Law; Robin Dapp; palmer
Subject: Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]
Fix all comment from Juzhe, thanks. Below is the new patch. Please use the
attachment if there is a problem with the format of the patch below.

PR 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/pr110119-1.c: New test.
* gcc.target/riscv/rvv/base/pr110119-2.c: New test.

---
gcc/config/riscv/riscv.cc | 17 ++++++++----
.../gcc.target/riscv/rvv/base/pr110119-1.c | 26 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr110119-2.c | 26 +++++++++++++++++++
3 files changed, 64 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-2.c
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dd5361c2bd2a..e5ae4e81b7a5 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. */
+ /* 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))
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))
+ 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/pr110119-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-1.c
new file mode 100644
index 000000000000..0edbb0626299
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-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/pr110119-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-2.c
new file mode 100644
index 000000000000..b233ff1e9040
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110119-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
 
 
------------------ Original ------------------
From:  "juzhe.zhong@rivai.ai"<juzhe.zhong@rivai.ai>;
Date:  Wed, Jun 14, 2023 07:20 PM
To:  "丁乐华"<lehua.ding@rivai.ai>; "gcc-patches"<gcc-patches@gcc.gnu.org>; 
Cc:  "Jeff Law"<jeffreyalaw@gmail.com>; "Robin Dapp"<rdapp.gcc@gmail.com>; "palmer"<palmer@rivosinc.com>; 
Subject:  回复: Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]
 
Also
p110119-1.c
change name of test into
pr110119-1.c


juzhe.zhong@rivai.ai
 
发件人： juzhe.zhong@rivai.ai
发送时间： 2023-06-14 19:17
收件人： 丁乐华; gcc-patches
抄送： jeffreyalaw; Robin Dapp; palmer
主题： Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]
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
  

> \ No newline at end of file
> Add newline for each test.



Address this comment, below is the V2 patch link.


https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621698.html
 
Best,
Lehua


  
  

Nit for test.

+/* { dg-options "-march=rv64gczve32x 
+--param=riscv-autovec-preference=fixed-vlmax" } */

To

+/* { dg-options "-march=rv64gc_zve32x --param=riscv-autovec-preference=fixed-vlmax" } */

Pan

-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+pan2.li=intel.com@gcc.gnu.org> On Behalf Of juzhe.zhong@rivai.ai
Sent: Wednesday, June 14, 2023 7:21 PM
To: 丁乐华 <lehua.ding@rivai.ai>; gcc-patches <gcc-patches@gcc.gnu.org>
Cc: jeffreyalaw <jeffreyalaw@gmail.com>; Robin Dapp <rdapp.gcc@gmail.com>; palmer <palmer@rivosinc.com>
Subject: 回复: Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119]

Also
p110119-1.c
change name of test into
pr110119-1.c


juzhe.zhong@rivai.ai
 
发件人： juzhe.zhong@rivai.ai
发送时间： 2023-06-14 19:17
收件人： 丁乐华; gcc-patches
抄送： jeffreyalaw; Robin Dapp; palmer
主题： Re: [PATCH] RISC-V: Ensure vector args and return use function stack to pass [PR110119] 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
  

> Nit for test.
> +/* { dg-options "-march=rv64gczve32x
> +--param=riscv-autovec-preference=fixed-vlmax" } */
> To
> +/* { dg-options "-march=rv64gc_zve32x --param=riscv-autovec-preference=fixed-vlmax" } */
Fixed in the V2 patch (https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621698.html), thank you.


Best,
Lehua