RISC-V: Fix PR 110119

Message ID 20230614103444.2179711-1-lehua.ding@rivai.ai
State Superseded
Headers
Series RISC-V: Fix PR 110119 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Lehua Ding June 14, 2023, 10:34 a.m. UTC
  Hi,

This patch fix the PR 110119. 

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

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     | 27 +++++++++++++++++++
 .../gcc.target/riscv/rvv/base/p110119-2.c     | 27 +++++++++++++++++++
 3 files changed, 67 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
  

Comments

钟居哲 June 14, 2023, 10:52 a.m. UTC | #1
Add PR target/pr110119



juzhe.zhong@rivai.ai
 
From: Lehua Ding
Date: 2023-06-14 18:34
To: gcc-patches; juzhe.zhong
Subject: [PATCH] RISC-V: Fix PR 110119
Hi,
 
This patch fix the PR 110119. 
 
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
 
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     | 27 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/p110119-2.c     | 27 +++++++++++++++++++
3 files changed, 67 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..3583e06f1a8d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv --param=riscv-autovec-preference=fixed-vlmax" } */
+/* { dg-skip-if "test rvv intrinsic" { *-*-* } { "*" } { "-march=rv*v*" } } */
+
+#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..1d12a610b677
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gczve32x --param=riscv-autovec-preference=fixed-vlmax" } */
+/* { dg-skip-if "test rvv intrinsic" { *-*-* } { "*" } { "-march=rv*v*" } } */
+
+#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
  
Lehua Ding June 14, 2023, 11:07 a.m. UTC | #2
Resubmitted a new, more standardized patch(bellow is the new patch link), thanks.


https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621683.html


 &nbsp;
  

Patch

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..3583e06f1a8d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-1.c
@@ -0,0 +1,27 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv --param=riscv-autovec-preference=fixed-vlmax" } */
+/* { dg-skip-if "test rvv intrinsic" { *-*-* } { "*" } { "-march=rv*v*" } } */
+
+#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..1d12a610b677
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/p110119-2.c
@@ -0,0 +1,27 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gczve32x --param=riscv-autovec-preference=fixed-vlmax" } */
+/* { dg-skip-if "test rvv intrinsic" { *-*-* } { "*" } { "-march=rv*v*" } } */
+
+#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;
+}