Summary: | [4.7 regression] Revision 174989 (ipa-inline-transform.c) regressions | ||
---|---|---|---|
Product: | gcc | Reporter: | Markus Trippelsdorf <octoploid> |
Component: | other | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | d.g.gorbachev, hubicka, jason |
Priority: | P3 | ||
Version: | 4.7.0 | ||
Target Milestone: | 4.7.0 | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | ||
Bug Depends on: | 49538 | ||
Bug Blocks: | |||
Attachments: |
ipa-pure-const.c fix
Proposed fix |
Description
Markus Trippelsdorf
2011-06-26 19:47:12 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 (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. > 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
The patch attached to Bug 49538 fixes this problem. 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 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. 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. 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; } has this been fixed? (In reply to comment #9) > has this been fixed? No. 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
*** Bug 50381 has been marked as a duplicate of this bug. *** The fix is fine. Thanks. Please don't forget to also close Bug 49665 when you close this bug. 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 Fixed. *** Bug 50226 has been marked as a duplicate of this bug. *** |