Bug 49533 - [4.7 regression] Revision 174989 (ipa-inline-transform.c) regressions
Summary: [4.7 regression] Revision 174989 (ipa-inline-transform.c) regressions
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 50226 50381 (view as bug list)
Depends on: 49538
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-26 19:47 UTC by Markus Trippelsdorf
Modified: 2011-09-14 18:28 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
ipa-pure-const.c fix (239 bytes, patch)
2011-06-26 19:47 UTC, Markus Trippelsdorf
Details | Diff
Proposed fix (526 bytes, patch)
2011-09-13 10:09 UTC, Jan Hubicka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Trippelsdorf 2011-06-26 19:47:12 UTC
Created attachment 24602 [details]
ipa-pure-const.c fix

The latest Firefox from http://hg.mozilla.org/mozilla-central/ fails
to build during a profiled-build.

/usr/bin/python2.7 /var/tmp/mozilla-central/js/src/config/pythonpath.py -I../config /var/tmp/mozilla-central/js/src/config/expandlibs_exec.py --uselist --  c++ -o js  -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -pedantic -Wno-long-long -march=native -ffunction-sections -fdata-sections -fno-strict-aliasing -pthread -pipe  -DNDEBUG -DTRIMMED -fprofile-generate -O3  js.o jsworkers.o   -lpthread -Wl,-O1,--hash-style=gnu,--as-needed,--no-keep-memory,--gc-sections,--icf=all,--icf-iterations=3  -fprofile-generate -Wl,-rpath-link,/bin -Wl,-rpath-link,/var/tmp/mozilla-central/moz-build-dir/dist/lib  -L../../../dist/bin -L../../../dist/lib -L/var/tmp/mozilla-central/moz-build-dir/dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl ../editline/libeditline.a ../libjs_static.a -ldl
/var/tmp/mozilla-central/moz-build-dir/js/src/shell/jsworkers.o:jsworkers.cpp:function js::workers::Worker::processOneEvent(): warning: relocation refers to discarded section
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'JSAutoRequest::~JSAutoRequest()' is not defined locally
/var/tmp/mozilla-central/moz-build-dir/js/src/shell/jsworkers.o:jsworkers.cpp:function js::workers::ThreadPool::start(JSContext*): warning: relocation refers to discarded section
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'JSAutoSuspendRequest::JSAutoSuspendRequest(JSContext*)' is not defined locally
../libjs_static.a(jsapi.o):jsapi.cpp:function StartRequest(JSContext*): warning: relocation refers to discarded section
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'js::AutoLockGC::~AutoLockGC()' is not defined locally
../libjs_static.a(jsapi.o):jsapi.cpp:function StopRequest(JSContext*): warning: relocation refers to discarded section
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'js::AutoLockGC::~AutoLockGC()' is not defined locally
../libjs_static.a(jsapi.o):jsapi.cpp:function JS_ConvertArgumentsVA: warning: relocation refers to discarded section
...

This is caused by commit 8c1fce46fc and the attached patch fixes this.


Even with this patch applied, libxul fails to link later:

