* [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and cc use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
@ 2013-10-05 20:22 ` olegendo at gcc dot gnu.org
2013-10-05 21:16 ` [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use olegendo at gcc dot gnu.org
` (15 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: olegendo at gcc dot gnu.org @ 2013-10-05 20:22 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2013-10-05
Component|target |rtl-optimization
Summary|[SH] wrong code with subc |icvt (after combine) puts
|and movsicc |ccreg clobbering insn
|(-mpretend-cmove) after ce2 |between ccset insn and cc
| |use
Ever confirmed|0 |1
--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> ---
This seems to be a bug in ifcvt.c
Reduced test cases:
int
test_subc (int q, int read, int end)
{
return q < read ? (read - q - 1) : (end - q);
}
int
test_addc (int q, int read, int end)
{
return q < read ? (read + q + 1) : (end + q);
}
compiled with -O2 -m4 -mpretend-cmove:
_test_subc:
sett
mov r5,r0
sub r4,r6
subc r4,r0 ! 36 *subc
bf 0f ! 34 *movsicc_t_true/1
mov r6,r0
0:
rts
nop
_test_addc:
sett
mov r4,r0
add r4,r6
addc r5,r0 ! 34 *addc
bf 0f ! 32 *movsicc_t_true/1
mov r6,r0
0:
rts
nop
In both above cases the comparison 'q < read' is eliminated, because ifcvt
moves the T_REG clobbering subc/addc insn between the 'q < read' comparison
(sets T_REG) and the conditional branch (uses T_REG).
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
2013-10-05 20:22 ` [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and cc use olegendo at gcc dot gnu.org
@ 2013-10-05 21:16 ` olegendo at gcc dot gnu.org
2013-10-05 22:17 ` olegendo at gcc dot gnu.org
` (14 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: olegendo at gcc dot gnu.org @ 2013-10-05 21:16 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> ---
The problem also happens on 4.8.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
2013-10-05 20:22 ` [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and cc use olegendo at gcc dot gnu.org
2013-10-05 21:16 ` [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use olegendo at gcc dot gnu.org
@ 2013-10-05 22:17 ` olegendo at gcc dot gnu.org
2013-10-05 23:23 ` steven at gcc dot gnu.org
` (13 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: olegendo at gcc dot gnu.org @ 2013-10-05 22:17 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |steven at gcc dot gnu.org
--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> ---
The problematic transformation happens in ifcvt.c (noce_try_cmove_arith).
I've only briefly looked over that code and can think of the following options
how to fix the issue:
1) Check whether insn_a and insn_b modify the src reg (ccreg) of the
conditional branch. If they do, don't do the transformation.
On SH, the resulting code would be the same as not specifying -mpretend-cmove:
cmp/ge r5,r4
mov r6,r0
bf/s .L6
sub r4,r0
rts
nop
.L6:
sett
mov r5,r0
rts
subc r4,r0
2) Check whether insn_a and insn_b modify the src reg (ccreg) of the
conditional branch. If they do, try to move the insn that sets the ccreg
before the conditional branch, _after_ the emitted insn_a and insn_b and right
before the conditional move.
On SH, the resulting code would look something like this:
sett
mov r5,r0
sub r4,r6
subc r4,r0
cmp/ge r5,r4
bf 0f
mov r6,r0
0:
rts
nop
Steven, since you're RTL optimizers reviewer, could you please provide some
feedback regarding the matter?
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] icvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (2 preceding siblings ...)
2013-10-05 22:17 ` olegendo at gcc dot gnu.org
@ 2013-10-05 23:23 ` steven at gcc dot gnu.org
2015-09-11 11:12 ` [Bug rtl-optimization/58517] ifcvt " ktkachov at gcc dot gnu.org
` (12 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: steven at gcc dot gnu.org @ 2013-10-05 23:23 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #4 from Steven Bosscher <steven at gcc dot gnu.org> ---
So if I understand correctly, this is what happens (sorry, reading
LIPSy RTL is still just too unnatural to me! ;-))
before ifcvt-after-combine:
r147:=(r424>r178)
r190:=r423-r189
if (r147==0) goto L433
{r190:=(~r189)+r424; r147:=X}
L433:
after ifcvt-after-combine:
r147:=(r424>r189)
r190:=r423-r189
{r945:=(~r189)+r424; r147:=X}
r946:=r423-r189
r190:=(r147==0) ? r946:r945 // but r147 was clobbered!
r147:=(r190==0)
if (r147==0) goto L501
This is noce_try_cmove_arith:
/* if (test) x = a + b; else x = c - d;
=> y = a + b;
x = c - d;
if (test)
x = y;
*/
with "(test)" being clobbered by the assignment to "y" or "x" on the
code after the transformation has been applied. noce_try_cmove_arith
does not expect that the arithmetic insns emitted to load vtrue or
vfalse into general operands may clobber the condition. There should
be tests like this:
Index: ifcvt.c
===================================================================
--- ifcvt.c (revision 203224)
+++ ifcvt.c (working copy)
@@ -1573,6 +1573,8 @@ noce_try_cmove_arith (struct noce_if_info *if_info
optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn_a)));
if (insn_cost == 0 || insn_cost > COSTS_N_INSNS (if_info->branch_cost))
return FALSE;
+ if (modified_in_p (if_info->cond, insn_a))
+ return FALSE;
}
else
insn_cost = 0;
@@ -1584,6 +1586,8 @@ noce_try_cmove_arith (struct noce_if_info *if_info
optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn_b)));
if (insn_cost == 0 || insn_cost > COSTS_N_INSNS (if_info->branch_cost))
return FALSE;
+ if (modified_in_p (if_info->cond, insn_b))
+ return FALSE;
}
/* Possibly rearrange operands to make things come out more natural. */
That's option 1 of comment #3. Option 2 is only viable if the insn
computing "cond" is nearby (i.e. immediately above insn_a / insn_b),
otherwise it'll be necessary to solve a code motion dataflow problem.
Within one basic block that still would be doable (modified_between_p),
but I doubt it's worth the trouble.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (3 preceding siblings ...)
2013-10-05 23:23 ` steven at gcc dot gnu.org
@ 2015-09-11 11:12 ` ktkachov at gcc dot gnu.org
2023-06-02 1:38 ` pinskia at gcc dot gnu.org
` (11 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: ktkachov at gcc dot gnu.org @ 2015-09-11 11:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
ktkachov at gcc dot gnu.org changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |ktkachov at gcc dot gnu.org
--- Comment #6 from ktkachov at gcc dot gnu.org ---
(In reply to Oleg Endo from comment #5)
> Kyrill, since you are currently working on ifcvt related things, could you
> maybe have a look at this issue? I haven't checked it in a while, so I
> don't know if it's still a problem.
Huh, I wasn't aware of this BZ.
This looks eerily similar to what I think is the root cause of PR 67481, which
I'm working on now.
After I'm done with PR 67481 it would be interesting to have a look whether
they are indeed the same issue
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (4 preceding siblings ...)
2015-09-11 11:12 ` [Bug rtl-optimization/58517] ifcvt " ktkachov at gcc dot gnu.org
@ 2023-06-02 1:38 ` pinskia at gcc dot gnu.org
2023-06-02 1:43 ` pinskia at gcc dot gnu.org
` (10 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-02 1:38 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55237
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55237&action=edit
Patch which I think will fix this
This is option 1 of comment #3 though with an updated version.
I have not tested this at all but I think it will work.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (5 preceding siblings ...)
2023-06-02 1:38 ` pinskia at gcc dot gnu.org
@ 2023-06-02 1:43 ` pinskia at gcc dot gnu.org
2023-06-02 1:44 ` pinskia at gcc dot gnu.org
` (9 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-02 1:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The reason why r6-3654-g6b7e867187889 didn't fix this case is because it was
not looking into clobbers only the set side.
Note the conditional in my patch should have been
if (reg_overlap_mentioned_p (DF_REF_REG (def), cond))
Maybe it should be bb_valid_for_noce_process_p instead, I am not 100% sure on
that.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (6 preceding siblings ...)
2023-06-02 1:43 ` pinskia at gcc dot gnu.org
@ 2023-06-02 1:44 ` pinskia at gcc dot gnu.org
2023-06-02 7:28 ` pinskia at gcc dot gnu.org
` (8 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-02 1:44 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed|2013-10-05 00:00:00 |2023-6-1
--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note the wrong code can be still reproduced in GCC 13 too.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (7 preceding siblings ...)
2023-06-02 1:44 ` pinskia at gcc dot gnu.org
@ 2023-06-02 7:28 ` pinskia at gcc dot gnu.org
2023-06-02 7:57 ` pinskia at gcc dot gnu.org
` (7 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-02 7:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #9)
> The reason why r6-3654-g6b7e867187889 didn't fix this case is because it was
> not looking into clobbers only the set side.
>
> Note the conditional in my patch should have been
> if (reg_overlap_mentioned_p (DF_REF_REG (def), cond))
>
> Maybe it should be bb_valid_for_noce_process_p instead, I am not 100% sure
> on that.
This does not fix the issue as (In reply to Andrew Pinski from comment #9)
> The reason why r6-3654-g6b7e867187889 didn't fix this case is because it was
> not looking into clobbers only the set side.
>
> Note the conditional in my patch should have been
> if (reg_overlap_mentioned_p (DF_REF_REG (def), cond))
>
> Maybe it should be bb_valid_for_noce_process_p instead, I am not 100% sure
> on that.
This patch is not enough. Though the patch in comment #4 is not enough any more
either.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (8 preceding siblings ...)
2023-06-02 7:28 ` pinskia at gcc dot gnu.org
@ 2023-06-02 7:57 ` pinskia at gcc dot gnu.org
2023-06-03 8:58 ` olegendo at gcc dot gnu.org
` (6 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-02 7:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55237|0 |1
is obsolete| |
--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55239
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55239&action=edit
Patch which does work on this
Though, I need to double to make sure it works for other cases still.
sh is the case where we have a non-CC mode register for the flags which is why
this was harder than other targets.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (9 preceding siblings ...)
2023-06-02 7:57 ` pinskia at gcc dot gnu.org
@ 2023-06-03 8:58 ` olegendo at gcc dot gnu.org
2023-06-03 9:02 ` pinskia at gcc dot gnu.org
` (5 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: olegendo at gcc dot gnu.org @ 2023-06-03 8:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #13 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #12)
> Created attachment 55239 [details]
> Patch which does work on this
>
> Though, I need to double to make sure it works for other cases still.
> sh is the case where we have a non-CC mode register for the flags which is
> why this was harder than other targets.
Thanks for looking at this (after 10 years .. wow, time flies).
> diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
> index 4622dba0121..ae1a0f7eb7c 100644
> --- a/gcc/config/sh/sh.md
> +++ b/gcc/config/sh/sh.md
> @@ -1510,7 +1510,7 @@ (define_expand "movsicc"
> rtx op0 = XEXP (operands[1], 0);
> rtx op1 = XEXP (operands[1], 1);
>
> - if (! currently_expanding_to_rtl)
> + if (0 && ! currently_expanding_to_rtl)
> FAIL;
>
> switch (code)
> diff --git a/gcc/config/sh/t-sh b/gcc/config/sh/t-sh
> index 1cc8c39d2a8..8ffcc288cf3 100644
> --- a/gcc/config/sh/t-sh
> +++ b/gcc/config/sh/t-sh
> @@ -89,8 +89,8 @@ MULTILIB_OSDIRNAMES = \
> m4a=!m4a $(OTHER_ENDIAN)/m4a=!$(OTHER_ENDIAN)/m4a \
> m4al=!m4al $(OTHER_ENDIAN)/m4al=!$(OTHER_ENDIAN)/m4al
>
> -$(out_object_file): gt-sh.h
> -gt-sh.h : s-gtype ; @true
> +#$(out_object_file): gt-sh.h
> +#gt-sh.h : s-gtype ; @true
>
> # Local Variables:
> # mode: Makefile
Are these hunks intentional?
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (10 preceding siblings ...)
2023-06-03 8:58 ` olegendo at gcc dot gnu.org
@ 2023-06-03 9:02 ` pinskia at gcc dot gnu.org
2023-06-03 21:50 ` pinskia at gcc dot gnu.org
` (4 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-03 9:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #13)
> (In reply to Andrew Pinski from comment #12)
> > Created attachment 55239 [details]
> > Patch which does work on this
> >
> > Though, I need to double to make sure it works for other cases still.
> > sh is the case where we have a non-CC mode register for the flags which is
> > why this was harder than other targets.
>
> Thanks for looking at this (after 10 years .. wow, time flies).
>
>
> > diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
> > index 4622dba0121..ae1a0f7eb7c 100644
> > --- a/gcc/config/sh/sh.md
> > +++ b/gcc/config/sh/sh.md
> > @@ -1510,7 +1510,7 @@ (define_expand "movsicc"
> > rtx op0 = XEXP (operands[1], 0);
> > rtx op1 = XEXP (operands[1], 1);
> >
> > - if (! currently_expanding_to_rtl)
> > + if (0 && ! currently_expanding_to_rtl)
> > FAIL;
> >
> > switch (code)
> > diff --git a/gcc/config/sh/t-sh b/gcc/config/sh/t-sh
> > index 1cc8c39d2a8..8ffcc288cf3 100644
> > --- a/gcc/config/sh/t-sh
> > +++ b/gcc/config/sh/t-sh
> > @@ -89,8 +89,8 @@ MULTILIB_OSDIRNAMES = \
> > m4a=!m4a $(OTHER_ENDIAN)/m4a=!$(OTHER_ENDIAN)/m4a \
> > m4al=!m4al $(OTHER_ENDIAN)/m4al=!$(OTHER_ENDIAN)/m4al
> >
> > -$(out_object_file): gt-sh.h
> > -gt-sh.h : s-gtype ; @true
> > +#$(out_object_file): gt-sh.h
> > +#gt-sh.h : s-gtype ; @true
> >
> > # Local Variables:
> > # mode: Makefile
>
> Are these hunks intentional?
No. The second one was me trying to figure out why if I did make clean, the
build would then fail, I thought it wad related to that but it was a m2 (the
language) vs having a multilib dir named m2.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (11 preceding siblings ...)
2023-06-03 9:02 ` pinskia at gcc dot gnu.org
@ 2023-06-03 21:50 ` pinskia at gcc dot gnu.org
2023-06-03 22:18 ` pinskia at gcc dot gnu.org
` (3 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-03 21:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I was thinking of improving this is modify noce_get_condition here (we still
need the previous patch just in case we still get the T register).
Right now noce_get_condition will stop at:
(eq (reg:SI 147 t) (const_int 0 [0]))
Because of:
/* If the condition variable is a register and is MODE_INT, accept it. */
cond = XEXP (SET_SRC (set), 0);
tmp = XEXP (cond, 0);
if (REG_P (tmp) && GET_MODE_CLASS (GET_MODE (tmp)) == MODE_INT
&& (GET_MODE (tmp) != BImode
|| !targetm.small_register_classes_for_mode_p (BImode)))
But since 147 is a hard register but maybe since it is hard register and
REGNO_REG_CLASS (147) == T_REGS and the register class of T_REGS is only one
register so consider a small register class.
This I think is related to the option 2 in comment #3.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (12 preceding siblings ...)
2023-06-03 21:50 ` pinskia at gcc dot gnu.org
@ 2023-06-03 22:18 ` pinskia at gcc dot gnu.org
2023-06-03 22:23 ` olegendo at gcc dot gnu.org
` (2 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-03 22:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Now I am curious if T_REG should be BImode rather than SImode ... Then ifcvt.cc
would not have to be modified. I Know BImode is newer than when sh target was
added but maybe if someone cares about the sh target could try to modify the
backend to use BImode for T_REG here.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (13 preceding siblings ...)
2023-06-03 22:18 ` pinskia at gcc dot gnu.org
@ 2023-06-03 22:23 ` olegendo at gcc dot gnu.org
2023-06-03 22:59 ` pinskia at gcc dot gnu.org
2023-06-04 0:48 ` pinskia at gcc dot gnu.org
16 siblings, 0 replies; 18+ messages in thread
From: olegendo at gcc dot gnu.org @ 2023-06-03 22:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #17 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #16)
> Now I am curious if T_REG should be BImode rather than SImode ... Then
> ifcvt.cc would not have to be modified. I Know BImode is newer than when sh
> target was added but maybe if someone cares about the sh target could try to
> modify the backend to use BImode for T_REG here.
Yeah, I know. Back then I was entertaining the idea. But T-bit being SImode is
so intertwined with all other things in the backend, it looks like a big job to
undo/redo that. For example there are patterns that use the T bit in other
arithmetic insns patterns. Would need to check what would happen to those.
Sounds like a can of worms.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (14 preceding siblings ...)
2023-06-03 22:23 ` olegendo at gcc dot gnu.org
@ 2023-06-03 22:59 ` pinskia at gcc dot gnu.org
2023-06-04 0:48 ` pinskia at gcc dot gnu.org
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-03 22:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #18 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55250
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55250&action=edit
This patch also fixes the issue by looking back
Here is a patch which does the lookback if the reg is a hard register and it is
considered a small reg class.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Bug rtl-optimization/58517] ifcvt (after combine) puts ccreg clobbering insn between ccset insn and ccreg use
2013-09-23 21:50 [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2 olegendo at gcc dot gnu.org
` (15 preceding siblings ...)
2023-06-03 22:59 ` pinskia at gcc dot gnu.org
@ 2023-06-04 0:48 ` pinskia at gcc dot gnu.org
16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-04 0:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
--- Comment #19 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #18)
> Created attachment 55250 [details]
> This patch also fixes the issue by looking back
>
> Here is a patch which does the lookback if the reg is a hard register and it
> is considered a small reg class.
Note with this version of the patch, you need to disable check against
currently_expanding_to_rtl in movsicc too. Otherwise you won't get the ifcvt
happening in general.
^ permalink raw reply [flat|nested] 18+ messages in thread