[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #19 from Li Pan ---
Thanks Juzhe. Here is another example
-
#include
extern size_t get_new_vl ();
size_t
__attribute__((noinline))
get_vl (size_t *c)
{
size_t vl = c[0] + c[1];
return vl;
}
vbool64_t
test_fail_2 (vuint64m1_t a, unsigned long b, size_t *c)
{
return __riscv_vmsne_vx_u64m1_b64 (a, b, get_vl (c));
}
---
test_fail_2:
[30/37834]
addisp,sp,-16
sd ra,8(sp)
sd s0,0(sp)
csrrt0,vlenb
sub sp,sp,t0
vs1r.v v1,0(sp)
sub sp,sp,t0
vs1r.v v2,0(sp)
sub sp,sp,t0
vs1r.v v3,0(sp)
sub sp,sp,t0
vs1r.v v4,0(sp)
sub sp,sp,t0
vs1r.v v5,0(sp)
sub sp,sp,t0
vs1r.v v6,0(sp)
sub sp,sp,t0
vs1r.v v7,0(sp)
sub sp,sp,t0
vs1r.v v24,0(sp)
sub sp,sp,t0
vs1r.v v25,0(sp)
sub sp,sp,t0
vs1r.v v26,0(sp)
sub sp,sp,t0
vs1r.v v27,0(sp)
sub sp,sp,t0
vs1r.v v28,0(sp)
sub sp,sp,t0
vs1r.v v29,0(sp)
sub sp,sp,t0
vs1r.v v30,0(sp)
sub sp,sp,t0
vs1r.v v31,0(sp)
csrrt0,vlenb
sub sp,sp,t0
vs1r.v v8,0(sp)
mv s0,a0
mv a0,a1
callget_vl
vl1re64.v v8,0(sp)
vsetvli zero,a0,e64,m1,ta,ma
vmsne.vxv0,v8,s0
csrrt0,vlenb
add sp,sp,t0
csrrt0,vlenb
vl1re64.v v31,0(sp)
add sp,sp,t0
vl1re64.v v30,0(sp)
add sp,sp,t0
vl1re64.v v29,0(sp)
add sp,sp,t0
vl1re64.v v28,0(sp)
...
As I understand, these callee saved vector registers are not required if the
function body doesn't pollute these registers. Only the polluted registers
need to go in/out stack.
However, it is somehow one optimization here, we can consider to improve this
in GCC-15 if my understanding is correct.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #18 from JuzheZhong --- (In reply to Li Pan from comment #17) > According to the V abi, looks like the asm code tries to save/restore the > callee-saved registers when there is a call in function body. > > | Name| ABI Mnemonic | Meaning | Preserved across > calls? > = > > | v0 | | Argument register| No > | v1-v7 | | Callee-saved registers | Yes > | v8-v23 | | Argument registers | No > | v24-v31 | | Callee-saved registers | Yes I see, https://godbolt.org/z/7bx1EEdGn When we use 44 instead of get_vl (), the load/store instructions are gone.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #17 from Li Pan --- According to the V abi, looks like the asm code tries to save/restore the callee-saved registers when there is a call in function body. | Name| ABI Mnemonic | Meaning | Preserved across calls? = | v0 | | Argument register| No | v1-v7 | | Callee-saved registers | Yes | v8-v23 | | Argument registers | No | v24-v31 | | Callee-saved registers | Yes
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #16 from JuzheZhong --- This issue is not fully fixed since the fixed patch only fixes ICE but there is a regression in codegen: https://godbolt.org/z/4nvxeqb6K Terrible codege: test(__rvv_uint64m4_t): addisp,sp,-16 csrrt0,vlenb sd ra,8(sp) sub sp,sp,t0 vs1r.v v1,0(sp) sub sp,sp,t0 vs1r.v v2,0(sp) sub sp,sp,t0 vs1r.v v3,0(sp) sub sp,sp,t0 vs1r.v v4,0(sp) sub sp,sp,t0 vs1r.v v5,0(sp) sub sp,sp,t0 vs1r.v v6,0(sp) sub sp,sp,t0 vs1r.v v7,0(sp) sub sp,sp,t0 vs1r.v v24,0(sp) sub sp,sp,t0 vs1r.v v25,0(sp) sub sp,sp,t0 vs1r.v v26,0(sp) sub sp,sp,t0 vs1r.v v27,0(sp) sub sp,sp,t0 vs1r.v v28,0(sp) sub sp,sp,t0 vs1r.v v29,0(sp) sub sp,sp,t0 vs1r.v v30,0(sp) sub sp,sp,t0 csrrt0,vlenb sllit1,t0,2 vs1r.v v31,0(sp) sub sp,sp,t1 vs4r.v v8,0(sp) callget_vl() csrrt0,vlenb sllit1,t0,2 vl4re64.v v8,0(sp) csrrt0,vlenb add sp,sp,t1 vl1re64.v v31,0(sp) add sp,sp,t0 vl1re64.v v30,0(sp) add sp,sp,t0 vl1re64.v v29,0(sp) add sp,sp,t0 vl1re64.v v28,0(sp) add sp,sp,t0 vl1re64.v v27,0(sp) add sp,sp,t0 vl1re64.v v26,0(sp) add sp,sp,t0 vl1re64.v v25,0(sp) add sp,sp,t0 vl1re64.v v24,0(sp) add sp,sp,t0 vl1re64.v v7,0(sp) add sp,sp,t0 vl1re64.v v6,0(sp) add sp,sp,t0 vl1re64.v v5,0(sp) add sp,sp,t0 vl1re64.v v4,0(sp) add sp,sp,t0 vl1re64.v v3,0(sp) add sp,sp,t0 vl1re64.v v2,0(sp) add sp,sp,t0 vl1re64.v v1,0(sp) add sp,sp,t0 ld ra,8(sp) vsetvli zero,a0,e64,m4,ta,ma vmsne.viv0,v8,0 addisp,sp,16 jr ra
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 Andreas Schwab changed: What|Removed |Added Target Milestone|--- |14.0 Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #15 from Andreas Schwab --- Fixed.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #14 from GCC Commits ---
The master branch has been updated by Pan Li :
https://gcc.gnu.org/g:e40a3d86511efcea71e9eadde8fb9f96be52f790
commit r14-9908-ge40a3d86511efcea71e9eadde8fb9f96be52f790
Author: Pan Li
Date: Thu Apr 11 09:39:44 2024 +0800
RISC-V: Bugfix ICE for the vector return arg in mode switch
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
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #13 from Li Pan ---
overriding TARGET_CLASS_LIKELY_SPILLED_P hook may not be a fix as it will
generate sorts of spill for the below sample code.
vbool2_t test_vmfge_vf_f16m8_b2(vfloat16m8_t op1, float16_t op2, size_t vl) {
return __riscv_vmfge_vf_f16m8_b2(op1, op2, vl);
}
need to re-think from the mode-switch side.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #12 from Li Pan ---
#include
extern unsigned long get_vl ();
#if 0
#else
vint32m1_t test (vint32m1_t a)
{
unsigned b;
return __riscv_vadd_vx_i32m1 (a, b, get_vl ()); // No ICE
}
vbool16_t test (vuint64m4_t a)
{
unsigned long b;
return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ()); // ICE
}
#endif
This is comes from the below parts:
!(targetm.class_likely_spilled_p (REGNO_REG_CLASS (ret_start)));
For RVV, the reg_class values are listed as below. Because the Vector Mask has
only one reg, then it will be considered as likely spilled as the hook
TARGET_CLASS_LIKELY_SPILLED_P default returns true if reg_class_size[class] ==
1.
Not very sure if overriding TARGET_CLASS_LIKELY_SPILLED_P hook for riscv is a
reasonable fix, trying to understand TARGET_CLASS_LIKELY_SPILLED_P...
panli-reg_class_size[0]=0
panli-reg_class_size[1]=14
panli-reg_class_size[2]=26
panli-reg_class_size[3]=32
panli-reg_class_size[4]=32
panli-reg_class_size[5]=2
panli-reg_class_size[6]=1 <= VM
panli-reg_class_size[7]=31 <= VD
panli-reg_class_size[8]=32 <= V
panli-reg_class_size[9]=98
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #11 from Li Pan --- (In reply to Li Pan from comment #10) > The #define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN) > of the riscv backend doesn't honor vector mode. Then the below part > > 370 if (!targetm.calls.function_value_regno_p > (copy_start)) > > 371 copy_num = 0; > > 372 else > 373 copy_num = hard_regno_nregs (copy_start, > 374 GET_MODE (copy_reg)); > > will have copy_num == 0 and then went to a different code path. > > Let me run fully riscv regression test for this fix first. Maybe misunderstand here, need to double-check the vector ABI for return values.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #10 from Li Pan --- The #define FUNCTION_VALUE_REGNO_P(N) ((N) == GP_RETURN || (N) == FP_RETURN) of the riscv backend doesn't honor vector mode. Then the below part 370 if (!targetm.calls.function_value_regno_p (copy_start)) 371 copy_num = 0; 372 else 373 copy_num = hard_regno_nregs (copy_start, 374 GET_MODE (copy_reg)); will have copy_num == 0 and then went to a different code path. Let me run fully riscv regression test for this fix first.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #9 from Uroš Bizjak --- (In reply to Andrew Pinski from comment #2) > /* If we didn't see a full return value copy, verify that there >is a plausible reason for this. If some, but not all of the >return register is likely spilled, we can expect that there >is a copy for the likely spilled part. */ This part of the mode-switching pass is a real PITA. The trick here is with the calculation of forced_late_switch (but please see N.b. comment at the beginning of the function where some failed assumptions are described).
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #8 from Li Pan ---
Find an even simpler code for reproduction.
#include
extern unsigned long get_vl ();
vbool16_t test (vuint64m4_t a)
{
unsigned long b;
return __riscv_vmsne_vx_u64m4_b16 (a, b, get_vl ());
}
../__RISC-V_INSTALL___RV64/bin/riscv64-unknown-elf-g++ -O3 -march=rv64gcv -c
ref.c -S -o -
acc22d56e140220e7dc6c138918cb6754b6d1c0b enabled the vector abi by default, and
trigger this assert in create_pre_exit. Replace get_vl () with a local variable
could bypass this issue. will continue to investigate.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #7 from Li Pan --- Looks this commit from bisect acc22d56e140220e7dc6c138918cb6754b6d1c0b, will take a look into it.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #6 from JuzheZhong --- Definitely it is a regression: https://compiler-explorer.com/z/e68x5sT9h GCC 13.2 is ok, but GCC 14 ICE. I think you should bisect first.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #5 from Li Pan ---
(In reply to Kito Cheng from comment #4)
> Reduced case:
> ```c
> typedef long c;
> #pragma riscv intrinsic "vector"
> template struct d {};
> struct e {
> using f = d<0>;
> };
> struct g {
> using f = e::f;
> };
> template using h = g::f;
> template long k(d);
> vbool16_t j(vuint64m4_t a) {
> c b;
> return __riscv_vmsne_vx_u64m4_b16(a, b, k(h()));
> }
>
> ```
Thanks Kito, reproduced on reduced case with option "riscv64-unknown-elf-g++
-O2 -march=rv64gcv". will take a look into it.
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
/home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/mode-switching.cc:451
0x3979e9e optimize_mode_switching
/home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/mode-switching.cc:849
0x397b9bc execute
/home/pli/gcc/555/riscv-gnu-toolchain/gcc/__RISCV_BUILD__/../gcc/mode-switching.cc:1324
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639
--- Comment #4 from Kito Cheng ---
Reduced case:
```c
typedef long c;
#pragma riscv intrinsic "vector"
template struct d {};
struct e {
using f = d<0>;
};
struct g {
using f = e::f;
};
template using h = g::f;
template long k(d);
vbool16_t j(vuint64m4_t a) {
c b;
return __riscv_vmsne_vx_u64m4_b16(a, b, k(h()));
}
```
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #3 from Li Pan --- Reproduced from my side too.
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 --- Comment #2 from Andrew Pinski --- /* If we didn't see a full return value copy, verify that there is a plausible reason for this. If some, but not all of the return register is likely spilled, we can expect that there is a copy for the likely spilled part. */ gcc_assert (!nregs || forced_late_switch || short_block || !(targetm.class_likely_spilled_p (REGNO_REG_CLASS (ret_start))) || nregs != REG_NREGS (ret_reg) /* For multi-hard-register floating point values, sometimes the likely-spilled part is ordinarily copied first, then the other part is set with an arithmetic operation. This doesn't actually cause reload failures, so let it pass. */ || (GET_MODE_CLASS (GET_MODE (ret_reg)) != MODE_INT && nregs != 1));
[Bug target/114639] [riscv] ICE in create_pre_exit, at mode-switching.cc:451
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114639 Kito Cheng changed: What|Removed |Added Status|UNCONFIRMED |NEW Ever confirmed|0 |1 Last reconfirmed||2024-04-08 --- Comment #1 from Kito Cheng --- Confirmed, and try to reducing the testcase.