usr/bin/python2.7 /var/tmp/mozilla-central/config/pythonpath.py -I../../config /var/tmp/mozilla-central/config/expandlibs_exec.py --uselist --  c++  -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -pedantic -Wno-long-long -march=native -ffunction-sections -fdata-sections -fno-strict-aliasing -std=gnu++0x -pthread -pipe  -DNDEBUG -DTRIMMED -fprofile-generate -O3 -fPIC -shared -Wl,-z,defs -Wl,-h,libxul.so -o libxul.so  nsStaticXULComponents.o nsUnicharUtils.o nsBidiUtils.o nsRDFResource.o     -lpthread -Wl,-O1,--hash-style=gnu,--as-needed,--no-keep-memory,--gc-sections,--icf=all,--icf-iterations=3  -fprofile-generate -Wl,-rpath-link,/var/tmp/mozilla-central/moz-build-dir/dist/bin -Wl,-rpath-link,/usr/lib  ../../toolkit/xre/libxulapp_s.a  ../../staticlib/components/libnecko.a ../../staticlib/components/libuconv.a ../../staticlib/components/libi18n.a ../../staticlib/components/libchardet.a ../../staticlib/components/libjar50.a ../../staticlib/components/libstartupcache.a ../../staticlib/components/libpref.a ../../staticlib/components/libhtmlpars.a ../../staticlib/components/libimglib2.a ../../staticlib/components/libgkgfx.a ../../staticlib/components/libgklayout.a ../../staticlib/components/libdocshell.a ../../staticlib/components/libembedcomponents.a ../../staticlib/components/libwebbrwsr.a ../../staticlib/components/libnsappshell.a ../../staticlib/components/libtxmgr.a ../../staticlib/components/libcommandlines.a ../../staticlib/components/libtoolkitcomps.a ../../staticlib/components/libpipboot.a ../../staticlib/components/libpipnss.a ../../staticlib/components/libappcomps.a ../../staticlib/components/libcomposer.a ../../staticlib/components/libjetpack_s.a ../../staticlib/components/libtelemetry.a ../../staticlib/components/libjsctypes.a ../../staticlib/components/libjsperf.a ../../staticlib/components/libgkplugin.a ../../staticlib/components/libunixproxy.a ../../staticlib/components/libjsd.a ../../staticlib/components/libautoconfig.a ../../staticlib/components/libauth.a ../../staticlib/components/libcookie.a ../../staticlib/components/libpermissions.a ../../staticlib/components/libuniversalchardet.a ../../staticlib/components/librdf.a ../../staticlib/components/libwindowds.a ../../staticlib/components/libfileview.a ../../staticlib/components/libstoragecomps.a ../../staticlib/components/libplaces.a ../../staticlib/components/libtkautocomplete.a ../../staticlib/components/libsatchel.a ../../staticlib/components/libpippki.a ../../staticlib/components/libwidget_gtk2.a ../../staticlib/components/libsystem-pref.a ../../staticlib/components/libimgicon.a ../../staticlib/components/libremoteservice.a ../../staticlib/components/libspellchecker.a ../../staticlib/components/libzipwriter.a ../../staticlib/components/libservices-crypto.a ../../staticlib/libjsipc_s.a ../../staticlib/libdomipc_s.a ../../staticlib/libdomplugins_s.a ../../staticlib/libmozipc_s.a ../../staticlib/libmozipdlgen_s.a ../../staticlib/libipcshell_s.a ../../staticlib/libgfx2d.a ../../staticlib/libgfxipc_s.a ../../staticlib/libxpcom_core.a ../../staticlib/libucvutil_s.a ../../staticlib/libchromium_s.a ../../staticlib/libmozreg_s.a ../../staticlib/libmorkreader_s.a ../../staticlib/libgtkxtbin.a ../../staticlib/libthebes.a ../../staticlib/libycbcr.a ../../staticlib/libangle.a  ../../dist/lib/libmozsqlite3.a -L../../dist/bin -L../../dist/lib -ljpeg  ../../modules/libimg/png/libmozpng.a ../../gfx/qcms/libmozqcms.a /var/tmp/mozilla-central/moz-build-dir/dist/lib/libjs_static.a -L../../dist/bin -L../../dist/lib -lcrmf -lsmime3 -lssl3 -lnss3 -lnssutil3 -lcairo -lpixman-1 -lfreetype -lfontconfig    -L/usr/lib64 -lXrender -lcairo -lX11   ../../gfx/harfbuzz/src/libmozharfbuzz.a ../../gfx/ots/src/libmozots.a   -lz  -L/usr/lib -levent -lasound   -lrt -L../../dist/bin -L../../dist/lib  -L/var/tmp/mozilla-central/moz-build-dir/dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl ../../dist/lib/libmozalloc.a -ldbus-1 -lpthread -lrt   -L/usr/lib64 -lX11  -lXext  -pthread -lpangoft2-1.0 -lfreetype -lfontconfig -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -lgthread-2.0 -lrt -lglib-2.0   -pthread -lgtk-x11-2.0 -latk-1.0 -lgio-2.0 -lpangoft2-1.0 -lfreetype -lfontconfig -lgdk-x11-2.0 -lpangocairo-1.0 -lgdk_pixbuf-2.0 -lm -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -lpng14 -lgthread-2.0 -lrt -lglib-2.0   -lXt -lgthread-2.0 -lfreetype -lz -lstartup-notification-1   -ldl  -lrt
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: /var/tmp/mozilla-central/moz-build-dir/toolkit/library/../../layout/ipc/RenderFrameParent.o: requires dynamic reloc which may overflow at runtime; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: /var/tmp/mozilla-central/moz-build-dir/toolkit/library/../../content/events/src/nsEventStateManager.o: requires dynamic reloc which may overflow at runtime; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: /var/tmp/mozilla-central/moz-build-dir/toolkit/library/../../content/xul/templates/src/nsRuleNetwork.o: requires dynamic reloc which may overflow at runtime; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: /var/tmp/mozilla-central/moz-build-dir/toolkit/library/../../gfx/thebes/GLContextProviderGLX.o: requires dynamic reloc which may overflow at runtime; recompile with -fPIC
/var/tmp/mozilla-central/moz-build-dir/toolkit/library/../../intl/uconv/ucvlatin/nsUnicodeToUCS2BE.o:nsUnicodeToUCS2BE.cpp:function vtable for nsUnicodeToUTF16BE: warning: relocation refers to discarded section
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'nsUnicodeToUTF16BE::~nsUnicodeToUTF16BE()' is not defined locally
/var/tmp/mozilla-central/moz-build-dir/toolkit/library/../../intl/uconv/ucvlatin/nsUnicodeToUCS2BE.o:nsUnicodeToUCS2BE.cpp:function vtable for nsUnicodeToUTF16LE: warning: relocation refers to discarded section
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'nsUnicodeToUTF16LE::~nsUnicodeToUTF16LE()' is not defined locally
...
I haven't figured out the cause of this failure yet. 
But please note that a normal (non pgo) build links just fine.
Comment 1 Jan Hubicka 2011-06-27 10:13:34 UTC
The patch you quote has the effect of disabling pure const discovery for functions calling functions with an aliases defined.  It is quite surprising that this does make a difference, but I still think this is most likely effect of PR47247.

