Message ID | 20240411021628.3470772-1-pan2.li@intel.com |
---|---|
State | New |
Headers | show |
Series | [v1] RISC-V: Bugfix ICE for the vector return arg in mode switch | expand |
Thanks for fixing it. LGTM from my side. I prefer wait kito for another ACK. juzhe.zhong@rivai.ai From: pan2.li Date: 2024-04-11 10:16 To: gcc-patches CC: juzhe.zhong; kito.cheng; Pan Li Subject: [PATCH v1] RISC-V: Bugfix ICE for the vector return arg in mode switch From: Pan Li <pan2.li@intel.com> This patch would like to fix a ICE in mode sw for below example code. during RTL pass: mode_sw test.c: In function ‘vbool16_t j(vuint64m4_t)’: test.c:15:1: internal compiler error: in create_pre_exit, at mode-switching.cc:451 15 | } | ^ 0x3978f12 create_pre_exit __RISCV_BUILD__/../gcc/mode-switching.cc:451 0x3979e9e optimize_mode_switching __RISCV_BUILD__/../gcc/mode-switching.cc:849 0x397b9bc execute __RISCV_BUILD__/../gcc/mode-switching.cc:1324 extern size_t get_vl (); vbool16_t test (vuint64m4_t a) { unsigned long b; return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); } The create_pre_exit would like to find a return value copy. If not, there will be a reason in assert but not available for above sample code when vector calling convension is enabled by default. This patch would like to override the TARGET_FUNCTION_VALUE_REGNO_P for vector register and then we will have hard_regno_nregs for copy_num, aka there is a return value copy. As a side-effect of allow vector in TARGET_FUNCTION_VALUE_REGNO_P, the TARGET_GET_RAW_RESULT_MODE will have vector mode and which is sizeless cannot be converted to fixed_size_mode. Thus override the hook TARGET_GET_RAW_RESULT_MODE and return VOIDmode when the regno is-not-a fixed_size_mode. The below tests are passed for this patch. * The fully riscv regression tests. * The reproducing test in bugzilla PR114639. PR target/114639 gcc/ChangeLog: * config/riscv/riscv.cc (riscv_function_value_regno_p): New func impl for hook TARGET_FUNCTION_VALUE_REGNO_P. (riscv_get_raw_result_mode): New func imple for hook TARGET_GET_RAW_RESULT_MODE. (TARGET_FUNCTION_VALUE_REGNO_P): Impl the hook. (TARGET_GET_RAW_RESULT_MODE): Ditto. * config/riscv/riscv.h (V_RETURN): New macro for vector return. (GP_RETURN_FIRST): New macro for the first GPR in return. (GP_RETURN_LAST): New macro for the last GPR in return. (FP_RETURN_FIRST): Diito but for FPR. (FP_RETURN_LAST): Ditto. (FUNCTION_VALUE_REGNO_P): Remove as deprecated and replace by TARGET_FUNCTION_VALUE_REGNO_P. gcc/testsuite/ChangeLog: * g++.target/riscv/rvv/base/pr114639-1.C: New test. * gcc.target/riscv/rvv/base/pr114639-1.c: New test. Signed-off-by: Pan Li <pan2.li@intel.com> --- gcc/config/riscv/riscv.cc | 34 +++++++++++++++++++ gcc/config/riscv/riscv.h | 8 +++-- .../g++.target/riscv/rvv/base/pr114639-1.C | 25 ++++++++++++++ .../gcc.target/riscv/rvv/base/pr114639-1.c | 14 ++++++++ 4 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 00defa69fd8..91f017dd52a 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -10997,6 +10997,34 @@ riscv_vector_mode_supported_any_target_p (machine_mode) return true; } +/* Implements hook TARGET_FUNCTION_VALUE_REGNO_P. */ + +static bool +riscv_function_value_regno_p (const unsigned regno) +{ + if (GP_RETURN_FIRST <= regno && regno <= GP_RETURN_LAST) + return true; + + if (FP_RETURN_FIRST <= regno && regno <= FP_RETURN_LAST) + return true; + + if (regno == V_RETURN) + return true; + + return false; +} + +/* Implements hook TARGET_GET_RAW_RESULT_MODE. */ + +static fixed_size_mode +riscv_get_raw_result_mode (int regno) +{ + if (!is_a <fixed_size_mode> (reg_raw_mode[regno])) + return as_a <fixed_size_mode> (VOIDmode); + + return default_get_reg_raw_mode (regno); +} + /* Initialize the GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t" @@ -11343,6 +11371,12 @@ riscv_vector_mode_supported_any_target_p (machine_mode) #undef TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P #define TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P riscv_vector_mode_supported_any_target_p +#undef TARGET_FUNCTION_VALUE_REGNO_P +#define TARGET_FUNCTION_VALUE_REGNO_P riscv_function_value_regno_p + +#undef TARGET_GET_RAW_RESULT_MODE +#define TARGET_GET_RAW_RESULT_MODE riscv_get_raw_result_mode + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-riscv.h" diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index 269b8c1f076..7797e67317a 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -683,6 +683,12 @@ enum reg_class #define GP_RETURN GP_ARG_FIRST #define FP_RETURN (UNITS_PER_FP_ARG == 0 ? GP_RETURN : FP_ARG_FIRST) +#define V_RETURN V_REG_FIRST + +#define GP_RETURN_FIRST GP_ARG_FIRST +#define GP_RETURN_LAST GP_ARG_FIRST + 1 +#define FP_RETURN_FIRST FP_RETURN +#define FP_RETURN_LAST FP_RETURN + 1 #define MAX_ARGS_IN_REGISTERS \ (riscv_abi == ABI_ILP32E || riscv_abi == ABI_LP64E \ @@ -714,8 +720,6 @@ enum reg_class #define FUNCTION_VALUE(VALTYPE, FUNC) \ riscv_function_value (VALTYPE, FUNC, VOIDmode) -#define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN) - /* 1 if N is a possible register number for function argument passing. We have no FP argument registers when soft-float. */ diff --git a/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C new file mode 100644 index 00000000000..9450b108ae5 --- /dev/null +++ b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C @@ -0,0 +1,25 @@ +/* Test that we do not have ice when compile */ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ + +typedef long c; + +#pragma riscv intrinsic "vector" + +template <unsigned long> struct d {}; + +struct e { + using f = d<0>; +}; + +struct g { + using f = e::f; +}; + +template <typename, int> using h = g::f; +template <unsigned long i> long get_vl (d<i>); + +vbool16_t test (vuint64m4_t a) { + c b; + return __riscv_vmsne_vx_u64m4_b16(a, b, get_vl (h<c, 2>())); +} diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c new file mode 100644 index 00000000000..3ad91dbf6bb --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c @@ -0,0 +1,14 @@ +/* Test that we do not have ice when compile */ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ + +#include <riscv_vector.h> + +extern size_t get_vl (); + +vbool16_t +test (vuint64m4_t a) +{ + unsigned long b; + return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); +}
I was thinking we may guarded with TARGET_VECTOR and TARGET_HARD_FLOAT or checking with ABI in riscv_function_value_regno_p, however I think it's fine with current implementation (no checking) after checking all use site of `targetm.calls.function_value_regno_p`, so LGTM :) Thanks Pan for fixing this issue! On Thu, Apr 11, 2024 at 10:23 AM juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai> wrote: > > Thanks for fixing it. LGTM from my side. > > I prefer wait kito for another ACK. > > ________________________________ > juzhe.zhong@rivai.ai > > > From: pan2.li > Date: 2024-04-11 10:16 > To: gcc-patches > CC: juzhe.zhong; kito.cheng; Pan Li > Subject: [PATCH v1] RISC-V: Bugfix ICE for the vector return arg in mode switch > From: Pan Li <pan2.li@intel.com> > > This patch would like to fix a ICE in mode sw for below example code. > > during RTL pass: mode_sw > test.c: In function ‘vbool16_t j(vuint64m4_t)’: > test.c:15:1: internal compiler error: in create_pre_exit, at > mode-switching.cc:451 > 15 | } > | ^ > 0x3978f12 create_pre_exit > __RISCV_BUILD__/../gcc/mode-switching.cc:451 > 0x3979e9e optimize_mode_switching > __RISCV_BUILD__/../gcc/mode-switching.cc:849 > 0x397b9bc execute > __RISCV_BUILD__/../gcc/mode-switching.cc:1324 > > extern size_t get_vl (); > > vbool16_t > test (vuint64m4_t a) > { > unsigned long b; > return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); > } > > The create_pre_exit would like to find a return value copy. If > not, there will be a reason in assert but not available for above > sample code when vector calling convension is enabled by default. > This patch would like to override the TARGET_FUNCTION_VALUE_REGNO_P > for vector register and then we will have hard_regno_nregs for copy_num, > aka there is a return value copy. > > As a side-effect of allow vector in TARGET_FUNCTION_VALUE_REGNO_P, the > TARGET_GET_RAW_RESULT_MODE will have vector mode and which is sizeless > cannot be converted to fixed_size_mode. Thus override the hook > TARGET_GET_RAW_RESULT_MODE and return VOIDmode when the regno is-not-a > fixed_size_mode. > > The below tests are passed for this patch. > * The fully riscv regression tests. > * The reproducing test in bugzilla PR114639. > > PR target/114639 > > gcc/ChangeLog: > > * config/riscv/riscv.cc (riscv_function_value_regno_p): New func > impl for hook TARGET_FUNCTION_VALUE_REGNO_P. > (riscv_get_raw_result_mode): New func imple for hook > TARGET_GET_RAW_RESULT_MODE. > (TARGET_FUNCTION_VALUE_REGNO_P): Impl the hook. > (TARGET_GET_RAW_RESULT_MODE): Ditto. > * config/riscv/riscv.h (V_RETURN): New macro for vector return. > (GP_RETURN_FIRST): New macro for the first GPR in return. > (GP_RETURN_LAST): New macro for the last GPR in return. > (FP_RETURN_FIRST): Diito but for FPR. > (FP_RETURN_LAST): Ditto. > (FUNCTION_VALUE_REGNO_P): Remove as deprecated and replace by > TARGET_FUNCTION_VALUE_REGNO_P. > > gcc/testsuite/ChangeLog: > > * g++.target/riscv/rvv/base/pr114639-1.C: New test. > * gcc.target/riscv/rvv/base/pr114639-1.c: New test. > > Signed-off-by: Pan Li <pan2.li@intel.com> > --- > gcc/config/riscv/riscv.cc | 34 +++++++++++++++++++ > gcc/config/riscv/riscv.h | 8 +++-- > .../g++.target/riscv/rvv/base/pr114639-1.C | 25 ++++++++++++++ > .../gcc.target/riscv/rvv/base/pr114639-1.c | 14 ++++++++ > 4 files changed, 79 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 00defa69fd8..91f017dd52a 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -10997,6 +10997,34 @@ riscv_vector_mode_supported_any_target_p (machine_mode) > return true; > } > +/* Implements hook TARGET_FUNCTION_VALUE_REGNO_P. */ > + > +static bool > +riscv_function_value_regno_p (const unsigned regno) > +{ > + if (GP_RETURN_FIRST <= regno && regno <= GP_RETURN_LAST) > + return true; > + > + if (FP_RETURN_FIRST <= regno && regno <= FP_RETURN_LAST) > + return true; > + > + if (regno == V_RETURN) > + return true; > + > + return false; > +} > + > +/* Implements hook TARGET_GET_RAW_RESULT_MODE. */ > + > +static fixed_size_mode > +riscv_get_raw_result_mode (int regno) > +{ > + if (!is_a <fixed_size_mode> (reg_raw_mode[regno])) > + return as_a <fixed_size_mode> (VOIDmode); > + > + return default_get_reg_raw_mode (regno); > +} > + > /* Initialize the GCC target structure. */ > #undef TARGET_ASM_ALIGNED_HI_OP > #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t" > @@ -11343,6 +11371,12 @@ riscv_vector_mode_supported_any_target_p (machine_mode) > #undef TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P > #define TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P riscv_vector_mode_supported_any_target_p > +#undef TARGET_FUNCTION_VALUE_REGNO_P > +#define TARGET_FUNCTION_VALUE_REGNO_P riscv_function_value_regno_p > + > +#undef TARGET_GET_RAW_RESULT_MODE > +#define TARGET_GET_RAW_RESULT_MODE riscv_get_raw_result_mode > + > struct gcc_target targetm = TARGET_INITIALIZER; > #include "gt-riscv.h" > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h > index 269b8c1f076..7797e67317a 100644 > --- a/gcc/config/riscv/riscv.h > +++ b/gcc/config/riscv/riscv.h > @@ -683,6 +683,12 @@ enum reg_class > #define GP_RETURN GP_ARG_FIRST > #define FP_RETURN (UNITS_PER_FP_ARG == 0 ? GP_RETURN : FP_ARG_FIRST) > +#define V_RETURN V_REG_FIRST > + > +#define GP_RETURN_FIRST GP_ARG_FIRST > +#define GP_RETURN_LAST GP_ARG_FIRST + 1 > +#define FP_RETURN_FIRST FP_RETURN > +#define FP_RETURN_LAST FP_RETURN + 1 > #define MAX_ARGS_IN_REGISTERS \ > (riscv_abi == ABI_ILP32E || riscv_abi == ABI_LP64E \ > @@ -714,8 +720,6 @@ enum reg_class > #define FUNCTION_VALUE(VALTYPE, FUNC) \ > riscv_function_value (VALTYPE, FUNC, VOIDmode) > -#define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN) > - > /* 1 if N is a possible register number for function argument passing. > We have no FP argument registers when soft-float. */ > diff --git a/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C > new file mode 100644 > index 00000000000..9450b108ae5 > --- /dev/null > +++ b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C > @@ -0,0 +1,25 @@ > +/* Test that we do not have ice when compile */ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +typedef long c; > + > +#pragma riscv intrinsic "vector" > + > +template <unsigned long> struct d {}; > + > +struct e { > + using f = d<0>; > +}; > + > +struct g { > + using f = e::f; > +}; > + > +template <typename, int> using h = g::f; > +template <unsigned long i> long get_vl (d<i>); > + > +vbool16_t test (vuint64m4_t a) { > + c b; > + return __riscv_vmsne_vx_u64m4_b16(a, b, get_vl (h<c, 2>())); > +} > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c > new file mode 100644 > index 00000000000..3ad91dbf6bb > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c > @@ -0,0 +1,14 @@ > +/* Test that we do not have ice when compile */ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +#include <riscv_vector.h> > + > +extern size_t get_vl (); > + > +vbool16_t > +test (vuint64m4_t a) > +{ > + unsigned long b; > + return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); > +} > -- > 2.34.1 > >
Committed, thanks Juzhe and Kito. Pan -----Original Message----- From: Kito Cheng <kito.cheng@gmail.com> Sent: Thursday, April 11, 2024 10:50 AM To: juzhe.zhong@rivai.ai Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches <gcc-patches@gcc.gnu.org> Subject: Re: [PATCH v1] RISC-V: Bugfix ICE for the vector return arg in mode switch I was thinking we may guarded with TARGET_VECTOR and TARGET_HARD_FLOAT or checking with ABI in riscv_function_value_regno_p, however I think it's fine with current implementation (no checking) after checking all use site of `targetm.calls.function_value_regno_p`, so LGTM :) Thanks Pan for fixing this issue! On Thu, Apr 11, 2024 at 10:23 AM juzhe.zhong@rivai.ai <juzhe.zhong@rivai.ai> wrote: > > Thanks for fixing it. LGTM from my side. > > I prefer wait kito for another ACK. > > ________________________________ > juzhe.zhong@rivai.ai > > > From: pan2.li > Date: 2024-04-11 10:16 > To: gcc-patches > CC: juzhe.zhong; kito.cheng; Pan Li > Subject: [PATCH v1] RISC-V: Bugfix ICE for the vector return arg in mode switch > From: Pan Li <pan2.li@intel.com> > > This patch would like to fix a ICE in mode sw for below example code. > > during RTL pass: mode_sw > test.c: In function ‘vbool16_t j(vuint64m4_t)’: > test.c:15:1: internal compiler error: in create_pre_exit, at > mode-switching.cc:451 > 15 | } > | ^ > 0x3978f12 create_pre_exit > __RISCV_BUILD__/../gcc/mode-switching.cc:451 > 0x3979e9e optimize_mode_switching > __RISCV_BUILD__/../gcc/mode-switching.cc:849 > 0x397b9bc execute > __RISCV_BUILD__/../gcc/mode-switching.cc:1324 > > extern size_t get_vl (); > > vbool16_t > test (vuint64m4_t a) > { > unsigned long b; > return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); > } > > The create_pre_exit would like to find a return value copy. If > not, there will be a reason in assert but not available for above > sample code when vector calling convension is enabled by default. > This patch would like to override the TARGET_FUNCTION_VALUE_REGNO_P > for vector register and then we will have hard_regno_nregs for copy_num, > aka there is a return value copy. > > As a side-effect of allow vector in TARGET_FUNCTION_VALUE_REGNO_P, the > TARGET_GET_RAW_RESULT_MODE will have vector mode and which is sizeless > cannot be converted to fixed_size_mode. Thus override the hook > TARGET_GET_RAW_RESULT_MODE and return VOIDmode when the regno is-not-a > fixed_size_mode. > > The below tests are passed for this patch. > * The fully riscv regression tests. > * The reproducing test in bugzilla PR114639. > > PR target/114639 > > gcc/ChangeLog: > > * config/riscv/riscv.cc (riscv_function_value_regno_p): New func > impl for hook TARGET_FUNCTION_VALUE_REGNO_P. > (riscv_get_raw_result_mode): New func imple for hook > TARGET_GET_RAW_RESULT_MODE. > (TARGET_FUNCTION_VALUE_REGNO_P): Impl the hook. > (TARGET_GET_RAW_RESULT_MODE): Ditto. > * config/riscv/riscv.h (V_RETURN): New macro for vector return. > (GP_RETURN_FIRST): New macro for the first GPR in return. > (GP_RETURN_LAST): New macro for the last GPR in return. > (FP_RETURN_FIRST): Diito but for FPR. > (FP_RETURN_LAST): Ditto. > (FUNCTION_VALUE_REGNO_P): Remove as deprecated and replace by > TARGET_FUNCTION_VALUE_REGNO_P. > > gcc/testsuite/ChangeLog: > > * g++.target/riscv/rvv/base/pr114639-1.C: New test. > * gcc.target/riscv/rvv/base/pr114639-1.c: New test. > > Signed-off-by: Pan Li <pan2.li@intel.com> > --- > gcc/config/riscv/riscv.cc | 34 +++++++++++++++++++ > gcc/config/riscv/riscv.h | 8 +++-- > .../g++.target/riscv/rvv/base/pr114639-1.C | 25 ++++++++++++++ > .../gcc.target/riscv/rvv/base/pr114639-1.c | 14 ++++++++ > 4 files changed, 79 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 00defa69fd8..91f017dd52a 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -10997,6 +10997,34 @@ riscv_vector_mode_supported_any_target_p (machine_mode) > return true; > } > +/* Implements hook TARGET_FUNCTION_VALUE_REGNO_P. */ > + > +static bool > +riscv_function_value_regno_p (const unsigned regno) > +{ > + if (GP_RETURN_FIRST <= regno && regno <= GP_RETURN_LAST) > + return true; > + > + if (FP_RETURN_FIRST <= regno && regno <= FP_RETURN_LAST) > + return true; > + > + if (regno == V_RETURN) > + return true; > + > + return false; > +} > + > +/* Implements hook TARGET_GET_RAW_RESULT_MODE. */ > + > +static fixed_size_mode > +riscv_get_raw_result_mode (int regno) > +{ > + if (!is_a <fixed_size_mode> (reg_raw_mode[regno])) > + return as_a <fixed_size_mode> (VOIDmode); > + > + return default_get_reg_raw_mode (regno); > +} > + > /* Initialize the GCC target structure. */ > #undef TARGET_ASM_ALIGNED_HI_OP > #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t" > @@ -11343,6 +11371,12 @@ riscv_vector_mode_supported_any_target_p (machine_mode) > #undef TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P > #define TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P riscv_vector_mode_supported_any_target_p > +#undef TARGET_FUNCTION_VALUE_REGNO_P > +#define TARGET_FUNCTION_VALUE_REGNO_P riscv_function_value_regno_p > + > +#undef TARGET_GET_RAW_RESULT_MODE > +#define TARGET_GET_RAW_RESULT_MODE riscv_get_raw_result_mode > + > struct gcc_target targetm = TARGET_INITIALIZER; > #include "gt-riscv.h" > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h > index 269b8c1f076..7797e67317a 100644 > --- a/gcc/config/riscv/riscv.h > +++ b/gcc/config/riscv/riscv.h > @@ -683,6 +683,12 @@ enum reg_class > #define GP_RETURN GP_ARG_FIRST > #define FP_RETURN (UNITS_PER_FP_ARG == 0 ? GP_RETURN : FP_ARG_FIRST) > +#define V_RETURN V_REG_FIRST > + > +#define GP_RETURN_FIRST GP_ARG_FIRST > +#define GP_RETURN_LAST GP_ARG_FIRST + 1 > +#define FP_RETURN_FIRST FP_RETURN > +#define FP_RETURN_LAST FP_RETURN + 1 > #define MAX_ARGS_IN_REGISTERS \ > (riscv_abi == ABI_ILP32E || riscv_abi == ABI_LP64E \ > @@ -714,8 +720,6 @@ enum reg_class > #define FUNCTION_VALUE(VALTYPE, FUNC) \ > riscv_function_value (VALTYPE, FUNC, VOIDmode) > -#define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN) > - > /* 1 if N is a possible register number for function argument passing. > We have no FP argument registers when soft-float. */ > diff --git a/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C > new file mode 100644 > index 00000000000..9450b108ae5 > --- /dev/null > +++ b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C > @@ -0,0 +1,25 @@ > +/* Test that we do not have ice when compile */ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +typedef long c; > + > +#pragma riscv intrinsic "vector" > + > +template <unsigned long> struct d {}; > + > +struct e { > + using f = d<0>; > +}; > + > +struct g { > + using f = e::f; > +}; > + > +template <typename, int> using h = g::f; > +template <unsigned long i> long get_vl (d<i>); > + > +vbool16_t test (vuint64m4_t a) { > + c b; > + return __riscv_vmsne_vx_u64m4_b16(a, b, get_vl (h<c, 2>())); > +} > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c > new file mode 100644 > index 00000000000..3ad91dbf6bb > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c > @@ -0,0 +1,14 @@ > +/* Test that we do not have ice when compile */ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +#include <riscv_vector.h> > + > +extern size_t get_vl (); > + > +vbool16_t > +test (vuint64m4_t a) > +{ > + unsigned long b; > + return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); > +} > -- > 2.34.1 > >
On 4/11/24 05:03, Li, Pan2 wrote: > Committed, thanks Juzhe and Kito. > > Pan Hi Pan, this commit caused a regression: FAIL: gcc.c-torture/compile/930623-1.c -O0 (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O1 (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O1 (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O2 (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O2 (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O3 -g (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O3 -g (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -Os (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -Os (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) gcc/testsuite/gcc.c-torture/compile/930623-1.c:10:3: internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059^M 0xbba2de riscv_vector::emit_vec_extract(rtx_def*, rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv-v.cc:5059^M 0x186945f riscv_legitimize_move(machine_mode, rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv.cc:2895^M 0x1ef50b2 gen_movsi(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv.md:2225^M 0xffc91c rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*>(rtx_def*, rtx_def*) const^M ../../gcc-trunk/gcc/recog.h:441^M 0xffc91c emit_move_insn_1(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/expr.cc:4551^M 0xffcdf4 emit_move_insn(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/expr.cc:4721^M 0x1002f17 emit_move_multi_word^M ../../gcc-trunk/gcc/expr.cc:4517^M 0xffcdf4 emit_move_insn(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/expr.cc:4721^M 0x1efc6b7 gen_untyped_call(rtx_def*, rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv.md:3478^M 0x185fc7c target_gen_untyped_call^M ../../gcc-trunk/gcc/config/riscv/riscv.md:3453^M 0xe8e81f expand_builtin_apply^M ../../gcc-trunk/gcc/builtins.cc:1761^M 0xea053c expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)^M ../../gcc-trunk/gcc/builtins.cc:8001^M 0xff9e27 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)^M ../../gcc-trunk/gcc/expr.cc:12353^M 0xec4c3d expand_expr(tree_node*, rtx_def*, machine_mode, expand_modifier)^M ../../gcc-trunk/gcc/expr.h:316^M 0xec4c3d expand_call_stmt^M ../../gcc-trunk/gcc/cfgexpand.cc:2865^M 0xec4c3d expand_gimple_stmt_1^M ../../gcc-trunk/gcc/cfgexpand.cc:3932^M 0xec4c3d expand_gimple_stmt^M ../../gcc-trunk/gcc/cfgexpand.cc:4077^M 0xeca206 expand_gimple_basic_block^M ../../gcc-trunk/gcc/cfgexpand.cc:6133^M 0xecc287 execute^M ../../gcc-trunk/gcc/cfgexpand.cc:6872^M Please submit a full bug report, with preprocessed source (by using -freport-bug).^M Please include the complete backtrace with any bug report.^M See <https://gcc.gnu.org/bugs/> for instructions.^M compiler exited with status 1 I've built the git revision f3fdcf4a37a with ../gcc-trunk/configure --target=riscv-unknown-elf --prefix=/home/ed/gnu/riscv-unknown-elf --enable-languages=c,c++ --disable-multilib --with-arch=rv32imac --with-abi=ilp32 I am a bit surprised since the target is not supposed to support floating point or vector instructions AFAIK. The issue does not happen with gcc-trunk from yesterday. Regards Bernd.
Thanks for reporting this. Just take a look from my test log that 930623-1.c is all pass. Thus I bet this difference comes from the build option --with-arch=rv32imac but my test script take rv64gcv. > I've built the git revision f3fdcf4a37a with > ../gcc-trunk/configure --target=riscv-unknown-elf --prefix=/home/ed/gnu/riscv-unknown-elf --enable-languages=c,c++ --disable-multilib --with-arch=rv32imac --with-abi=ilp32 > I am a bit surprised since the target is not supposed to support floating point > or vector instructions AFAIK. Because you specify rv32imac, with doesn't include f/d/v extension, aka single/double floating point and vector extension. Thus, related functionality are disabled. > The issue does not happen with gcc-trunk from yesterday. Ack, will look into it. Pan -----Original Message----- From: Bernd Edlinger <bernd.edlinger@hotmail.de> Sent: Thursday, April 11, 2024 7:52 PM To: Li, Pan2 <pan2.li@intel.com>; Kito Cheng <kito.cheng@gmail.com>; juzhe.zhong@rivai.ai Cc: gcc-patches <gcc-patches@gcc.gnu.org> Subject: Re: [PATCH v1] RISC-V: Bugfix ICE for the vector return arg in mode switch On 4/11/24 05:03, Li, Pan2 wrote: > Committed, thanks Juzhe and Kito. > > Pan Hi Pan, this commit caused a regression: FAIL: gcc.c-torture/compile/930623-1.c -O0 (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O1 (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O1 (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O2 (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O2 (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O3 -g (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O3 -g (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -Os (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -Os (test for excess errors) FAIL: gcc.c-torture/compile/930623-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059) FAIL: gcc.c-torture/compile/930623-1.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) gcc/testsuite/gcc.c-torture/compile/930623-1.c:10:3: internal compiler error: in emit_vec_extract, at config/riscv/riscv-v.cc:5059^M 0xbba2de riscv_vector::emit_vec_extract(rtx_def*, rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv-v.cc:5059^M 0x186945f riscv_legitimize_move(machine_mode, rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv.cc:2895^M 0x1ef50b2 gen_movsi(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv.md:2225^M 0xffc91c rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*>(rtx_def*, rtx_def*) const^M ../../gcc-trunk/gcc/recog.h:441^M 0xffc91c emit_move_insn_1(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/expr.cc:4551^M 0xffcdf4 emit_move_insn(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/expr.cc:4721^M 0x1002f17 emit_move_multi_word^M ../../gcc-trunk/gcc/expr.cc:4517^M 0xffcdf4 emit_move_insn(rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/expr.cc:4721^M 0x1efc6b7 gen_untyped_call(rtx_def*, rtx_def*, rtx_def*)^M ../../gcc-trunk/gcc/config/riscv/riscv.md:3478^M 0x185fc7c target_gen_untyped_call^M ../../gcc-trunk/gcc/config/riscv/riscv.md:3453^M 0xe8e81f expand_builtin_apply^M ../../gcc-trunk/gcc/builtins.cc:1761^M 0xea053c expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)^M ../../gcc-trunk/gcc/builtins.cc:8001^M 0xff9e27 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)^M ../../gcc-trunk/gcc/expr.cc:12353^M 0xec4c3d expand_expr(tree_node*, rtx_def*, machine_mode, expand_modifier)^M ../../gcc-trunk/gcc/expr.h:316^M 0xec4c3d expand_call_stmt^M ../../gcc-trunk/gcc/cfgexpand.cc:2865^M 0xec4c3d expand_gimple_stmt_1^M ../../gcc-trunk/gcc/cfgexpand.cc:3932^M 0xec4c3d expand_gimple_stmt^M ../../gcc-trunk/gcc/cfgexpand.cc:4077^M 0xeca206 expand_gimple_basic_block^M ../../gcc-trunk/gcc/cfgexpand.cc:6133^M 0xecc287 execute^M ../../gcc-trunk/gcc/cfgexpand.cc:6872^M Please submit a full bug report, with preprocessed source (by using -freport-bug).^M Please include the complete backtrace with any bug report.^M See <https://gcc.gnu.org/bugs/> for instructions.^M compiler exited with status 1 I've built the git revision f3fdcf4a37a with ../gcc-trunk/configure --target=riscv-unknown-elf --prefix=/home/ed/gnu/riscv-unknown-elf --enable-languages=c,c++ --disable-multilib --with-arch=rv32imac --with-abi=ilp32 I am a bit surprised since the target is not supposed to support floating point or vector instructions AFAIK. The issue does not happen with gcc-trunk from yesterday. Regards Bernd.
On 4/11/2024 5:45 AM, Li, Pan2 wrote: > Thanks for reporting this. Just take a look from my test log that 930623-1.c is all pass. > > Thus I bet this difference comes from the build option --with-arch=rv32imac but my test script take rv64gcv. > >> I've built the git revision f3fdcf4a37a with >> ../gcc-trunk/configure --target=riscv-unknown-elf --prefix=/home/ed/gnu/riscv-unknown-elf --enable-languages=c,c++ --disable-multilib --with-arch=rv32imac --with-abi=ilp32 > >> I am a bit surprised since the target is not supposed to support floating point >> or vector instructions AFAIK. > > Because you specify rv32imac, with doesn't include f/d/v extension, aka single/double floating point and vector extension. Thus, related functionality are disabled. > >> The issue does not happen with gcc-trunk from yesterday. > > Ack, will look into it. > > Pan > Hi Pan, Our postcommit-ci found that it breaks for non-vector targets on rv32/64 newlib/linux https://github.com/patrick-rivos/gcc-postcommit-ci/issues/757. The patchwork precommit-ci also appeared to have flagged it https://github.com/ewlu/gcc-precommit-ci/issues/1417#issuecomment-2048846532 Edwin
Thanks Edwin, should be one silly mistake, will fix it ASAP. Pan -----Original Message----- From: Edwin Lu <ewlu@rivosinc.com> Sent: Friday, April 12, 2024 5:20 AM To: Li, Pan2 <pan2.li@intel.com>; Bernd Edlinger <bernd.edlinger@hotmail.de>; Kito Cheng <kito.cheng@gmail.com>; juzhe.zhong@rivai.ai Cc: gcc-patches <gcc-patches@gcc.gnu.org> Subject: Re: [PATCH v1] RISC-V: Bugfix ICE for the vector return arg in mode switch On 4/11/2024 5:45 AM, Li, Pan2 wrote: > Thanks for reporting this. Just take a look from my test log that 930623-1.c is all pass. > > Thus I bet this difference comes from the build option --with-arch=rv32imac but my test script take rv64gcv. > >> I've built the git revision f3fdcf4a37a with >> ../gcc-trunk/configure --target=riscv-unknown-elf --prefix=/home/ed/gnu/riscv-unknown-elf --enable-languages=c,c++ --disable-multilib --with-arch=rv32imac --with-abi=ilp32 > >> I am a bit surprised since the target is not supposed to support floating point >> or vector instructions AFAIK. > > Because you specify rv32imac, with doesn't include f/d/v extension, aka single/double floating point and vector extension. Thus, related functionality are disabled. > >> The issue does not happen with gcc-trunk from yesterday. > > Ack, will look into it. > > Pan > Hi Pan, Our postcommit-ci found that it breaks for non-vector targets on rv32/64 newlib/linux https://github.com/patrick-rivos/gcc-postcommit-ci/issues/757. The patchwork precommit-ci also appeared to have flagged it https://github.com/ewlu/gcc-precommit-ci/issues/1417#issuecomment-2048846532 Edwin
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 00defa69fd8..91f017dd52a 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -10997,6 +10997,34 @@ riscv_vector_mode_supported_any_target_p (machine_mode) return true; } +/* Implements hook TARGET_FUNCTION_VALUE_REGNO_P. */ + +static bool +riscv_function_value_regno_p (const unsigned regno) +{ + if (GP_RETURN_FIRST <= regno && regno <= GP_RETURN_LAST) + return true; + + if (FP_RETURN_FIRST <= regno && regno <= FP_RETURN_LAST) + return true; + + if (regno == V_RETURN) + return true; + + return false; +} + +/* Implements hook TARGET_GET_RAW_RESULT_MODE. */ + +static fixed_size_mode +riscv_get_raw_result_mode (int regno) +{ + if (!is_a <fixed_size_mode> (reg_raw_mode[regno])) + return as_a <fixed_size_mode> (VOIDmode); + + return default_get_reg_raw_mode (regno); +} + /* Initialize the GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t" @@ -11343,6 +11371,12 @@ riscv_vector_mode_supported_any_target_p (machine_mode) #undef TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P #define TARGET_VECTOR_MODE_SUPPORTED_ANY_TARGET_P riscv_vector_mode_supported_any_target_p +#undef TARGET_FUNCTION_VALUE_REGNO_P +#define TARGET_FUNCTION_VALUE_REGNO_P riscv_function_value_regno_p + +#undef TARGET_GET_RAW_RESULT_MODE +#define TARGET_GET_RAW_RESULT_MODE riscv_get_raw_result_mode + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-riscv.h" diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index 269b8c1f076..7797e67317a 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -683,6 +683,12 @@ enum reg_class #define GP_RETURN GP_ARG_FIRST #define FP_RETURN (UNITS_PER_FP_ARG == 0 ? GP_RETURN : FP_ARG_FIRST) +#define V_RETURN V_REG_FIRST + +#define GP_RETURN_FIRST GP_ARG_FIRST +#define GP_RETURN_LAST GP_ARG_FIRST + 1 +#define FP_RETURN_FIRST FP_RETURN +#define FP_RETURN_LAST FP_RETURN + 1 #define MAX_ARGS_IN_REGISTERS \ (riscv_abi == ABI_ILP32E || riscv_abi == ABI_LP64E \ @@ -714,8 +720,6 @@ enum reg_class #define FUNCTION_VALUE(VALTYPE, FUNC) \ riscv_function_value (VALTYPE, FUNC, VOIDmode) -#define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN) - /* 1 if N is a possible register number for function argument passing. We have no FP argument registers when soft-float. */ diff --git a/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C new file mode 100644 index 00000000000..9450b108ae5 --- /dev/null +++ b/gcc/testsuite/g++.target/riscv/rvv/base/pr114639-1.C @@ -0,0 +1,25 @@ +/* Test that we do not have ice when compile */ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ + +typedef long c; + +#pragma riscv intrinsic "vector" + +template <unsigned long> struct d {}; + +struct e { + using f = d<0>; +}; + +struct g { + using f = e::f; +}; + +template <typename, int> using h = g::f; +template <unsigned long i> long get_vl (d<i>); + +vbool16_t test (vuint64m4_t a) { + c b; + return __riscv_vmsne_vx_u64m4_b16(a, b, get_vl (h<c, 2>())); +} diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c new file mode 100644 index 00000000000..3ad91dbf6bb --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114639-1.c @@ -0,0 +1,14 @@ +/* Test that we do not have ice when compile */ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ + +#include <riscv_vector.h> + +extern size_t get_vl (); + +vbool16_t +test (vuint64m4_t a) +{ + unsigned long b; + return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); +}