| Message ID | 5673016F.8060105@acm.org |
|---|---|
| State | New |
| Headers | show |
> gcc.dg/20031102-1.c now causes some 'surprising' optimization > behaviour. It is essentially > > int FooBar(void) > { > ... stuff > return 0; > } > > int main(void) > { > return FooBar(); > } > > > What happens is that FooBar gets inlined into main, and then > ipa-icf notices FooBar and main have identical bodies. It chooses > to have FooBar tail call main, which results in a surprising call > of 'main'. On PTX this is particularly unfortunate because we have > to emit a single prototype for main with the regular argc and argv > arguments (the backend gets around 'int main (void)' by faking the > additional 2 args). But that fails here because the tail call > doesn't match the prototype. > > Anyway, picking 'main' as the source function struck me as a poor > choice, hence the attached patch. It picks the second function of a > congruent set, if the first is 'main'. Note that even on, say > x86-linux, we emit a tail call rather than an alias for the included > testcase. > > I removed the gcc_assert, as the vector indexing operator already > checks the subscript is within range. > > Alternatively I could probably just fixup the testcase to make > FooBar uninlinable, as I suspect that might have been the original > intent. > > tested on x86_64-linux and ptx-none. > > nathan > 2015-12-17 Nathan Sidwell <nathan@acm.org> > > gcc/ > * ipa-icf.c (sem_item_optimizer::merge): Don't pick 'main' as the > source function. > > gcc/testsuite/ > * gcc.dg/ipa/ipa-icf-merge-1.c: New. OK, thanks. Indeed we should not introduce new calls to main :) It contains some magic stuff on x86 targets, too. Honza
2015-12-17 Nathan Sidwell <nathan@acm.org> gcc/ * ipa-icf.c (sem_item_optimizer::merge): Don't pick 'main' as the source function. gcc/testsuite/ * gcc.dg/ipa/ipa-icf-merge-1.c: New. Index: ipa-icf.c =================================================================== --- ipa-icf.c (revision 231770) +++ ipa-icf.c (working copy) @@ -3398,14 +3398,20 @@ sem_item_optimizer::merge_classes (unsig if (c->members.length () == 1) continue; - gcc_assert (c->members.length ()); - sem_item *source = c->members[0]; - for (unsigned int j = 1; j < c->members.length (); j++) + if (MAIN_NAME_P (DECL_NAME (source->decl))) + /* If merge via wrappers, picking main as the target can be + problematic. */ + source = c->members[1]; + + for (unsigned int j = 0; j < c->members.length (); j++) { sem_item *alias = c->members[j]; + if (alias == source) + continue; + if (dump_file) { fprintf (dump_file, "Semantic equality hit:%s->%s\n", Index: testsuite/gcc.dg/ipa/ipa-icf-merge-1.c =================================================================== --- testsuite/gcc.dg/ipa/ipa-icf-merge-1.c (revision 0) +++ testsuite/gcc.dg/ipa/ipa-icf-merge-1.c (working copy) @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-O2 -fdump-ipa-icf" } */ + +/* Picking 'main' as a candiate target for equivalent functios is not a + good idea. */ + +int baz (int); + +int foo () +{ + return baz (baz (0)); +} + + +int main () +{ + return baz (baz (0)); +} + +/* Notice the two functions are the same. */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:foo->main" "icf" } } */ + +/* Make sure we don't tail call main. */ +/* { dg-final { scan-ipa-dump-not "= main \\(\\);" "icf" } } */ + +/* Make sure we tail call foo. */ +/* { dg-final { scan-ipa-dump "= foo \\(\\);" "icf" } } */
gcc.dg/20031102-1.c now causes some 'surprising' optimization behaviour. It is essentially int FooBar(void) { ... stuff return 0; } int main(void) { return FooBar(); } What happens is that FooBar gets inlined into main, and then ipa-icf notices FooBar and main have identical bodies. It chooses to have FooBar tail call main, which results in a surprising call of 'main'. On PTX this is particularly unfortunate because we have to emit a single prototype for main with the regular argc and argv arguments (the backend gets around 'int main (void)' by faking the additional 2 args). But that fails here because the tail call doesn't match the prototype. Anyway, picking 'main' as the source function struck me as a poor choice, hence the attached patch. It picks the second function of a congruent set, if the first is 'main'. Note that even on, say x86-linux, we emit a tail call rather than an alias for the included testcase. I removed the gcc_assert, as the vector indexing operator already checks the subscript is within range. Alternatively I could probably just fixup the testcase to make FooBar uninlinable, as I suspect that might have been the original intent. tested on x86_64-linux and ptx-none. nathan