public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/59291] New: [SH] Group T bit related isnsn before combine
@ 2013-11-25 17:42 olegendo at gcc dot gnu.org
2024-05-27 19:49 ` [Bug target/59291] [SH] Group T bit related insns " pietro.gcc at sociotechnical dot xyz
2024-05-27 20:50 ` olegendo at gcc dot gnu.org
0 siblings, 2 replies; 3+ messages in thread
From: olegendo at gcc dot gnu.org @ 2013-11-25 17:42 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59291
Bug ID: 59291
Summary: [SH] Group T bit related isnsn before combine
Product: gcc
Version: 4.9.0
Status: UNCONFIRMED
Severity: enhancement
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: olegendo at gcc dot gnu.org
Target: sh*-*-*
The following example function
struct result
{
int a, b;
};
result test2 (int a, int b, int d)
{
result r;
r.a = a == b;
r.b = d + b + 1;
return r;
};
compiled with -O2 -m4 -ml results in
cmp/eq r5,r4
add r5,r6
mov r6,r1
movt r0
rts
add #1,r1 <<< missed addc combine opportunity.
Initially the function is expanded to...
(notice the eq:SI that sets the T_REG and the movt that stores the comparison
result are next to each other)
(note 1 0 6 NOTE_INSN_DELETED)
(note 6 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 2 6 3 2 (set (reg/v:SI 166 [ a ])
(reg:SI 4 r4 [ a ])) sh_tmp.cpp:7 -1
(nil))
(insn 3 2 4 2 (set (reg/v:SI 167 [ b ])
(reg:SI 5 r5 [ b ])) sh_tmp.cpp:7 -1
(nil))
(insn 4 3 5 2 (set (reg/v:SI 168 [ d ])
(reg:SI 6 r6 [ d ])) sh_tmp.cpp:7 -1
(nil))
(note 5 4 8 2 NOTE_INSN_FUNCTION_BEG)
(insn 8 5 9 2 (set (reg:SI 147 t)
(eq:SI (reg/v:SI 166 [ a ])
(reg/v:SI 167 [ b ]))) sh_tmp.cpp:9 -1
(nil))
(insn 9 8 10 2 (set (reg:SI 170 [ D.1945 ])
(reg:SI 147 t)) sh_tmp.cpp:9 -1
(nil))
(insn 10 9 11 2 (set (subreg:SI (reg:DI 164 [ D.1936 ]) 0)
(reg:SI 170 [ D.1945 ])) sh_tmp.cpp:11 -1
(nil))
(insn 11 10 12 2 (set (reg:SI 171 [ D.1946 ])
(plus:SI (reg/v:SI 168 [ d ])
(reg/v:SI 167 [ b ]))) sh_tmp.cpp:10 -1
(nil))
(insn 12 11 13 2 (set (reg:SI 172 [ D.1946 ])
(plus:SI (reg:SI 171 [ D.1946 ])
(const_int 1 [0x1]))) sh_tmp.cpp:10 -1
(nil))
(insn 13 12 14 2 (set (subreg:SI (reg:DI 164 [ D.1936 ]) 4)
(reg:SI 172 [ D.1946 ])) sh_tmp.cpp:11 -1
(nil))
(insn 14 13 18 2 (set (reg:DI 165 [ <retval> ])
(reg:DI 164 [ D.1936 ])) sh_tmp.cpp:11 -1
(nil))
(insn 18 14 21 2 (set (reg/i:DI 0 r0)
(reg:DI 165 [ <retval> ])) sh_tmp.cpp:12 -1
(nil))
(insn 21 18 0 2 (use (reg/i:DI 0 r0)) sh_tmp.cpp:12 -1
(nil))
The fwprop1 pass however, changes this to...
;; total ref usage 48{26d,22u,0e} in 9{9 regular + 0 call} insns.
(note 6 0 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 2 6 3 2 (set (reg/v:SI 166 [ a ])
(reg:SI 4 r4 [ a ])) sh_tmp.cpp:7 257 {movsi_ie}
(expr_list:REG_DEAD (reg:SI 4 r4 [ a ])
(nil)))
(insn 3 2 4 2 (set (reg/v:SI 167 [ b ])
(reg:SI 5 r5 [ b ])) sh_tmp.cpp:7 257 {movsi_ie}
(expr_list:REG_DEAD (reg:SI 5 r5 [ b ])
(nil)))
(insn 4 3 5 2 (set (reg/v:SI 168 [ d ])
(reg:SI 6 r6 [ d ])) sh_tmp.cpp:7 257 {movsi_ie}
(expr_list:REG_DEAD (reg:SI 6 r6 [ d ])
(nil)))
(note 5 4 8 2 NOTE_INSN_FUNCTION_BEG)
(insn 8 5 11 2 (set (reg:SI 147 t)
(eq:SI (reg/v:SI 166 [ a ])
(reg/v:SI 167 [ b ]))) sh_tmp.cpp:9 17 {cmpeqsi_t}
(expr_list:REG_DEAD (reg/v:SI 166 [ a ])
(nil)))
(insn 11 8 12 2 (set (reg:SI 171 [ D.1946 ])
(plus:SI (reg/v:SI 168 [ d ])
(reg/v:SI 167 [ b ]))) sh_tmp.cpp:10 75 {*addsi3_compact}
(expr_list:REG_DEAD (reg/v:SI 168 [ d ])
(expr_list:REG_DEAD (reg/v:SI 167 [ b ])
(nil))))
(insn 12 11 26 2 (set (reg:SI 172 [ D.1946 ])
(plus:SI (reg:SI 171 [ D.1946 ])
(const_int 1 [0x1]))) sh_tmp.cpp:10 75 {*addsi3_compact}
(expr_list:REG_DEAD (reg:SI 171 [ D.1946 ])
(nil)))
(insn 26 12 27 2 (set (reg:SI 0 r0)
(reg:SI 147 t)) sh_tmp.cpp:12 399 {movt}
(expr_list:REG_DEAD (reg:SI 147 t)
(nil)))
(insn 27 26 21 2 (set (reg:SI 1 r1 [+4 ])
(reg:SI 172 [ D.1946 ])) sh_tmp.cpp:12 257 {movsi_ie}
(expr_list:REG_DEAD (reg:SI 172 [ D.1946 ])
(nil)))
(insn 21 27 0 2 (use (reg/i:DI 0 r0)) sh_tmp.cpp:12 -1
(nil))
... which makes the T_REG live across the addsi insns and thus combine will
fail to replace them with an addc insn which clobbers the T_REG.
Insns that use the T_REG (the movt insn in this case) should be reordered in
such a way that they immediately follow the insn that sets the T_REG (the
cmpeqsi insn in this case) before combine. This would increase the probability
of combine successes.
However, there could be some unlucky cases. If the movt destination is 'r0' as
above, and it is moved above other insns which might clobber/need the r0 reg
(e.g. tst #imm, logical #imm ops etc), there could be reload problems or the
code might get worse for such sequences.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Bug target/59291] [SH] Group T bit related insns before combine
2013-11-25 17:42 [Bug target/59291] New: [SH] Group T bit related isnsn before combine olegendo at gcc dot gnu.org
@ 2024-05-27 19:49 ` pietro.gcc at sociotechnical dot xyz
2024-05-27 20:50 ` olegendo at gcc dot gnu.org
1 sibling, 0 replies; 3+ messages in thread
From: pietro.gcc at sociotechnical dot xyz @ 2024-05-27 19:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59291
pietro <pietro.gcc at sociotechnical dot xyz> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |pietro.gcc at sociotechnical dot x
| |yz
--- Comment #1 from pietro <pietro.gcc at sociotechnical dot xyz> ---
Is this still valid? GCC 14 on the Compiler Explorer[0] show GCC 9.5 producing
the same assembly, but 12 and above (it doesn't have SH GCC 10 and 11)
produces:
_test2:
cmp/eq r5,r4
mov r5,r1
add r6,r1
add #1,r1
rts
movt r0
[0]: https://godbolt.org/z/668ax5ehj
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Bug target/59291] [SH] Group T bit related insns before combine
2013-11-25 17:42 [Bug target/59291] New: [SH] Group T bit related isnsn before combine olegendo at gcc dot gnu.org
2024-05-27 19:49 ` [Bug target/59291] [SH] Group T bit related insns " pietro.gcc at sociotechnical dot xyz
@ 2024-05-27 20:50 ` olegendo at gcc dot gnu.org
1 sibling, 0 replies; 3+ messages in thread
From: olegendo at gcc dot gnu.org @ 2024-05-27 20:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59291
Oleg Endo <olegendo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
Last reconfirmed| |2024-05-27
--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to pietro from comment #1)
> Is this still valid? GCC 14 on the Compiler Explorer[0] show GCC 9.5
> producing the same assembly, but 12 and above (it doesn't have SH GCC 10 and
> 11) produces:
>
> _test2:
> cmp/eq r5,r4
> mov r5,r1
> add r6,r1
> add #1,r1
> rts
> movt r0
>
> [0]: https://godbolt.org/z/668ax5ehj
Yes, still valid. Nothing has been done to explicitly address or improve the
issue. The ideal code should be something like:
cmp/eq r5,r4
movt r0
sett
addc r5,r6
rts
mov r6,r1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-27 20:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-25 17:42 [Bug target/59291] New: [SH] Group T bit related isnsn before combine olegendo at gcc dot gnu.org
2024-05-27 19:49 ` [Bug target/59291] [SH] Group T bit related insns " pietro.gcc at sociotechnical dot xyz
2024-05-27 20:50 ` olegendo 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).