On Linux x86-64 with at least with GCC 8.2.1 (gcc-8.2.1-6.fc29.x86_64) and recent trunk towards GCC 9, compiling > $ cat test.cc > void f1(); > struct S1 { static void f2(); }; > struct __attribute__ ((visibility("default"))) S2: S1 { static void f2(); }; > struct S3: S1 { static void f2(); }; > struct S4: S3 { static void f2(); }; > void S2::f2() { S1::f2(); } > void S3::f2() { S1::f2(); } > void S4::f2() { > f1(); > S3::f2(); // MISSING > } > > $ g++ -fvisibility=hidden -fpic -O2 -fno-inline -S test.cc causes the call to S3::f2 to be missing from the code generated for S4::f2: > .globl _ZN2S42f2Ev > .hidden _ZN2S42f2Ev > .type _ZN2S42f2Ev, @function > _ZN2S42f2Ev: > .LFB2: > .cfi_startproc > jmp _Z2f1v@PLT > .cfi_endproc > .LFE2: > .size _ZN2S42f2Ev, .-_ZN2S42f2Ev
Oddly with just -O2 -fno-inline we call S2::f2 () (it got ICFed with S3::f2 it seems). Disabling ICF also fixes the omission of the call for -fpic -fvisibility=hidden.
Let me take a look..
I slightly renamed the functions to have it more readable: $ cat pr89009.c void foo(); struct S1 { static void bar(); }; struct __attribute__ ((visibility("default"))) S2: S1 { static void bar(); }; struct S3: S1 { static void bar(); }; struct S4: S3 { static void bar(); }; void S2::bar() { S1::bar(); } void S3::bar() { S1::bar(); } void S4::bar() { foo(); S3::bar(); // MISSING } Now ICF does following folding: Equals called for: static void S3::bar()/1:static void S2::bar()/0 with result: true and new localalias is added: _ZN2S23barEv.localalias/5 (static void S2::_ZN2S23barEv.localalias()) @0x7ffff6b559d8 Type: function definition analyzed alias Visibility: visibility_specified References: _ZN2S23barEv/0 (alias) Referring: Availability: available Function flags: icf_merged Called by: static void S3::bar()/1 (1073741824 (estimated locally),1.00 per call) (can throw external) Calls: and I see the call in S4::f2 is removed here: pr89009.c.103t.dce2: ;; Function S4::bar (_ZN2S43barEv, funcdef_no=2, decl_uid=2311, cgraph_uid=3, symbol_order=2) Marking useful stmt: foo (); Marking useful stmt: return; Processing worklist: processing: return; processing: foo (); Eliminating unnecessary statements: Deleting : S3::bar (); Removed 1 of 3 statements (33%) Removed 0 of 0 PHI nodes (0%) S4::bar () { <bb 2> [local count: 1073741824]: foo (); return; } Richi do you have any idea what?
> Richi do you have any idea what? So it's caused by fact that IPA pure-const promotes S3::bar to be const: Visiting static void S3::bar()/1 state:const looping 0 Call to static void S2::_ZN2S23barEv.localalias()/5 state:const looping:0 Result const looping 0 Function found not to call free: static void S3::bar() Function found to be const: static void S3::bar() Declaration updated to be const: static void S3::bar() Then DCE is happy to remote the function as it's supposed to not have a side effect.
Well, I believe the test-case is invalid as one can't have hidden visibility and then defined S1::f2 in a different translation unit. I would like to see an example which would be possible to link. This works for me: $ cat pr89009.h void foo(); struct S1 { static void bar(); }; struct __attribute__ ((visibility("default"))) S2: S1 { static void bar(); }; struct S3: S1 { static void bar(); }; struct S4: S3 { static void bar(); }; $ cat pr89009.c #include "pr89009.h" void S2::bar() { S1::bar(); } void S3::bar() { S1::bar(); } void S4::bar() { foo(); S3::bar(); // MISSING } int main() { S4::bar (); } $ cat pr89009-2.c #include "pr89009.h" void foo() { __builtin_printf ("foo called\n"); } void S1::bar() { __builtin_printf ("S1::bar\n"); }
(In reply to Martin Liška from comment #5) > Well, I believe the test-case is invalid as one can't have hidden visibility > and then defined S1::f2 in a different translation unit. Why? Isn't hidden visibility just about symbol visibility at the executable/dynamic library level?
(In reply to Stephan Bergmann from comment #6) > (In reply to Martin Liška from comment #5) > > Well, I believe the test-case is invalid as one can't have hidden visibility > > and then defined S1::f2 in a different translation unit. > > Why? Isn't hidden visibility just about symbol visibility at the > executable/dynamic library level? I might be wrong. But I would like to see a linkable test-case.
...and adding to the test.cc from comment 0 an additional > $ cat main.cc > void f1(); > struct S1 { static void f2(); }; > struct __attribute__ ((visibility("default"))) S2: S1 { static void f2(); }; > struct S3: S1 { static void f2(); }; > struct S4: S3 { static void f2(); }; > void f1() { __builtin_printf("f1\n"); } > void S1::f2() { __builtin_printf("S1::f2\n"); } > int main() { S4::f2(); } > > $ g++ -fvisibility=hidden -fpic -O2 -fno-inline test.cc main.cc > $ ./a.out > f1 to form a complete program still fails for me (GCC 8.2.1).
Thanks for it, confirmed! Started with r244273, but the revision probably only exposed a latent issue.
Ok, so it's as old as ICF. There's a minimal test-case: $ cat test.cc #pragma GCC visibility push(default) void foo1() { __builtin_printf ("foo\n"); } #pragma GCC visibility pop void foo2() { __builtin_printf ("foo\n"); } int main() { foo2(); } (for older revisions than r244273 one needs: $ cat test.cc void foo2() { __builtin_printf ("foo\n"); } #pragma GCC visibility push(default) void foo1() { __builtin_printf ("foo\n"); } #pragma GCC visibility pop int main() { foo2(); }
So, shall we punt to ICF functions/methods with different visibility, or do we need to treat it specially during later IPA optimizations?
(In reply to Jakub Jelinek from comment #11) > So, shall we punt to ICF functions/methods with different visibility, or do > we need to treat it specially during later IPA optimizations? I'll discuss that with Honza this week and propose a patch for it.
I've got a patch candidate for it, will send it on Monday.
Author: marxin Date: Mon Feb 11 08:13:03 2019 New Revision: 268762 URL: https://gcc.gnu.org/viewcvs?rev=268762&root=gcc&view=rev Log: Construct ipa_reduced_postorder always for overwritable (PR ipa/89009). 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * ipa-cp.c (build_toporder_info): Remove usage of a param. * ipa-inline.c (inline_small_functions): Likewise. * ipa-pure-const.c (propagate_pure_const): Likewise. (propagate_nothrow): Likewise. * ipa-reference.c (propagate): Likewise. * ipa-utils.c (struct searchc_env): Remove unused field. (searchc): Always search across AVAIL_INTERPOSABLE. (ipa_reduced_postorder): Always allow AVAIL_INTERPOSABLE as the only called IPA pure const can properly not propagate across interposable boundary. * ipa-utils.h (ipa_reduced_postorder): Remove param. 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * g++.dg/ipa/pr89009.C: New test. Added: trunk/gcc/testsuite/g++.dg/ipa/pr89009.C Modified: trunk/gcc/ChangeLog trunk/gcc/ipa-cp.c trunk/gcc/ipa-inline.c trunk/gcc/ipa-pure-const.c trunk/gcc/ipa-reference.c trunk/gcc/ipa-utils.c trunk/gcc/ipa-utils.h trunk/gcc/testsuite/ChangeLog
Fixed on trunk so far.
Author: marxin Date: Thu Feb 14 11:24:45 2019 New Revision: 268871 URL: https://gcc.gnu.org/viewcvs?rev=268871&root=gcc&view=rev Log: Backport r268762 2019-02-14 Martin Liska <mliska@suse.cz> Backport from mainline 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * ipa-cp.c (build_toporder_info): Remove usage of a param. * ipa-inline.c (inline_small_functions): Likewise. * ipa-pure-const.c (propagate_pure_const): Likewise. (propagate_nothrow): Likewise. * ipa-reference.c (propagate): Likewise. * ipa-utils.c (struct searchc_env): Remove unused field. (searchc): Always search across AVAIL_INTERPOSABLE. (ipa_reduced_postorder): Always allow AVAIL_INTERPOSABLE as the only called IPA pure const can properly not propagate across interposable boundary. * ipa-utils.h (ipa_reduced_postorder): Remove param. 2019-02-14 Martin Liska <mliska@suse.cz> Backport from mainline 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * g++.dg/ipa/pr89009.C: New test. Added: branches/gcc-8-branch/gcc/testsuite/g++.dg/ipa/pr89009.C Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/ipa-cp.c branches/gcc-8-branch/gcc/ipa-inline.c branches/gcc-8-branch/gcc/ipa-pure-const.c branches/gcc-8-branch/gcc/ipa-reference.c branches/gcc-8-branch/gcc/ipa-utils.c branches/gcc-8-branch/gcc/ipa-utils.h branches/gcc-8-branch/gcc/testsuite/ChangeLog
Author: marxin Date: Fri Feb 15 10:58:49 2019 New Revision: 268937 URL: https://gcc.gnu.org/viewcvs?rev=268937&root=gcc&view=rev Log: Backport r268762 2019-02-15 Martin Liska <mliska@suse.cz> Backport from mainline 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * ipa-cp.c (build_toporder_info): Remove usage of a param. * ipa-inline.c (inline_small_functions): Likewise. * ipa-pure-const.c (propagate_pure_const): Likewise. (propagate_nothrow): Likewise. * ipa-reference.c (propagate): Likewise. * ipa-utils.c (struct searchc_env): Remove unused field. (searchc): Always search across AVAIL_INTERPOSABLE. (ipa_reduced_postorder): Always allow AVAIL_INTERPOSABLE as the only called IPA pure const can properly not propagate across interposable boundary. * ipa-utils.h (ipa_reduced_postorder): Remove param. 2019-02-15 Martin Liska <mliska@suse.cz> Backport from mainline 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * g++.dg/ipa/pr89009.C: New test. Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/ipa-cp.c branches/gcc-7-branch/gcc/ipa-inline.c branches/gcc-7-branch/gcc/ipa-pure-const.c branches/gcc-7-branch/gcc/ipa-reference.c branches/gcc-7-branch/gcc/ipa-utils.c branches/gcc-7-branch/gcc/ipa-utils.h branches/gcc-7-branch/gcc/testsuite/ChangeLog
Fixed on all active branches.
Fixed.
Author: marxin Date: Tue May 28 11:54:33 2019 New Revision: 271697 URL: https://gcc.gnu.org/viewcvs?rev=271697&root=gcc&view=rev Log: Backport r268762 (test-suite) 2019-05-28 Martin Liska <mliska@suse.cz> Backport from mainline 2019-02-11 Martin Liska <mliska@suse.cz> PR ipa/89009 * g++.dg/ipa/pr89009.C: New test. Added: branches/gcc-7-branch/gcc/testsuite/g++.dg/ipa/pr89009.C Modified: branches/gcc-7-branch/gcc/testsuite/ChangeLog