Re: [PATCH] c++, v3: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208]

[Jason Merrill <jason@redhat.com>]

On 4/24/24 15:47, Jakub Jelinek wrote:
> On Wed, Apr 24, 2024 at 06:39:33PM -0400, Jason Merrill wrote:
>>> --- gcc/cp/decl2.cc.jj	2024-04-23 14:49:41.933186265 +0200
>>> +++ gcc/cp/decl2.cc	2024-04-24 15:17:09.043625729 +0200
>>> @@ -3314,7 +3314,16 @@ tentative_decl_linkage (tree decl)
>>>    	     to mark the functions at this point.  */
>>>    	  if (DECL_DECLARED_INLINE_P (decl)
>>>    	      && (!DECL_IMPLICIT_INSTANTIATION (decl)
>>> -		  || DECL_DEFAULTED_FN (decl)))
>>> +		  || DECL_DEFAULTED_FN (decl)
>>> +		  /* For implicit instantiations of cdtors,
>>> +		     if import_export_decl would use comdat linkage,
>>> +		     make sure to use it right away, so that maybe_clone_body
>>> +		     can use aliases.  See PR113208.  */
>>> +		  || (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)
>>> +		      && (flag_implicit_templates
>>> +			  || flag_implicit_inline_templates)
>>> +		      && flag_weak
>>> +		      && TARGET_SUPPORTS_ALIASES)))
>>
>> It seems wrong to set DECL_INTERFACE_KNOWN for cdtors that might get an
>> explicit instantiation later, and likewise for comdat_linkage when
>> !flag_weak; instead of adding this condition to the if, how about adding an
>> else like
>>
>>>            else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
>>>              /* For implicit instantiations of cdtors,
>>> if import_export_decl would use comdat linkage,
>>> make sure to use it right away, so that maybe_clone_body
>>> can use aliases.  See PR113208.  */
>>>              maybe_make_one_only (decl);
>>
>> ?
> 
> Then can_alias_cdtor would return false, because it ends with:
>    /* 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))));
> Should we change that DECL_INTERFACE_KNOWN (fn) in there to
> (DECL_INTERFACE_KNOWN (fn) || something) then and what that
> something should be?  HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn)?

Yes, I think reorganize to

((DECL_INTERFACE_KNOWN (fn) && !DECL_WEAK (fn) && !DECL_ONE_ONLY (fn))
  || (HAVE_COMDAT_GROUP && DECL_ONE_ONLY (fn))

Jason