public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/94146] New: Merging functions with same bodies stopped working
@ 2020-03-11 16:11 antoshkka at gmail dot com
2020-03-11 17:48 ` [Bug middle-end/94146] [10 Regression] " jakub at gcc dot gnu.org
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: antoshkka at gmail dot com @ 2020-03-11 16:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
Bug ID: 94146
Summary: Merging functions with same bodies stopped working
Product: gcc
Version: 10.0
Status: UNCONFIRMED
Keywords: missed-optimization
Severity: normal
Priority: P3
Component: middle-end
Assignee: unassigned at gcc dot gnu.org
Reporter: antoshkka at gmail dot com
Target Milestone: ---
Consider the example:
extern int x , y;
int ternary(int i) { return i > 0 ? x : y; }
int ternary2(int i) { return i > 0 ? x : y; }
GCC9 was merging the functions with -O2:
ternary2(int):
jmp ternary(int)
With GCC10 merging at -O2 is missing and function bodies are duplicated even
for very big functions: https://godbolt.org/z/2kH8VR
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
@ 2020-03-11 17:48 ` jakub at gcc dot gnu.org
2020-03-11 17:54 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-03-11 17:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2020-03-11
Target Milestone|--- |10.0
CC| |hubicka at gcc dot gnu.org,
| |jakub at gcc dot gnu.org,
| |marxin at gcc dot gnu.org
Status|UNCONFIRMED |NEW
Summary|Merging functions with same |[10 Regression] Merging
|bodies stopped working |functions with same bodies
| |stopped working
Ever confirmed|0 |1
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r10-3583-g562d1e9556777988ae46c5d1357af2636bc272ea
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
2020-03-11 17:48 ` [Bug middle-end/94146] [10 Regression] " jakub at gcc dot gnu.org
@ 2020-03-11 17:54 ` jakub at gcc dot gnu.org
2020-03-11 17:57 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-03-11 17:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
My guess is that ICF works fine but then inliner inlines it back into the
thunk, which it didn't before.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
2020-03-11 17:48 ` [Bug middle-end/94146] [10 Regression] " jakub at gcc dot gnu.org
2020-03-11 17:54 ` jakub at gcc dot gnu.org
@ 2020-03-11 17:57 ` jakub at gcc dot gnu.org
2020-03-12 8:57 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-03-11 17:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If not already marked clearly as an ICF created thunk, I'd say it should be and
then inliner should take it into account (and only inline if the function
became very small or not at all).
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
` (2 preceding siblings ...)
2020-03-11 17:57 ` jakub at gcc dot gnu.org
@ 2020-03-12 8:57 ` rguenth at gcc dot gnu.org
2020-03-12 9:07 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-03-12 8:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> If not already marked clearly as an ICF created thunk, I'd say it should be
> and then inliner should take it into account (and only inline if the
> function became very small or not at all).
It looks like ICF really creates a forwarding call:
ternary2 (int i)
{
int retval.4;
<bb 2> [local count: 1073741824]:
retval.4_3 = ternary (i_2(D)); [tail call]
return retval.4_3;
}
so IMHO for small functions the inlining is good (but why don't we create
an alias or an alternate entry symbol instead of a full (aligned) function?)
For big functions the inlining shouldn't happen indeed, possibly by detecting
this kind of forwarders?
So we're missing a testcase showing the regression IMHO. It still works
with -Os for the testcase.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
` (3 preceding siblings ...)
2020-03-12 8:57 ` rguenth at gcc dot gnu.org
@ 2020-03-12 9:07 ` jakub at gcc dot gnu.org
2020-03-12 9:18 ` marxin at gcc dot gnu.org
2020-03-12 10:38 ` pinskia at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-03-12 9:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> (In reply to Jakub Jelinek from comment #3)
> > If not already marked clearly as an ICF created thunk, I'd say it should be
> > and then inliner should take it into account (and only inline if the
> > function became very small or not at all).
>
> It looks like ICF really creates a forwarding call:
>
> ternary2 (int i)
> {
> int retval.4;
>
> <bb 2> [local count: 1073741824]:
> retval.4_3 = ternary (i_2(D)); [tail call]
> return retval.4_3;
>
> }
>
> so IMHO for small functions the inlining is good (but why don't we create
> an alias or an alternate entry symbol instead of a full (aligned) function?)
ICF has I think 3 options, one is adjust all callers if they can be adjusted,
another one is to use alias and another one to use a thunk. Whether to use an
alias or thunk depends on whether something might perform function address
comparisons.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
` (4 preceding siblings ...)
2020-03-12 9:07 ` jakub at gcc dot gnu.org
@ 2020-03-12 9:18 ` marxin at gcc dot gnu.org
2020-03-12 10:38 ` pinskia at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-03-12 9:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
Yes, ICF wrappers can be inlined back and that's what happened here since
r10-3583-g562d1e9556777988.
I don't see what's problematic here?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug middle-end/94146] [10 Regression] Merging functions with same bodies stopped working
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
` (5 preceding siblings ...)
2020-03-12 9:18 ` marxin at gcc dot gnu.org
@ 2020-03-12 10:38 ` pinskia at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: pinskia at gcc dot gnu.org @ 2020-03-12 10:38 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94146
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |INVALID
--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Both functions are extern. So both functions can have their address taken out
side of this TU so they cannot be alias of each other. Yes inlining inlined
back the original function but that is ok because the cost model is saying that
is cheaper than the call itself.
Note even though the cost model could be improved, tail call detection happens
late so the cost for call does not take into account tail call. Oh not every
target has a tail call pattern (major ones do).
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-12 10:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 16:11 [Bug middle-end/94146] New: Merging functions with same bodies stopped working antoshkka at gmail dot com
2020-03-11 17:48 ` [Bug middle-end/94146] [10 Regression] " jakub at gcc dot gnu.org
2020-03-11 17:54 ` jakub at gcc dot gnu.org
2020-03-11 17:57 ` jakub at gcc dot gnu.org
2020-03-12 8:57 ` rguenth at gcc dot gnu.org
2020-03-12 9:07 ` jakub at gcc dot gnu.org
2020-03-12 9:18 ` marxin at gcc dot gnu.org
2020-03-12 10:38 ` 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).