stop IPA wrapping 'main'

stop IPA wrapping 'main'

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]

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

[-- Attachment #2: trunk-ipa.patch --]
[-- Type: text/x-patch, Size: 1857 bytes --]

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" } } */

Re: stop IPA wrapping 'main'

[Jan Hubicka]

> 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