Between 20110502 and 20110518, g++.dg/torture/pr41257-2.C started to FAIL on Tru64 UNIX: A::~A() typeinfo for A collect2: error: ld returned 1 exit status which is a link error (undefined symbols). This is a regression from the 4.6 branch. While the input file is identical, the generated code is far longer on mainline, and contains a call to an undefined function _ZN1AD2Ev (A::~A()). This may be related to the fact that Tru64 UNIX doesn't support weak definitions, but the code is plain wrong. I'm attaching assembler output from 4.6 and 4.7.
Created attachment 24621 [details] gcc 4.6 assembler output at -O0
Created attachment 24622 [details] gcc 4.7 assembler output at -O0
Confirmed, we shouldn't be emitting ~B because it's not needed. Probably introduced by r173517, 2011-05-06 Jan Hubicka <jh@suse.cz> * cgraph.c (cgraph_add_thunk): Create real function node instead of alias node; finalize it and mark needed/reachale; arrange visibility to be right and add it into the corresponding same comdat group list. ...
> Confirmed, we shouldn't be emitting ~B because it's not needed. Probably > introduced by r173517, > > 2011-05-06 Jan Hubicka <jh@suse.cz> > > * cgraph.c (cgraph_add_thunk): Create real function node instead > of alias node; finalize it and mark needed/reachale; arrange visibility > to be right and add it into the corresponding same comdat group list. > ... I believed that thunks always belong to same comdat group as the function they are associated with. Perhaps same comdat groups behave differently on HP? I will check. Honza
> --- Comment #4 from Jan Hubicka <hubicka at ucw dot cz> 2011-07-01 08:40:13 UTC --- [...] > I believed that thunks always belong to same comdat group as the function they > are associated with. > Perhaps same comdat groups behave differently on HP? This is Tru64 UNIX with ECOFF, no named sections, no COMDAT groups. Rainer
> This is Tru64 UNIX with ECOFF, no named sections, no COMDAT groups. Yep, I know, but the question is how absence of COMDAT groups changes behaviour of thunks. The problem here is that the thunks are !cgraph_can_remove_if_no_direct_calls_and_refs_p and thus we don't optimize them out. The reason is the test: 2902 if (node->local.externally_visible 2903 && (!DECL_COMDAT (node->decl) 2904 || cgraph_used_from_object_file_p (node))) 2905 return false; the decl is not comdat, just weak and thus we think we must keep it in program. The declaration of the destructor itself do have COMDAT flag. The following patch should fix the problem: Index: ipa.c =================================================================== --- ipa.c (revision 175748) +++ ipa.c (working copy) @@ -871,9 +871,9 @@ function_and_variable_visibility (bool w We also need to arrange the thunk into the same comdat group as the function it reffers to. */ + DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); if (DECL_ONE_ONLY (decl_node->decl)) { - DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); DECL_COMDAT_GROUP (node->decl) = DECL_COMDAT_GROUP (decl_node->decl); if (DECL_ONE_ONLY (decl_node->decl) && !node->same_comdat_group) { Jason, the whole code copying visibilities to thunk decls is just a hack. Do you think you can make C++ FE to put proper visibility flags on thunks and same body aliases? Honza
> The declaration of the destructor itself do have COMDAT flag. > The following patch should fix the problem: > Index: ipa.c > =================================================================== > --- ipa.c (revision 175748) > +++ ipa.c (working copy) > @@ -871,9 +871,9 @@ function_and_variable_visibility (bool w > > We also need to arrange the thunk into the same comdat group as > the function it reffers to. */ ^ typo > + DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); > if (DECL_ONE_ONLY (decl_node->decl)) > { > - DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); > DECL_COMDAT_GROUP (node->decl) = DECL_COMDAT_GROUP > (decl_node->decl); > if (DECL_ONE_ONLY (decl_node->decl) && !node->same_comdat_group) > { I can include it in this weekend's bootstrap. Rainer
(In reply to comment #6) > + DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); > if (DECL_ONE_ONLY (decl_node->decl)) > { > - DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); Looks good to me. > Jason, the whole code copying visibilities to thunk decls is just a hack. Do > you think you can make C++ FE to put proper visibility flags on thunks and > same body aliases? I can certainly copy some flags across. But it seems rather cumbersome to have to manage same_comdat_group in the front end. There's also the issue that in order to set DECL_COMDAT_GROUP (and thus DECL_ONE_ONLY) we need to build the mangled name for the main decl which otherwise might not be needed.
> The following patch should fix the problem: > Index: ipa.c > =================================================================== > --- ipa.c (revision 175748) > +++ ipa.c (working copy) > @@ -871,9 +871,9 @@ function_and_variable_visibility (bool w > > We also need to arrange the thunk into the same comdat group as > the function it reffers to. */ > + DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); > if (DECL_ONE_ONLY (decl_node->decl)) > { > - DECL_COMDAT (node->decl) = DECL_COMDAT (decl_node->decl); > DECL_COMDAT_GROUP (node->decl) = DECL_COMDAT_GROUP > (decl_node->decl); > if (DECL_ONE_ONLY (decl_node->decl) && !node->same_comdat_group) > { It does indeed and introduces no regressions. Thanks. Rainer
Author: jason Date: Wed Jul 6 21:22:16 2011 New Revision: 175938 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175938 Log: PR c++/49568 * method.c (make_thunk, use_thunk): Copy DECL_COMDAT. Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/method.c
Should be fixed now.
Indeed, thanks. Rainer