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
Started with r10-3583-g562d1e9556777988ae46c5d1357af2636bc272ea
My guess is that ICF works fine but then inliner inlines it back into the thunk, which it didn't before.
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).
(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.
(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.
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?
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).