* [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand @ 2023-08-07 12:22 pan2.li 2023-08-07 13:13 ` Richard Biener ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: pan2.li @ 2023-08-07 12:22 UTC (permalink / raw) To: gcc-patches; +Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw From: Pan Li <pan2.li@intel.com> In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will be only 1 operand when SET_SRC in create_pre_exit. For example as below. (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 (expr_list:REG_UNUSED (reg/i:TI 10 a0) (nil))) Unfortunately, SET_SRC requires at least 2 operands and then Segment Fault here. This patch would like to fix this ICE by adding operand length check before SET_SRC. Signed-off-by: Pan Li <pan2.li@intel.com> gcc/ChangeLog: * mode-switching.cc (create_pre_exit): Add operand length check. gcc/testsuite/ChangeLog: * gcc.target/riscv/mode-switch-ice-1.c: New test. --- gcc/mode-switching.cc | 2 ++ .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc index 64ae2bc29c3..cbf03c02159 100644 --- a/gcc/mode-switching.cc +++ b/gcc/mode-switching.cc @@ -411,6 +411,8 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) conflict with address reloads. */ if (copy_start >= ret_start && copy_start + copy_num <= ret_end + && GET_RTX_LENGTH (GET_CODE (return_copy_pat)) >= 2 + /* SET_SRC requires at least 2 operands. */ && OBJECT_P (SET_SRC (return_copy_pat))) forced_late_switch = true; break; diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c new file mode 100644 index 00000000000..1b34a471904 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +struct A { char e, f; }; + +struct B +{ + int g; + struct A h[4]; +}; + +extern void bar (int, int); + +struct B foo (void) +{ + bar (2, 1); +} + +void baz () +{ + foo (); +} -- 2.34.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand 2023-08-07 12:22 [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand pan2.li @ 2023-08-07 13:13 ` Richard Biener 2023-08-07 13:19 ` Li, Pan2 2023-08-07 16:31 ` Jeff Law 2023-08-08 3:09 ` [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER pan2.li ` (2 subsequent siblings) 3 siblings, 2 replies; 16+ messages in thread From: Richard Biener @ 2023-08-07 13:13 UTC (permalink / raw) To: pan2.li; +Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw On Mon, Aug 7, 2023 at 2:30 PM Pan Li via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. This patch would like to fix this ICE by adding operand > length check before SET_SRC. > > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add operand length check. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. > --- > gcc/mode-switching.cc | 2 ++ > .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ > 2 files changed, 24 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > > diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc > index 64ae2bc29c3..cbf03c02159 100644 > --- a/gcc/mode-switching.cc > +++ b/gcc/mode-switching.cc > @@ -411,6 +411,8 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) > conflict with address reloads. */ > if (copy_start >= ret_start > && copy_start + copy_num <= ret_end > + && GET_RTX_LENGTH (GET_CODE (return_copy_pat)) >= 2 > + /* SET_SRC requires at least 2 operands. */ maybe check that return_copy_pat is a SET instead? > && OBJECT_P (SET_SRC (return_copy_pat))) > forced_late_switch = true; > break; > diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > new file mode 100644 > index 00000000000..1b34a471904 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +struct A { char e, f; }; > + > +struct B > +{ > + int g; > + struct A h[4]; > +}; > + > +extern void bar (int, int); > + > +struct B foo (void) > +{ > + bar (2, 1); > +} > + > +void baz () > +{ > + foo (); > +} > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand 2023-08-07 13:13 ` Richard Biener @ 2023-08-07 13:19 ` Li, Pan2 2023-08-07 16:31 ` Jeff Law 1 sibling, 0 replies; 16+ messages in thread From: Li, Pan2 @ 2023-08-07 13:19 UTC (permalink / raw) To: Richard Biener Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang, jeffreyalaw > maybe check that return_copy_pat is a SET instead? You mean something like this? I tried this but I am not sure single_set is good enough here, because it should be "narrow" than GET_RTX_LENGTH. + && single_set (return_copy) Pan -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Monday, August 7, 2023 9:14 PM To: Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com Subject: Re: [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand On Mon, Aug 7, 2023 at 2:30 PM Pan Li via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. This patch would like to fix this ICE by adding operand > length check before SET_SRC. > > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add operand length check. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. > --- > gcc/mode-switching.cc | 2 ++ > .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ > 2 files changed, 24 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > > diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc > index 64ae2bc29c3..cbf03c02159 100644 > --- a/gcc/mode-switching.cc > +++ b/gcc/mode-switching.cc > @@ -411,6 +411,8 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) > conflict with address reloads. */ > if (copy_start >= ret_start > && copy_start + copy_num <= ret_end > + && GET_RTX_LENGTH (GET_CODE (return_copy_pat)) >= 2 > + /* SET_SRC requires at least 2 operands. */ maybe check that return_copy_pat is a SET instead? > && OBJECT_P (SET_SRC (return_copy_pat))) > forced_late_switch = true; > break; > diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > new file mode 100644 > index 00000000000..1b34a471904 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +struct A { char e, f; }; > + > +struct B > +{ > + int g; > + struct A h[4]; > +}; > + > +extern void bar (int, int); > + > +struct B foo (void) > +{ > + bar (2, 1); > +} > + > +void baz () > +{ > + foo (); > +} > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand 2023-08-07 13:13 ` Richard Biener 2023-08-07 13:19 ` Li, Pan2 @ 2023-08-07 16:31 ` Jeff Law 2023-08-08 3:14 ` Li, Pan2 1 sibling, 1 reply; 16+ messages in thread From: Jeff Law @ 2023-08-07 16:31 UTC (permalink / raw) To: Richard Biener, pan2.li Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang On 8/7/23 07:13, Richard Biener wrote: > On Mon, Aug 7, 2023 at 2:30 PM Pan Li via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> From: Pan Li <pan2.li@intel.com> >> >> In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will >> be only 1 operand when SET_SRC in create_pre_exit. For example as below. >> >> (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 >> (expr_list:REG_UNUSED (reg/i:TI 10 a0) >> (nil))) >> >> Unfortunately, SET_SRC requires at least 2 operands and then Segment >> Fault here. This patch would like to fix this ICE by adding operand >> length check before SET_SRC. >> >> Signed-off-by: Pan Li <pan2.li@intel.com> >> >> gcc/ChangeLog: >> >> * mode-switching.cc (create_pre_exit): Add operand length check. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/mode-switch-ice-1.c: New test. >> --- >> gcc/mode-switching.cc | 2 ++ >> .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ >> 2 files changed, 24 insertions(+) >> create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c >> >> diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc >> index 64ae2bc29c3..cbf03c02159 100644 >> --- a/gcc/mode-switching.cc >> +++ b/gcc/mode-switching.cc >> @@ -411,6 +411,8 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) >> conflict with address reloads. */ >> if (copy_start >= ret_start >> && copy_start + copy_num <= ret_end >> + && GET_RTX_LENGTH (GET_CODE (return_copy_pat)) >= 2 >> + /* SET_SRC requires at least 2 operands. */ > > maybe check that return_copy_pat is a SET instead? Or just ignore the case of an insn that is a toplevel USE/CLOBBER? I bet we even have a function for that somewhere given how commonly we need to do that. jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand 2023-08-07 16:31 ` Jeff Law @ 2023-08-08 3:14 ` Li, Pan2 0 siblings, 0 replies; 16+ messages in thread From: Li, Pan2 @ 2023-08-08 3:14 UTC (permalink / raw) To: Jeff Law, Richard Biener Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang Thanks Jeff. > Or just ignore the case of an insn that is a toplevel USE/CLOBBER? I > bet we even have a function for that somewhere given how commonly we > need to do that. I found there are something like "GET_CODE(x) == USE || GET_CODE(x) == CLOBBER" everywhere but lack of a function or macro. Thus, I add new macro for this as below v2. https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626590.html Pan -----Original Message----- From: Jeff Law <jeffreyalaw@gmail.com> Sent: Tuesday, August 8, 2023 12:31 AM To: Richard Biener <richard.guenther@gmail.com>; Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com> Subject: Re: [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand On 8/7/23 07:13, Richard Biener wrote: > On Mon, Aug 7, 2023 at 2:30 PM Pan Li via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> From: Pan Li <pan2.li@intel.com> >> >> In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will >> be only 1 operand when SET_SRC in create_pre_exit. For example as below. >> >> (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 >> (expr_list:REG_UNUSED (reg/i:TI 10 a0) >> (nil))) >> >> Unfortunately, SET_SRC requires at least 2 operands and then Segment >> Fault here. This patch would like to fix this ICE by adding operand >> length check before SET_SRC. >> >> Signed-off-by: Pan Li <pan2.li@intel.com> >> >> gcc/ChangeLog: >> >> * mode-switching.cc (create_pre_exit): Add operand length check. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/mode-switch-ice-1.c: New test. >> --- >> gcc/mode-switching.cc | 2 ++ >> .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ >> 2 files changed, 24 insertions(+) >> create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c >> >> diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc >> index 64ae2bc29c3..cbf03c02159 100644 >> --- a/gcc/mode-switching.cc >> +++ b/gcc/mode-switching.cc >> @@ -411,6 +411,8 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) >> conflict with address reloads. */ >> if (copy_start >= ret_start >> && copy_start + copy_num <= ret_end >> + && GET_RTX_LENGTH (GET_CODE (return_copy_pat)) >= 2 >> + /* SET_SRC requires at least 2 operands. */ > > maybe check that return_copy_pat is a SET instead? Or just ignore the case of an insn that is a toplevel USE/CLOBBER? I bet we even have a function for that somewhere given how commonly we need to do that. jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER 2023-08-07 12:22 [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand pan2.li 2023-08-07 13:13 ` Richard Biener @ 2023-08-08 3:09 ` pan2.li 2023-08-08 7:54 ` Richard Biener 2023-08-09 3:05 ` [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn pan2.li 2023-08-13 0:56 ` [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit pan2.li 3 siblings, 1 reply; 16+ messages in thread From: pan2.li @ 2023-08-08 3:09 UTC (permalink / raw) To: gcc-patches; +Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw From: Pan Li <pan2.li@intel.com> In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will be only 1 operand when SET_SRC in create_pre_exit. For example as below. (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 (expr_list:REG_UNUSED (reg/i:TI 10 a0) (nil))) Unfortunately, SET_SRC requires at least 2 operands and then Segment Fault here. This patch would like to fix this ICE by ignoring the USE and CLOBBER insn before SET_SRC. Signed-off-by: Pan Li <pan2.li@intel.com> gcc/ChangeLog: * mode-switching.cc (create_pre_exit): Add USE and CLOBBER check. * rtl.h (CLOBBER_OR_USE_P): New macro. gcc/testsuite/ChangeLog: * gcc.target/riscv/mode-switch-ice-1.c: New test. --- gcc/mode-switching.cc | 1 + gcc/rtl.h | 4 ++++ .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ 3 files changed, 27 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc index 64ae2bc29c3..8bb016a811d 100644 --- a/gcc/mode-switching.cc +++ b/gcc/mode-switching.cc @@ -411,6 +411,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) conflict with address reloads. */ if (copy_start >= ret_start && copy_start + copy_num <= ret_end + && !CLOBBER_OR_USE_P (return_copy_pat) && OBJECT_P (SET_SRC (return_copy_pat))) forced_late_switch = true; break; diff --git a/gcc/rtl.h b/gcc/rtl.h index e1c51156f90..cdc2941c210 100644 --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -1768,6 +1768,10 @@ extern const char * const reg_note_name[]; #define VAR_LOC_UNKNOWN_P(X) \ (GET_CODE (X) == CLOBBER && XEXP ((X), 0) == const0_rtx) +/* Determine whether RTX is USE or CLOBBER. */ +#define CLOBBER_OR_USE_P(RTX) \ + (GET_CODE (RTX) == USE || GET_CODE (RTX) == CLOBBER) + /* 1 if RTX is emitted after a call, but it should take effect before the call returns. */ #define NOTE_DURING_CALL_P(RTX) \ diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c new file mode 100644 index 00000000000..1b34a471904 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +struct A { char e, f; }; + +struct B +{ + int g; + struct A h[4]; +}; + +extern void bar (int, int); + +struct B foo (void) +{ + bar (2, 1); +} + +void baz () +{ + foo (); +} -- 2.34.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER 2023-08-08 3:09 ` [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER pan2.li @ 2023-08-08 7:54 ` Richard Biener 2023-08-08 9:34 ` Li, Pan2 0 siblings, 1 reply; 16+ messages in thread From: Richard Biener @ 2023-08-08 7:54 UTC (permalink / raw) To: pan2.li; +Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang, jeffreyalaw On Tue, Aug 8, 2023 at 5:10 AM Pan Li via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. This patch would like to fix this ICE by ignoring the USE > and CLOBBER insn before SET_SRC. Note I don't think we're supposed to arrive here and it needs more explanation as to why this is the correct fix to deal with the CLOBBER arriving here. Simply making sure to not ICE can lead to wrong-code. Earlier in the code we make sure to either have a single_set or a CLOBBER: /* If the return register is not (in its entirety) likely spilled, the return copy might be partially or completely optimized away. */ return_copy_pat = single_set (return_copy); if (!return_copy_pat) { return_copy_pat = PATTERN (return_copy); if (GET_CODE (return_copy_pat) != CLOBBER) break; else if (!optimize) { /* This might be (clobber (reg [<result>])) when not optimizing. Then check if the previous insn is the clobber for the return register. */ copy_reg = SET_DEST (return_copy_pat); ... } copy_reg = SET_DEST (return_copy_pat); so it looks "wrong" here already (try to enable RTL checking and extra checking). Well, we have #define SET_DEST(RTX) XC2EXP (RTX, 0, SET, CLOBBER) #define SET_SRC(RTX) XCEXP (RTX, 1, SET) so SET_DEST is fine for CLOBBER, so I suppose the SH4 code is incorrect. Please check when that was added and why and how that should be made more correct. Richard. > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add USE and CLOBBER check. > * rtl.h (CLOBBER_OR_USE_P): New macro. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. > --- > gcc/mode-switching.cc | 1 + > gcc/rtl.h | 4 ++++ > .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ > 3 files changed, 27 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > > diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc > index 64ae2bc29c3..8bb016a811d 100644 > --- a/gcc/mode-switching.cc > +++ b/gcc/mode-switching.cc > @@ -411,6 +411,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) > conflict with address reloads. */ > if (copy_start >= ret_start > && copy_start + copy_num <= ret_end > + && !CLOBBER_OR_USE_P (return_copy_pat) > && OBJECT_P (SET_SRC (return_copy_pat))) > forced_late_switch = true; > break; > diff --git a/gcc/rtl.h b/gcc/rtl.h > index e1c51156f90..cdc2941c210 100644 > --- a/gcc/rtl.h > +++ b/gcc/rtl.h > @@ -1768,6 +1768,10 @@ extern const char * const reg_note_name[]; > #define VAR_LOC_UNKNOWN_P(X) \ > (GET_CODE (X) == CLOBBER && XEXP ((X), 0) == const0_rtx) > > +/* Determine whether RTX is USE or CLOBBER. */ > +#define CLOBBER_OR_USE_P(RTX) \ > + (GET_CODE (RTX) == USE || GET_CODE (RTX) == CLOBBER) > + > /* 1 if RTX is emitted after a call, but it should take effect before > the call returns. */ > #define NOTE_DURING_CALL_P(RTX) \ > diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > new file mode 100644 > index 00000000000..1b34a471904 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +struct A { char e, f; }; > + > +struct B > +{ > + int g; > + struct A h[4]; > +}; > + > +extern void bar (int, int); > + > +struct B foo (void) > +{ > + bar (2, 1); > +} > + > +void baz () > +{ > + foo (); > +} > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER 2023-08-08 7:54 ` Richard Biener @ 2023-08-08 9:34 ` Li, Pan2 2023-08-08 9:42 ` Robin Dapp 0 siblings, 1 reply; 16+ messages in thread From: Li, Pan2 @ 2023-08-08 9:34 UTC (permalink / raw) To: Richard Biener Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang, jeffreyalaw Hi Richard B, Thanks for comments. Looks this SH4 parts existed in lcm.c from day 1 about 19 years ago, see this commit https://github.com/gcc-mirror/gcc/commit/cf99f196e2e18b62becbb660761fc2a586b85d78. As you said, the problem exists from the original commit. Aks the return_copy_pat can be either single_set or CLOBBER but it may still SET_SRC anyway. Thus, I think below parts should be only valid when single_set as the comments keep taking load/reloads. Then we can filter out the CLOBBER similar like "if (j >= 0 && GET_CODE (return_copy_pat) != CLOBBER)". Not quit sure if my understanding is correct as not familiar with SH4. if (j >= 0) { /* __builtin_return emits a sequence of loads to all return registers. One of them might require another mode than MODE_EXIT, even if it is unrelated to the return value, so we want to put the final mode switch after it. */ if (multi_reg_return && targetm.calls.function_value_regno_p (copy_start)) forced_late_switch = true; /* For the SH4, floating point loads depend on fpscr, thus we might need to put the final mode switch after the return value copy. That is still OK, because a floating point return value does not conflict with address reloads. */ if (copy_start >= ret_start && copy_start + copy_num <= ret_end && OBJECT_P (SET_SRC (return_copy_pat))) forced_late_switch = true; break; } > so it looks "wrong" here already (try to enable RTL checking and extra > checking). Could you please help to share how to enable checks here? Pan -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Tuesday, August 8, 2023 3:54 PM To: Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com Subject: Re: [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER On Tue, Aug 8, 2023 at 5:10 AM Pan Li via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. This patch would like to fix this ICE by ignoring the USE > and CLOBBER insn before SET_SRC. Note I don't think we're supposed to arrive here and it needs more explanation as to why this is the correct fix to deal with the CLOBBER arriving here. Simply making sure to not ICE can lead to wrong-code. Earlier in the code we make sure to either have a single_set or a CLOBBER: /* If the return register is not (in its entirety) likely spilled, the return copy might be partially or completely optimized away. */ return_copy_pat = single_set (return_copy); if (!return_copy_pat) { return_copy_pat = PATTERN (return_copy); if (GET_CODE (return_copy_pat) != CLOBBER) break; else if (!optimize) { /* This might be (clobber (reg [<result>])) when not optimizing. Then check if the previous insn is the clobber for the return register. */ copy_reg = SET_DEST (return_copy_pat); ... } copy_reg = SET_DEST (return_copy_pat); so it looks "wrong" here already (try to enable RTL checking and extra checking). Well, we have #define SET_DEST(RTX) XC2EXP (RTX, 0, SET, CLOBBER) #define SET_SRC(RTX) XCEXP (RTX, 1, SET) so SET_DEST is fine for CLOBBER, so I suppose the SH4 code is incorrect. Please check when that was added and why and how that should be made more correct. Richard. > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add USE and CLOBBER check. > * rtl.h (CLOBBER_OR_USE_P): New macro. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. > --- > gcc/mode-switching.cc | 1 + > gcc/rtl.h | 4 ++++ > .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ > 3 files changed, 27 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > > diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc > index 64ae2bc29c3..8bb016a811d 100644 > --- a/gcc/mode-switching.cc > +++ b/gcc/mode-switching.cc > @@ -411,6 +411,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) > conflict with address reloads. */ > if (copy_start >= ret_start > && copy_start + copy_num <= ret_end > + && !CLOBBER_OR_USE_P (return_copy_pat) > && OBJECT_P (SET_SRC (return_copy_pat))) > forced_late_switch = true; > break; > diff --git a/gcc/rtl.h b/gcc/rtl.h > index e1c51156f90..cdc2941c210 100644 > --- a/gcc/rtl.h > +++ b/gcc/rtl.h > @@ -1768,6 +1768,10 @@ extern const char * const reg_note_name[]; > #define VAR_LOC_UNKNOWN_P(X) \ > (GET_CODE (X) == CLOBBER && XEXP ((X), 0) == const0_rtx) > > +/* Determine whether RTX is USE or CLOBBER. */ > +#define CLOBBER_OR_USE_P(RTX) \ > + (GET_CODE (RTX) == USE || GET_CODE (RTX) == CLOBBER) > + > /* 1 if RTX is emitted after a call, but it should take effect before > the call returns. */ > #define NOTE_DURING_CALL_P(RTX) \ > diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > new file mode 100644 > index 00000000000..1b34a471904 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +struct A { char e, f; }; > + > +struct B > +{ > + int g; > + struct A h[4]; > +}; > + > +extern void bar (int, int); > + > +struct B foo (void) > +{ > + bar (2, 1); > +} > + > +void baz () > +{ > + foo (); > +} > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER 2023-08-08 9:34 ` Li, Pan2 @ 2023-08-08 9:42 ` Robin Dapp 2023-08-08 14:03 ` Li, Pan2 0 siblings, 1 reply; 16+ messages in thread From: Robin Dapp @ 2023-08-08 9:42 UTC (permalink / raw) To: Li, Pan2, Richard Biener Cc: rdapp.gcc, gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang, jeffreyalaw > Could you please help to share how to enable checks here? Build with --enable-checking or rather --enable-checking=extra. Regards Robin ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER 2023-08-08 9:42 ` Robin Dapp @ 2023-08-08 14:03 ` Li, Pan2 0 siblings, 0 replies; 16+ messages in thread From: Li, Pan2 @ 2023-08-08 14:03 UTC (permalink / raw) To: Robin Dapp, Richard Biener Cc: gcc-patches, juzhe.zhong, kito.cheng, Wang, Yanzhang, jeffreyalaw Thanks Robin, will have a try without this PATCH. Pan -----Original Message----- From: Robin Dapp <rdapp.gcc@gmail.com> Sent: Tuesday, August 8, 2023 5:43 PM To: Li, Pan2 <pan2.li@intel.com>; Richard Biener <richard.guenther@gmail.com> Cc: rdapp.gcc@gmail.com; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com>; jeffreyalaw@gmail.com Subject: Re: [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER > Could you please help to share how to enable checks here? Build with --enable-checking or rather --enable-checking=extra. Regards Robin ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn 2023-08-07 12:22 [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand pan2.li 2023-08-07 13:13 ` Richard Biener 2023-08-08 3:09 ` [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER pan2.li @ 2023-08-09 3:05 ` pan2.li 2023-08-11 20:56 ` Jeff Law 2023-08-13 0:56 ` [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit pan2.li 3 siblings, 1 reply; 16+ messages in thread From: pan2.li @ 2023-08-09 3:05 UTC (permalink / raw) To: gcc-patches; +Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw From: Pan Li <pan2.li@intel.com> In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will be only 1 operand when SET_SRC in create_pre_exit. For example as below. (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 (expr_list:REG_UNUSED (reg/i:TI 10 a0) (nil))) Unfortunately, SET_SRC requires at least 2 operands and then Segment Fault here. For SH4 part result in Segment Fault, it looks like only valid when the return_copy_pat is load or something like that. Thus, this patch try to fix it by ingnoring the CLOBBER insn for SH4. Signed-off-by: Pan Li <pan2.li@intel.com> gcc/ChangeLog: * mode-switching.cc (create_pre_exit): Add CLOBBER check. gcc/testsuite/ChangeLog: * gcc.target/riscv/mode-switch-ice-1.c: New test. --- gcc/mode-switching.cc | 2 +- .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc index 64ae2bc29c3..b034cf7d437 100644 --- a/gcc/mode-switching.cc +++ b/gcc/mode-switching.cc @@ -392,7 +392,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) && mode != targetm.mode_switching.exit (e)) break; } - if (j >= 0) + if (j >= 0 && GET_CODE (return_copy_pat) != CLOBBER) { /* __builtin_return emits a sequence of loads to all return registers. One of them might require diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c new file mode 100644 index 00000000000..1b34a471904 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +struct A { char e, f; }; + +struct B +{ + int g; + struct A h[4]; +}; + +extern void bar (int, int); + +struct B foo (void) +{ + bar (2, 1); +} + +void baz () +{ + foo (); +} -- 2.34.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn 2023-08-09 3:05 ` [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn pan2.li @ 2023-08-11 20:56 ` Jeff Law 2023-08-12 4:53 ` Li, Pan2 0 siblings, 1 reply; 16+ messages in thread From: Jeff Law @ 2023-08-11 20:56 UTC (permalink / raw) To: pan2.li, gcc-patches; +Cc: juzhe.zhong, kito.cheng, yanzhang.wang On 8/8/23 21:05, pan2.li@intel.com wrote: > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. For SH4 part result in Segment Fault, it looks like only > valid when the return_copy_pat is load or something like that. Thus, > this patch try to fix it by ingnoring the CLOBBER insn for SH4. > > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add CLOBBER check. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. > --- > gcc/mode-switching.cc | 2 +- > .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ > 2 files changed, 23 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > > diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc > index 64ae2bc29c3..b034cf7d437 100644 > --- a/gcc/mode-switching.cc > +++ b/gcc/mode-switching.cc > @@ -392,7 +392,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) > && mode != targetm.mode_switching.exit (e)) > break; > } > - if (j >= 0) > + if (j >= 0 && GET_CODE (return_copy_pat) != CLOBBER) > { > /* __builtin_return emits a sequence of loads to all > return registers. One of them might require I'd tend to prefer to guard the code a bit later so that the test for CLOBBERS is closer to the point where they're not allowed. ie > > /* For the SH4, floating point loads depend on fpscr, > thus we might need to put the final mode switch > after the return value copy. That is still OK, > because a floating point return value does not > conflict with address reloads. */ > if (copy_start >= ret_start > && copy_start + copy_num <= ret_end > && OBJECT_P (SET_SRC (return_copy_pat))) > forced_late_switch = true; > break; I'd put it in that code. Probably something like && GET_CODE (return_copy_pat) = SET && OBJECT_P (SET_SRC (return_copy_pat))) That way we make it clear that we should only be looking at SET_SRC of an actual SET. Is there some reason you put the guard earlier? jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn 2023-08-11 20:56 ` Jeff Law @ 2023-08-12 4:53 ` Li, Pan2 0 siblings, 0 replies; 16+ messages in thread From: Li, Pan2 @ 2023-08-12 4:53 UTC (permalink / raw) To: Jeff Law, gcc-patches; +Cc: juzhe.zhong, kito.cheng, Wang, Yanzhang Thanks Jeff for comments. > I'd put it in that code. Probably something like > && GET_CODE (return_copy_pat) = SET > && OBJECT_P (SET_SRC (return_copy_pat))) > That way we make it clear that we should only be looking at SET_SRC of > an actual SET. > Is there some reason you put the guard earlier? It looks like the code under "if (j >= 0)" only care about set insns, but I am not 100% confirmed this as the existing comments don't have a explicit description about this. I will have a try for your suggestion and will set the v4 if no surprise from tests. Pan -----Original Message----- From: Jeff Law <jeffreyalaw@gmail.com> Sent: Saturday, August 12, 2023 4:57 AM To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com> Subject: Re: [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn On 8/8/23 21:05, pan2.li@intel.com wrote: > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. For SH4 part result in Segment Fault, it looks like only > valid when the return_copy_pat is load or something like that. Thus, > this patch try to fix it by ingnoring the CLOBBER insn for SH4. > > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add CLOBBER check. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. > --- > gcc/mode-switching.cc | 2 +- > .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ > 2 files changed, 23 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c > > diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc > index 64ae2bc29c3..b034cf7d437 100644 > --- a/gcc/mode-switching.cc > +++ b/gcc/mode-switching.cc > @@ -392,7 +392,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) > && mode != targetm.mode_switching.exit (e)) > break; > } > - if (j >= 0) > + if (j >= 0 && GET_CODE (return_copy_pat) != CLOBBER) > { > /* __builtin_return emits a sequence of loads to all > return registers. One of them might require I'd tend to prefer to guard the code a bit later so that the test for CLOBBERS is closer to the point where they're not allowed. ie > > /* For the SH4, floating point loads depend on fpscr, > thus we might need to put the final mode switch > after the return value copy. That is still OK, > because a floating point return value does not > conflict with address reloads. */ > if (copy_start >= ret_start > && copy_start + copy_num <= ret_end > && OBJECT_P (SET_SRC (return_copy_pat))) > forced_late_switch = true; > break; I'd put it in that code. Probably something like && GET_CODE (return_copy_pat) = SET && OBJECT_P (SET_SRC (return_copy_pat))) That way we make it clear that we should only be looking at SET_SRC of an actual SET. Is there some reason you put the guard earlier? jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit 2023-08-07 12:22 [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand pan2.li ` (2 preceding siblings ...) 2023-08-09 3:05 ` [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn pan2.li @ 2023-08-13 0:56 ` pan2.li 2023-08-14 17:20 ` Jeff Law 3 siblings, 1 reply; 16+ messages in thread From: pan2.li @ 2023-08-13 0:56 UTC (permalink / raw) To: gcc-patches; +Cc: juzhe.zhong, kito.cheng, pan2.li, yanzhang.wang, jeffreyalaw From: Pan Li <pan2.li@intel.com> In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will be only 1 operand when SET_SRC in create_pre_exit. For example as below. (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 (expr_list:REG_UNUSED (reg/i:TI 10 a0) (nil))) Unfortunately, SET_SRC requires at least 2 operands and then Segment Fault here. For SH4 part result in Segment Fault, it looks like only valid when the return_copy_pat is load or something like that. Thus, this patch try to fix it by restrict the SET insn for SET_SRC. Signed-off-by: Pan Li <pan2.li@intel.com> gcc/ChangeLog: * mode-switching.cc (create_pre_exit): Add SET insn check. gcc/testsuite/ChangeLog: * gcc.target/riscv/mode-switch-ice-1.c: New test. --- gcc/mode-switching.cc | 1 + .../gcc.target/riscv/mode-switch-ice-1.c | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c diff --git a/gcc/mode-switching.cc b/gcc/mode-switching.cc index 64ae2bc29c3..f483c831c35 100644 --- a/gcc/mode-switching.cc +++ b/gcc/mode-switching.cc @@ -411,6 +411,7 @@ create_pre_exit (int n_entities, int *entity_map, const int *num_modes) conflict with address reloads. */ if (copy_start >= ret_start && copy_start + copy_num <= ret_end + && GET_CODE (return_copy_pat) == SET && OBJECT_P (SET_SRC (return_copy_pat))) forced_late_switch = true; break; diff --git a/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c new file mode 100644 index 00000000000..1b34a471904 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/mode-switch-ice-1.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +struct A { char e, f; }; + +struct B +{ + int g; + struct A h[4]; +}; + +extern void bar (int, int); + +struct B foo (void) +{ + bar (2, 1); +} + +void baz () +{ + foo (); +} -- 2.34.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit 2023-08-13 0:56 ` [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit pan2.li @ 2023-08-14 17:20 ` Jeff Law 2023-08-15 2:29 ` Li, Pan2 0 siblings, 1 reply; 16+ messages in thread From: Jeff Law @ 2023-08-14 17:20 UTC (permalink / raw) To: pan2.li, gcc-patches; +Cc: juzhe.zhong, kito.cheng, yanzhang.wang On 8/12/23 18:56, pan2.li@intel.com wrote: > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. For SH4 part result in Segment Fault, it looks like only > valid when the return_copy_pat is load or something like that. Thus, > this patch try to fix it by restrict the SET insn for SET_SRC. > > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add SET insn check. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. OK. Thanks for the updated version. jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit 2023-08-14 17:20 ` Jeff Law @ 2023-08-15 2:29 ` Li, Pan2 0 siblings, 0 replies; 16+ messages in thread From: Li, Pan2 @ 2023-08-15 2:29 UTC (permalink / raw) To: Jeff Law, gcc-patches; +Cc: juzhe.zhong, kito.cheng, Wang, Yanzhang Committed as passed both the bootstrap and regression test in x86, thanks Jeff. Pan -----Original Message----- From: Jeff Law <jeffreyalaw@gmail.com> Sent: Tuesday, August 15, 2023 1:21 AM To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Wang, Yanzhang <yanzhang.wang@intel.com> Subject: Re: [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit On 8/12/23 18:56, pan2.li@intel.com wrote: > From: Pan Li <pan2.li@intel.com> > > In same cases, like gcc/testsuite/gcc.dg/pr78148.c in RISC-V, there will > be only 1 operand when SET_SRC in create_pre_exit. For example as below. > > (insn 13 9 14 2 (clobber (reg/i:TI 10 a0)) "gcc/testsuite/gcc.dg/pr78148.c":24:1 -1 > (expr_list:REG_UNUSED (reg/i:TI 10 a0) > (nil))) > > Unfortunately, SET_SRC requires at least 2 operands and then Segment > Fault here. For SH4 part result in Segment Fault, it looks like only > valid when the return_copy_pat is load or something like that. Thus, > this patch try to fix it by restrict the SET insn for SET_SRC. > > Signed-off-by: Pan Li <pan2.li@intel.com> > > gcc/ChangeLog: > > * mode-switching.cc (create_pre_exit): Add SET insn check. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/mode-switch-ice-1.c: New test. OK. Thanks for the updated version. jeff ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-08-15 2:29 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-07 12:22 [PATCH v1] Mode-Switching: Fix SET_SRC ICE when only one operand pan2.li 2023-08-07 13:13 ` Richard Biener 2023-08-07 13:19 ` Li, Pan2 2023-08-07 16:31 ` Jeff Law 2023-08-08 3:14 ` Li, Pan2 2023-08-08 3:09 ` [PATCH v2] Mode-Switching: Fix SET_SRC ICE when USE or CLOBBER pan2.li 2023-08-08 7:54 ` Richard Biener 2023-08-08 9:34 ` Li, Pan2 2023-08-08 9:42 ` Robin Dapp 2023-08-08 14:03 ` Li, Pan2 2023-08-09 3:05 ` [PATCH v3] Mode-Switching: Fix SET_SRC ICE when CLOBBER insn pan2.li 2023-08-11 20:56 ` Jeff Law 2023-08-12 4:53 ` Li, Pan2 2023-08-13 0:56 ` [PATCH v4] Mode-Switching: Fix SET_SRC ICE for create_pre_exit pan2.li 2023-08-14 17:20 ` Jeff Law 2023-08-15 2:29 ` Li, Pan2
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).