public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/58517] New: [SH] wrong code with subc and movsicc (-mpretend-cmove) after ce2
@ 2013-09-23 21:50 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
` (16 more replies)
0 siblings, 17 replies; 18+ messages in thread
From: olegendo at gcc dot gnu.org @ 2013-09-23 21:50 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58517
Bug ID: 58517
Summary: [SH] wrong code with subc and movsicc
(-mpretend-cmove) after ce2
Product: gcc
Version: 4.9.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: olegendo at gcc dot gnu.org
Target: sh*-*-*
While working on something else, I've noticed the following sequence in the
CSiBE set in the file zlib-1.1.4/infblock.s, compiling with -m4-single -ml -O2
-mpretend-cmove (trunk rev 201978):
.L257:
sett ! 1740 sett [length = 2]
mov r2,r13 ! 2180 movsi_ie/2 [length = 2]
sub r5,r1 ! 1588 *subsi3_internal [length = 2]
subc r5,r13 ! 1741 *subc [length = 2]
bt 0f ! 1589 *movsicc_t_false/1 [length = 4]
mov r1,r13
0:
tst r13,r13 ! 435 cmpeqsi_t/1 [length = 2]
bf .L67 ! 436 *cbranch_t [length = 2]
bra .L124
nop ! 2774 jump_compact [length = 4]
The subc insn clobbers the T_REG and the following 'bt 0f' just looks wrong.
It seems that something goes wrong and the insn that does the test before
*movsicc_t_false gets eliminated because the sett-subc sequence is placed in
between.
After the combine pass the sequence looks like:
(note 422 421 423 45 [bb 45] NOTE_INSN_BASIC_BLOCK)
(insn 423 422 432 45 (set (reg:SI 147 t)
(gtu:SI (reg/f:SI 424 [ D.2926 ])
(reg/v/f:SI 189 [ q ]))) infblock.c:202 34 {cmpgtusi_t}
(nil))
(insn 432 423 424 45 (set (reg/v:SI 190 [ n ])
(minus:SI (reg/v/f:SI 423 [ q ])
(reg/v/f:SI 189 [ q ]))) infblock.c:202 76 {*subsi3_internal}
(expr_list:REG_DEAD (reg/v/f:SI 423 [ q ])
(nil)))
(jump_insn 424 432 425 45 (set (pc)
(if_then_else (eq (reg:SI 147 t)
(const_int 0 [0]))
(label_ref:SI 433)
(pc))) infblock.c:202 295 {*cbranch_t}
(expr_list:REG_DEAD (reg:SI 147 t)
(expr_list:REG_BR_PROB (const_int 5000 [0x1388])
(nil)))
-> 433)
(note 425 424 426 46 [bb 46] NOTE_INSN_BASIC_BLOCK)
(note 426 425 427 46 NOTE_INSN_DELETED)
(insn 427 426 433 46 (parallel [
(set (reg/v:SI 190 [ n ])
(plus:SI (not:SI (reg/v/f:SI 189 [ q ]))
(reg/f:SI 424 [ D.2926 ])))
(clobber (reg:SI 147 t))
]) infblock.c:202 74 {*subc}
(expr_list:REG_UNUSED (reg:SI 147 t)
(expr_list:REG_DEAD (reg/f:SI 424 [ D.2926 ])
(nil))))
(code_label 433 427 434 47 70 "" [1 uses])
After the following pass ce2 it becomes:
(note 422 421 423 45 [bb 45] NOTE_INSN_BASIC_BLOCK)
(insn 423 422 432 45 (set (reg:SI 147 t)
(gtu:SI (reg/f:SI 424 [ D.2926 ])
(reg/v/f:SI 189 [ q ]))) infblock.c:202 34 {cmpgtusi_t}
(nil))
(insn 432 423 1587 45 (set (reg/v:SI 190 [ n ])
(minus:SI (reg/v/f:SI 423 [ q ])
(reg/v/f:SI 189 [ q ]))) infblock.c:202 76 {*subsi3_internal}
(expr_list:REG_DEAD (reg/v/f:SI 423 [ q ])
(nil)))
(insn 1587 432 1588 45 (parallel [
(set (reg:SI 945)
(plus:SI (not:SI (reg/v/f:SI 189 [ q ]))
(reg/f:SI 424 [ D.2926 ])))
(clobber (reg:SI 147 t))
]) infblock.c:202 74 {*subc}
(nil))
(insn 1588 1587 1589 45 (set (reg:SI 946)
(minus:SI (reg/v/f:SI 423 [ q ])
(reg/v/f:SI 189 [ q ]))) infblock.c:202 76 {*subsi3_internal}
(nil))
(insn 1589 1588 435 45 (set (reg/v:SI 190 [ n ])
(if_then_else:SI (eq (reg:SI 147 t)
(const_int 0 [0]))
(reg:SI 946)
(reg:SI 945))) infblock.c:202 54 {*movsicc_t_false}
(nil))
(insn 435 1589 436 45 (set (reg:SI 147 t)
(eq:SI (reg/v:SI 190 [ n ])
(const_int 0 [0]))) infblock.c:202 17 {cmpeqsi_t}
(nil))
(jump_insn 436 435 1378 45 (set (pc)
(if_then_else (eq (reg:SI 147 t)
(const_int 0 [0]))
(label_ref 501)
(pc))) infblock.c:202 295 {*cbranch_t}
(expr_list:REG_DEAD (reg:SI 147 t)
(expr_list:REG_BR_PROB (const_int 10000 [0x2710])
(nil)))
-> 501)
which is wrong. I haven't checked whether it also happens on current trunk.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [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
end of thread, other threads:[~2023-06-04 0:48 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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
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
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
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).