The COMDAT problem causes inline functions with address taken to not be optimized when their address is taken and indirect call profiling takes address of all virtuals.

Carry made an gold update that will hopefully hit the CVS binutils soonish and I will update then the GCC side.  Until that happens, I would advice using -fprofile-generate -fno-lto (i.e. LTO is not needed for the instrumented compiler).

Honza
Comment 2 Markus Trippelsdorf 2011-06-27 10:25:13 UTC
(In reply to comment #1)
> The patch you quote has the effect of disabling pure const discovery for
> functions calling functions with an aliases defined.  It is quite surprising
> that this does make a difference, but I still think this is most likely effect
> of PR47247.
> 
> The COMDAT problem causes inline functions with address taken to not be
> optimized when their address is taken and indirect call profiling takes address
> of all virtuals.
> 
> Carry made an gold update that will hopefully hit the CVS binutils soonish and
> I will update then the GCC side.  Until that happens, I would advice using
> -fprofile-generate -fno-lto (i.e. LTO is not needed for the instrumented
> compiler).

Note that this happens with "-fprofile-generate" only (no lto involved).
 
Sprinkling "__attribute__((used))" all over the Javascript code, or using
ld.bfd solves the first issue, too.
Comment 3 Jan Hubicka 2011-06-27 10:53:14 UTC
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49533
> 
> --- Comment #2 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2011-06-27 10:25:13 UTC ---
> (In reply to comment #1)
> > The patch you quote has the effect of disabling pure const discovery for
> > functions calling functions with an aliases defined.  It is quite surprising
> > that this does make a difference, but I still think this is most likely effect
> > of PR47247.
> > 
> > The COMDAT problem causes inline functions with address taken to not be
> > optimized when their address is taken and indirect call profiling takes address
> > of all virtuals.
> > 
> > Carry made an gold update that will hopefully hit the CVS binutils soonish and
> > I will update then the GCC side.  Until that happens, I would advice using
> > -fprofile-generate -fno-lto (i.e. LTO is not needed for the instrumented
> > compiler).
> 
> Note that this happens with "-fprofile-generate" only (no lto involved).

Hmm, sorry then it is different issue.  I take a look.

Honza
Comment 4 Markus Trippelsdorf 2011-06-28 14:51:15 UTC
The patch attached to Bug 49538 fixes this problem.
Comment 5 Markus Trippelsdorf 2011-07-07 16:20:44 UTC
This is caused by:

7791b0eb56c3c324004e6fffe2d5f21241c038f7 is the first bad commit
commit 7791b0eb56c3c324004e6fffe2d5f21241c038f7
Author: hubicka <hubicka@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Jun 13 13:12:23 2011 +0000

        * ipa-inline-transform.c (+can_remove_node_now_p_1): Break out from...
        (can_remove_node_now_p): ... here; handle same comdat groups.
        (clone_inlined_nodes): Update use of can_remove_node_now_p add TODO.
        (inline_call): Update use of can_remove_node_now_p.
    
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@174989 138bc75d-0d04-0410-961f-82ee72b054a4
Comment 6 Markus Trippelsdorf 2011-07-07 20:29:44 UTC
With 7791b0eb56c3c324004e6fffe2d5f21241c038f7 reverted and after reverting the following hunk: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49538#c6 , Firefox builds and runs fine again.
Comment 7 Markus Trippelsdorf 2011-07-11 19:45:06 UTC
Another instance of the same regression (caused by commit 7791b0eb56c3c):

stellarium build (without PGO) with latest gcc crashes on exit:

[Thread 0x7fffee9cb700 (LWP 16857) exited]
[Thread 0x7ffff4d5a700 (LWP 16821) exited]

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x00007ffff6c331b1 in __run_exit_handlers (status=0, listp=0x7ffff6d594c8, run_list_atexit=true) at exit.c:78
#2  0x00007ffff6c33235 in exit (status=Unhandled dwarf expression opcode 0xf3
) at exit.c:100
#3  0x00007ffff6c1cf89 in __libc_start_main (main=0x60b500 <main>, argc=1, ubp_av=0x7fffffffe158, init=Unhandled dwarf expression opcode 0xf3
) at libc-start.c:258
#4  0x00000000004a2589 in _start ()

