Committed, thanks Juzhe and Kito.
Pan
-----Original Message-----
From: Kito Cheng <kito.ch...@gmail.com>
Sent: Thursday, April 11, 2024 10:50 AM
To: juzhe.zh...@rivai.ai
Cc: Li, Pan2 <pan2...@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.zh...@rivai.ai
<juzhe.zh...@rivai.ai> wrote:
>
> Thanks for fixing it. LGTM from my side.
>
> I prefer wait kito for another ACK.
>
> ________________________________
> juzhe.zh...@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...@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...@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
>
>
