Re: [PATCH] c++: Optimize in maybe_clone_body aliases even when not at_eof [PR113208]

[Jakub Jelinek <jakub@redhat.com>]

On Fri, May 10, 2024 at 03:59:25PM -0400, Jason Merrill wrote:
> > 2024-05-09  Jakub Jelinek  <jakub@redhat.com>
> > 	    Jason Merrill  <jason@redhat.com>
> > 
> > 	PR lto/113208
> > 	* cp-tree.h (maybe_optimize_cdtor): Remove.
> > 	* decl2.cc (tentative_decl_linkage): Call maybe_make_one_only
> > 	for implicit instantiations of maybe in charge ctors/dtors
> > 	declared inline.
> > 	(import_export_decl): Don't call maybe_optimize_cdtor.
> > 	(c_parse_final_cleanups): Formatting fixes.
> > 	* optimize.cc (can_alias_cdtor): Adjust condition, for
> > 	HAVE_COMDAT_GROUP && DECL_ONE_ONLY && DECL_WEAK return true even
> > 	if not DECL_INTERFACE_KNOWN.
> 
> > --- gcc/cp/optimize.cc.jj	2024-04-25 20:33:30.771858912 +0200
> > +++ gcc/cp/optimize.cc	2024-05-09 17:10:23.920478922 +0200
> > @@ -220,10 +220,8 @@ can_alias_cdtor (tree fn)
> >     gcc_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (fn));
> >     /* Don't use aliases for weak/linkonce definitions unless we can put both
> >        symbols in the same COMDAT group.  */
> > -  return (DECL_INTERFACE_KNOWN (fn)
> > -	  && (SUPPORTS_ONE_ONLY || !DECL_WEAK (fn))
> > -	  && (!DECL_ONE_ONLY (fn)
> > -	      || (HAVE_COMDAT_GROUP && DECL_WEAK (fn))));
> > +  return (DECL_WEAK (fn) ? (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn))
> > +			 : (DECL_INTERFACE_KNOWN (fn) && !DECL_ONE_ONLY (fn)));
> 
> Hmm, would
> 
> (DECL_ONE_ONLY (fn) ? HAVE_COMDAT_GROUP
>  : (DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn)))
> 
> make sense instead?  I don't think DECL_WEAK is necessary for COMDAT.

I think it isn't indeed necessary for COMDAT, although e.g. comdat_linkage
will not call make_decl_one_only if !flag_weak.

But I think it is absolutely required for the alias cdtor optimization
in question, because otherwise it would be an ABI change.
Consider older version of GCC or some other compiler emitting
_ZN6vectorI12QualityValueEC1ERKS1_
and
_ZN6vectorI12QualityValueEC2ERKS1_
symbols not as aliases, each in their own comdat groups, so
.text._ZN6vectorI12QualityValueEC1ERKS1_ in _ZN6vectorI12QualityValueEC1ERKS1_
comdat group and
.text._ZN6vectorI12QualityValueEC2ERKS1_ in _ZN6vectorI12QualityValueEC2ERKS1_
comdat group.  And then comes GCC with the above patch without the DECL_WEAK
check in there, and decides to use alias, so
_ZN6vectorI12QualityValueEC1ERKS1_ is an alias to
_ZN6vectorI12QualityValueEC2ERKS1_ and both live in
.text._ZN6vectorI12QualityValueEC2ERKS1_ section in
_ZN6vectorI12QualityValueEC5ERKS1_ comdat group.  If you mix TUs with this,
the linker can keep one of the section sets from the _ZN6vectorI12QualityValueEC1ERKS1_
and _ZN6vectorI12QualityValueEC2ERKS1_ and _ZN6vectorI12QualityValueEC5ERKS1_
comdat groups.  If there is no .weak for the symbols, this will fail to
link, one can emit it either the old way or the new way but never both, it
is part of an ABI.
While with .weak, mixing it is possible, worst case one gets some unused
code in the linked binary or shared library.  Of course the desirable case
is that there is no mixing and there is no unused code, but if it happens,
no big deal.  Without .weak it is a big deal.

	Jakub