Reverting 7791b0eb56c3c "solves" the problem.

And I guess Bug 49665 is a dup of the same issue.
Comment 8 Markus Trippelsdorf 2011-07-13 21:13:31 UTC
Hmm, that whole approach doesn't seem to work.

The following patch survives building of stellarium and explains the 
failures that I've reported above:

diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index c329bea..bef1d38 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -122,7 +122,10 @@ can_remove_node_now_p (struct cgraph_node *node, struct cgraph_edge *e)
        next != node; next = next->same_comdat_group)
     if (node->callers && node->callers != e
        && !can_remove_node_now_p_1 (node))
-      return false;
+      {
+        gcc_unreachable();
+        return false;
+      }
   return true;
 }

Even when one changes node to next in the if clause,
"next->callers == e" is always true and the following 
patch also survives building of stellarium. But it merely
brings us back to the status quo before commit 7791b0eb56c3c
went in... 

 diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index c329bea..2a09de8 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -120,9 +120,13 @@ can_remove_node_now_p (struct cgraph_node *node, struct cgraph_edge *e)
     return true;
   for (next = node->same_comdat_group;
        next != node; next = next->same_comdat_group)
-    if (node->callers && node->callers != e
-       && !can_remove_node_now_p_1 (node))
-      return false;
+    if (next->callers
+       && !can_remove_node_now_p_1 (next))
+      {
+        gcc_assert (next->callers == e);
+        return false;
+      }
+  gcc_unreachable();
   return true;
 }
Comment 9 Andrew Pinski 2011-08-31 21:05:14 UTC
has this been fixed?
Comment 10 Markus Trippelsdorf 2011-09-01 06:44:39 UTC
(In reply to comment #9)
> has this been fixed?

No.
Comment 11 Jan Hubicka 2011-09-13 10:09:17 UTC
Created attachment 25261 [details]
Proposed fix

Hi,
there seems to be 3 bugs cooperating on this problem.
1) we sometimes forget to output alias in comdat group since assemble_alias gets into wrong code path when called from cgraphunit
2) I made pasto while walking aliases testing always the master node instead of alias
3) test checking aliases looks only for unremovable function that are called, instead for functions that are either unremovable or called.

Honza
Comment 12 Jan Hubicka 2011-09-13 10:14:30 UTC
*** Bug 50381 has been marked as a duplicate of this bug. ***
Comment 13 Markus Trippelsdorf 2011-09-13 10:57:28 UTC
The fix is fine. Thanks.

Please don't forget to also close Bug 49665 when you close this bug.
Comment 14 Jan Hubicka 2011-09-13 14:28:47 UTC
Author: hubicka
Date: Tue Sep 13 14:28:39 2011
New Revision: 178810

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178810
Log:

	PR other/49533
	* cgraphunit.c (assemble_thunks_and_aliases): Force alias to be output.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cgraphunit.c
Comment 15 Jan Hubicka 2011-09-13 14:56:17 UTC
Fixed.
Comment 16 Dmitry Gorbachev 2011-09-14 18:28:20 UTC
*** Bug 50226 has been marked as a duplicate of this bug. ***