diff mbox series

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

Message ID ZiaFXgt+sYAPPn4G@tucnak
State New
Headers show
Series c++, v2: Retry the aliasing of base/complete cdtor optimization at import_export_decl time [PR113208] | expand

Commit Message

Jakub Jelinek April 22, 2024, 3:42 p.m. UTC 
On Wed, Apr 17, 2024 at 09:42:47AM +0200, Jakub Jelinek wrote:
> When expand_or_defer_fn is called at_eof time, it calls import_export_decl
> and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a
> couple of places to try to optimize cdtors which are known to have the
> same body by making the complete cdtor an alias to base cdtor (and in
> that case also uses *[CD]5* as comdat group name instead of the normal
> comdat group names specific to each mangled name).
> Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN,
> maybe_clone_body and can_alias_cdtor use:
>       if (DECL_ONE_ONLY (fn))
>         cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone));
> ...
>   bool can_alias = can_alias_cdtor (fn);
> ...
>       /* Tell cgraph if both ctors or both dtors are known to have
>          the same body.  */
>       if (can_alias
>           && fns[0]
>           && idx == 1
>           && cgraph_node::get_create (fns[0])->create_same_body_alias
>                (clone, fns[0]))
>         {
>           alias = true;
>           if (DECL_ONE_ONLY (fns[0]))
>             {
>               /* For comdat base and complete cdtors put them
>                  into the same, *[CD]5* comdat group instead of
>                  *[CD][12]*.  */
>               comdat_group = cdtor_comdat_group (fns[1], fns[0]);
>               cgraph_node::get_create (fns[0])->set_comdat_group (comdat_group);
>               if (symtab_node::get (clone)->same_comdat_group)
>                 symtab_node::get (clone)->remove_from_same_comdat_group ();
>               symtab_node::get (clone)->add_to_same_comdat_group
>                 (symtab_node::get (fns[0]));
>             }
>         }
> and
>   /* 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))));
> The following testcase regressed with Marek's r14-5979 change,
> when pr113208_0.C is compiled where the ctor is marked constexpr,
> we no longer perform this optimization, where
> _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
> _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
> _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
> instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
> _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
> content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
> _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
> Now, the linker seems to somehow cope with that, eventhough it
> probably keeps both copies of the ctor, but seems LTO can't cope
> with that and Honza doesn't know what it should do in that case
> (linker decides that the prevailing symbol is
> _ZN6vectorI12QualityValueEC2ERKS1_ (from the
> _ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and
> _ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU,
> from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)).
> 
> Note, the case where some constructor is marked constexpr in one
> TU and not in another one happens pretty often in libstdc++ when
> one mixes -std= flags used to compile different compilation units.
> 
> The reason the optimization doesn't trigger when the constructor is
> constexpr is that expand_or_defer_fn is called in that case much earlier
> than when it is not constexpr; in the former case it is called when we
> try to constant evaluate that constructor.  But DECL_INTERFACE_KNOWN
> is false in that case and comdat_linkage hasn't been called either
> (I think it is desirable, because comdat group is stored in the cgraph
> node and am not sure it is a good idea to create cgraph nodes for
> something that might not be needed later on at all), so maybe_clone_body
> clones the bodies, but doesn't make them as aliases.
> 
> The following patch is an attempt to redo this optimization when
> import_export_decl is called at_eof time on the base/complete cdtor
> (or deleting dtor).  It will not do anything if maybe_clone_body
> hasn't been called uyet (the TREE_ASM_WRITTEN check on the
> DECL_MAYBE_IN_CHARGE_CDTOR_P), or when one or both of the base/complete
> cdtors have been lowered already, or when maybe_clone_body called
> maybe_thunk_body and it was successful.  Otherwise retries the
> can_alias_cdtor check and makes the complete cdtor alias to the
> base cdtor with adjustments to the comdat group.

Here is an updated version of the patch which changes
-  DECL_SAVED_TREE (fns[1]) = NULL_TREE;
+  release_function_body (fns[1]);
as suggested by Honza.

Bootstrapped/regtested on x86_64-linux and i686-linux again, ok for trunk?

2024-04-22  Jakub Jelinek  <jakub@redhat.com>

	PR lto/113208
	* cp-tree.h (maybe_optimize_cdtor): Declare.
	* decl2.cc (import_export_decl): Call it for cloned cdtors.
	* optimize.cc (maybe_optimize_cdtor): New function.

	* g++.dg/lto/pr113208_0.C: New test.
	* g++.dg/lto/pr113208_1.C: New file.
	* g++.dg/lto/pr113208.h: New file.



	Jakub

Comments

Jason Merrill April 23, 2024, 3:14 a.m. UTC | #1 
On 4/22/24 08:42, Jakub Jelinek wrote:
> On Wed, Apr 17, 2024 at 09:42:47AM +0200, Jakub Jelinek wrote:
>> When expand_or_defer_fn is called at_eof time, it calls import_export_decl
>> and then maybe_clone_body, which uses DECL_ONE_ONLY and comdat name in a
>> couple of places to try to optimize cdtors which are known to have the
>> same body by making the complete cdtor an alias to base cdtor (and in
>> that case also uses *[CD]5* as comdat group name instead of the normal
>> comdat group names specific to each mangled name).
>> Now, this optimization depends on DECL_ONE_ONLY and DECL_INTERFACE_KNOWN,
>> maybe_clone_body and can_alias_cdtor use:
>>        if (DECL_ONE_ONLY (fn))
>>          cgraph_node::get_create (clone)->set_comdat_group (cxx_comdat_group (clone));
>> ...
>>    bool can_alias = can_alias_cdtor (fn);
>> ...
>>        /* Tell cgraph if both ctors or both dtors are known to have
>>           the same body.  */
>>        if (can_alias
>>            && fns[0]
>>            && idx == 1
>>            && cgraph_node::get_create (fns[0])->create_same_body_alias
>>                 (clone, fns[0]))
>>          {
>>            alias = true;
>>            if (DECL_ONE_ONLY (fns[0]))
>>              {
>>                /* For comdat base and complete cdtors put them
>>                   into the same, *[CD]5* comdat group instead of
>>                   *[CD][12]*.  */
>>                comdat_group = cdtor_comdat_group (fns[1], fns[0]);
>>                cgraph_node::get_create (fns[0])->set_comdat_group (comdat_group);
>>                if (symtab_node::get (clone)->same_comdat_group)
>>                  symtab_node::get (clone)->remove_from_same_comdat_group ();
>>                symtab_node::get (clone)->add_to_same_comdat_group
>>                  (symtab_node::get (fns[0]));
>>              }
>>          }
>> and
>>    /* 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))));
>> The following testcase regressed with Marek's r14-5979 change,
>> when pr113208_0.C is compiled where the ctor is marked constexpr,
>> we no longer perform this optimization, where
>> _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
>> _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
>> _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
>> instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
>> _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
>> content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
>> _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.

This seems like an ABI bug that could use a non-LTO testcase.

>> Now, the linker seems to somehow cope with that, eventhough it
>> probably keeps both copies of the ctor, but seems LTO can't cope
>> with that and Honza doesn't know what it should do in that case
>> (linker decides that the prevailing symbol is
>> _ZN6vectorI12QualityValueEC2ERKS1_ (from the
>> _ZN6vectorI12QualityValueEC2ERKS1_ comdat group) and
>> _ZN6vectorI12QualityValueEC1ERKS1_ alias (from the other TU,
>> from _ZN6vectorI12QualityValueEC5ERKS1_ comdat group)).
>>
>> Note, the case where some constructor is marked constexpr in one
>> TU and not in another one happens pretty often in libstdc++ when
>> one mixes -std= flags used to compile different compilation units.
>>
>> The reason the optimization doesn't trigger when the constructor is
>> constexpr is that expand_or_defer_fn is called in that case much earlier
>> than when it is not constexpr; in the former case it is called when we
>> try to constant evaluate that constructor.  But DECL_INTERFACE_KNOWN
>> is false in that case and comdat_linkage hasn't been called either
>> (I think it is desirable, because comdat group is stored in the cgraph
>> node and am not sure it is a good idea to create cgraph nodes for
>> something that might not be needed later on at all), so maybe_clone_body
>> clones the bodies, but doesn't make them as aliases.

Hmm, cloning the bodies and then discarding them later seems like more 
extra work than creating the cgraph nodes.

Jason
Jakub Jelinek April 23, 2024, 4:04 p.m. UTC | #2 
On Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote:
> > > The following testcase regressed with Marek's r14-5979 change,
> > > when pr113208_0.C is compiled where the ctor is marked constexpr,
> > > we no longer perform this optimization, where
> > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
> > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
> > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
> > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
> > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
> > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
> > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
> 
> This seems like an ABI bug that could use a non-LTO testcase.

Well, except for the issues it causes to LTO I think it is compatible,
worst case we get the body of the ctor duplicated in the executable
and the linker picks some of the weak symbols as the symbol definitions.
Anyway, I've added a non-LTO testcase for that in the patch below.

> Hmm, cloning the bodies and then discarding them later seems like more extra
> work than creating the cgraph nodes.

So, I've tried to handle that in tentative_decl_linkage, like that function
already handles functions declared inline except for implicit template
instantiations.  If we expect that import_export_decl will do comdat_linkage
for the ctor later on do it right away.

That fixes the testcases too, but seems to regress
+FAIL: libstdc++-abi/abi_check
on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from
libstdc++.so.6:
_ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
_ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
_ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev

Will need to study why that happened, it might be that it was ok because
I think the filesystem stuff is unlike the rest compiled with no exported
templates, but would need at least some hacks in libstdc++ to preserve
previously exported symbols.
Still, feels like a risky change this late if it wouldn't break ABI of other
libraries.

2024-04-23  Jakub Jelinek  <jakub@redhat.com>

	PR lto/113208
	* decl2.cc (tentative_decl_linkage): Use comdat_linkage also
	for implicit instantiations of maybe in charge ctors/dtors
	if -fimplicit-templates or -fimplicit-inline-templates and
	-fweak and target supports aliases.

	* g++.dg/abi/comdat2.C: New test.
	* g++.dg/lto/pr113208_0.C: New test.
	* g++.dg/lto/pr113208_1.C: New file.
	* g++.dg/lto/pr113208.h: New file.

--- gcc/cp/decl2.cc.jj	2024-04-22 15:16:55.328548807 +0200
+++ gcc/cp/decl2.cc	2024-04-23 09:52:18.993250442 +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)))
 	    {
 	      /* This function must have external linkage, as
 		 otherwise DECL_INTERFACE_KNOWN would have been
--- gcc/testsuite/g++.dg/abi/comdat2.C.jj	2024-04-23 10:04:28.485964610 +0200
+++ gcc/testsuite/g++.dg/abi/comdat2.C	2024-04-23 10:05:24.757171194 +0200
@@ -0,0 +1,26 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2 -fkeep-inline-functions" }
+// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } }
+
+template <typename T>
+struct A {
+  int foo () const;
+  A (int, int);
+};
+template <typename T>
+struct B : A<T> {
+  constexpr B (const B &x) : A<T> (1, x.foo ()) {}
+  B () : A<T> (1, 2) {}
+};
+struct C;
+struct D : B<C> {};
+void bar (D);
+
+void
+baz (D x)
+{
+  bar (x);
+}
--- gcc/testsuite/g++.dg/lto/pr113208.h.jj	2024-04-23 09:44:07.523179110 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208.h	2024-04-23 09:44:07.523179110 +0200
@@ -0,0 +1,10 @@
+template <typename _Tp> struct _Vector_base {
+  int g() const;
+  _Vector_base(int, int);
+};
+template <typename _Tp>
+struct vector : _Vector_base<_Tp> {
+  CONSTEXPR vector(const vector &__x)
+      : _Vector_base<_Tp>(1, __x.g()) {}
+ vector() : _Vector_base<_Tp>(1, 2) {}
+};
--- gcc/testsuite/g++.dg/lto/pr113208_1.C.jj	2024-04-23 09:44:07.523179110 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208_1.C	2024-04-23 09:44:07.523179110 +0200
@@ -0,0 +1,6 @@
+#define CONSTEXPR 
+#include "pr113208.h"
+
+struct QualityValue;
+vector<QualityValue> values1;
+vector<QualityValue> values{values1};
--- gcc/testsuite/g++.dg/lto/pr113208_0.C.jj	2024-04-23 09:44:07.523179110 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208_0.C	2024-04-23 09:44:07.523179110 +0200
@@ -0,0 +1,13 @@
+// { dg-lto-do link }
+// { dg-lto-options { {-O1 -std=c++20 -flto}} }
+// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
+// { dg-require-linker-plugin "" }
+
+#define CONSTEXPR constexpr
+#include "pr113208.h"
+
+struct QualityValue;
+struct k : vector<QualityValue> {};
+
+void m(k);
+void n(k i) { m(i); }


	Jakub
Jonathan Wakely April 24, 2024, 9:16 a.m. UTC | #3 
On Tue, 23 Apr 2024 at 17:05, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Apr 22, 2024 at 11:14:35PM -0400, Jason Merrill wrote:
> > > > The following testcase regressed with Marek's r14-5979 change,
> > > > when pr113208_0.C is compiled where the ctor is marked constexpr,
> > > > we no longer perform this optimization, where
> > > > _ZN6vectorI12QualityValueEC2ERKS1_ was emitted in the
> > > > _ZN6vectorI12QualityValueEC5ERKS1_ comdat group and
> > > > _ZN6vectorI12QualityValueEC1ERKS1_ was made an alias to it,
> > > > instead we emit _ZN6vectorI12QualityValueEC2ERKS1_ in
> > > > _ZN6vectorI12QualityValueEC2ERKS1_ comdat group and the same
> > > > content _ZN6vectorI12QualityValueEC1ERKS1_ as separate symbol in
> > > > _ZN6vectorI12QualityValueEC1ERKS1_ comdat group.
> >
> > This seems like an ABI bug that could use a non-LTO testcase.
>
> Well, except for the issues it causes to LTO I think it is compatible,
> worst case we get the body of the ctor duplicated in the executable
> and the linker picks some of the weak symbols as the symbol definitions.
> Anyway, I've added a non-LTO testcase for that in the patch below.
>
> > Hmm, cloning the bodies and then discarding them later seems like more extra
> > work than creating the cgraph nodes.
>
> So, I've tried to handle that in tentative_decl_linkage, like that function
> already handles functions declared inline except for implicit template
> instantiations.  If we expect that import_export_decl will do comdat_linkage
> for the ctor later on do it right away.
>
> That fixes the testcases too, but seems to regress
> +FAIL: libstdc++-abi/abi_check
> on both x86_64-linux and i686-linux, in each case 8 symbols disappeared from
> libstdc++.so.6:
> _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem4_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
> _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1Ev
> _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
> _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE2EEC2Ev
> _ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2Ev
>
> Will need to study why that happened, it might be that it was ok because
> I think the filesystem stuff is unlike the rest compiled with no exported
> templates, but would need at least some hacks in libstdc++ to preserve
> previously exported symbols.

There are explicit instantiation definitions that should instantiate
those types:

src/c++17/fs_dir.cc:template class std::__shared_ptr<fs::_Dir>;
src/c++17/fs_dir.cc:template class
std::__shared_ptr<fs::recursive_directory_iterator::_Dir_stack>;
src/c++17/fs_path.cc:template class std::__shared_ptr<const
fs::filesystem_error::_Impl>;

So the missing symbols should be present in cow-fs_dir.o and cow-fs_path.o



> Still, feels like a risky change this late if it wouldn't break ABI of other
> libraries.
>
> 2024-04-23  Jakub Jelinek  <jakub@redhat.com>
>
>         PR lto/113208
>         * decl2.cc (tentative_decl_linkage): Use comdat_linkage also
>         for implicit instantiations of maybe in charge ctors/dtors
>         if -fimplicit-templates or -fimplicit-inline-templates and
>         -fweak and target supports aliases.
>
>         * g++.dg/abi/comdat2.C: New test.
>         * g++.dg/lto/pr113208_0.C: New test.
>         * g++.dg/lto/pr113208_1.C: New file.
>         * g++.dg/lto/pr113208.h: New file.
>
> --- gcc/cp/decl2.cc.jj  2024-04-22 15:16:55.328548807 +0200
> +++ gcc/cp/decl2.cc     2024-04-23 09:52:18.993250442 +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)))
>             {
>               /* This function must have external linkage, as
>                  otherwise DECL_INTERFACE_KNOWN would have been
> --- gcc/testsuite/g++.dg/abi/comdat2.C.jj       2024-04-23 10:04:28.485964610 +0200
> +++ gcc/testsuite/g++.dg/abi/comdat2.C  2024-04-23 10:05:24.757171194 +0200
> @@ -0,0 +1,26 @@
> +// PR lto/113208
> +// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
> +// { dg-additional-options "-O2 -fkeep-inline-functions" }
> +// { dg-final { scan-assembler "_ZN1BI1CEC5ERKS1_,comdat" } }
> +// { dg-final { scan-assembler-not "_ZN1BI1CEC1ERKS1_,comdat" } }
> +// { dg-final { scan-assembler-not "_ZN1BI1CEC2ERKS1_,comdat" } }
> +
> +template <typename T>
> +struct A {
> +  int foo () const;
> +  A (int, int);
> +};
> +template <typename T>
> +struct B : A<T> {
> +  constexpr B (const B &x) : A<T> (1, x.foo ()) {}
> +  B () : A<T> (1, 2) {}
> +};
> +struct C;
> +struct D : B<C> {};
> +void bar (D);
> +
> +void
> +baz (D x)
> +{
> +  bar (x);
> +}
> --- gcc/testsuite/g++.dg/lto/pr113208.h.jj      2024-04-23 09:44:07.523179110 +0200
> +++ gcc/testsuite/g++.dg/lto/pr113208.h 2024-04-23 09:44:07.523179110 +0200
> @@ -0,0 +1,10 @@
> +template <typename _Tp> struct _Vector_base {
> +  int g() const;
> +  _Vector_base(int, int);
> +};
> +template <typename _Tp>
> +struct vector : _Vector_base<_Tp> {
> +  CONSTEXPR vector(const vector &__x)
> +      : _Vector_base<_Tp>(1, __x.g()) {}
> + vector() : _Vector_base<_Tp>(1, 2) {}
> +};
> --- gcc/testsuite/g++.dg/lto/pr113208_1.C.jj    2024-04-23 09:44:07.523179110 +0200
> +++ gcc/testsuite/g++.dg/lto/pr113208_1.C       2024-04-23 09:44:07.523179110 +0200
> @@ -0,0 +1,6 @@
> +#define CONSTEXPR
> +#include "pr113208.h"
> +
> +struct QualityValue;
> +vector<QualityValue> values1;
> +vector<QualityValue> values{values1};
> --- gcc/testsuite/g++.dg/lto/pr113208_0.C.jj    2024-04-23 09:44:07.523179110 +0200
> +++ gcc/testsuite/g++.dg/lto/pr113208_0.C       2024-04-23 09:44:07.523179110 +0200
> @@ -0,0 +1,13 @@
> +// { dg-lto-do link }
> +// { dg-lto-options { {-O1 -std=c++20 -flto}} }
> +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
> +// { dg-require-linker-plugin "" }
> +
> +#define CONSTEXPR constexpr
> +#include "pr113208.h"
> +
> +struct QualityValue;
> +struct k : vector<QualityValue> {};
> +
> +void m(k);
> +void n(k i) { m(i); }
>
>
>         Jakub
>
diff mbox series

Patch

--- gcc/cp/cp-tree.h.jj	2024-04-16 17:18:37.286279533 +0200
+++ gcc/cp/cp-tree.h	2024-04-16 17:45:01.685635709 +0200
@@ -7447,6 +7447,7 @@  extern bool handle_module_option (unsign
 /* In optimize.cc */
 extern tree clone_attrs				(tree);
 extern bool maybe_clone_body			(tree);
+extern void maybe_optimize_cdtor		(tree);
 
 /* In parser.cc */
 extern tree cp_convert_range_for (tree, tree, tree, cp_decomp *, bool,
--- gcc/cp/decl2.cc.jj	2024-04-16 17:18:37.287279519 +0200
+++ gcc/cp/decl2.cc	2024-04-16 17:45:01.686635695 +0200
@@ -3568,6 +3568,9 @@  import_export_decl (tree decl)
     }
 
   DECL_INTERFACE_KNOWN (decl) = 1;
+
+  if (DECL_CLONED_FUNCTION_P (decl))
+    maybe_optimize_cdtor (decl);
 }
 
 /* Return an expression that performs the destruction of DECL, which
--- gcc/cp/optimize.cc.jj	2024-04-16 17:18:37.374278327 +0200
+++ gcc/cp/optimize.cc	2024-04-22 14:52:59.310630914 +0200
@@ -723,3 +723,58 @@  maybe_clone_body (tree fn)
   /* We don't need to process the original function any further.  */
   return 1;
 }
+
+/* If maybe_clone_body is called while the cdtor is still tentative,
+   DECL_ONE_ONLY will be false and so will be can_alias_cdtor (fn).
+   In that case we wouldn't try to optimize using an alias and instead
+   would emit separate base and complete cdtor.  The following function
+   attempts to still optimize that case when we import_export_decl
+   is called first time on one of the clones.  */
+
+void
+maybe_optimize_cdtor (tree orig_decl)
+{
+  tree fns[3];
+  tree fn = DECL_CLONED_FUNCTION (orig_decl);
+  gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (fn));
+  if (DECL_INTERFACE_KNOWN (fn)
+      || !TREE_ASM_WRITTEN (fn)
+      || !DECL_ONE_ONLY (orig_decl)
+      || symtab->global_info_ready)
+    return;
+
+  populate_clone_array (fn, fns);
+
+  if (!fns[0] || !fns[1])
+    return;
+  for (int i = 2 - !fns[2]; i >= 0; --i)
+    if (fns[i] != orig_decl && DECL_INTERFACE_KNOWN (fns[i]))
+      return;
+  DECL_INTERFACE_KNOWN (fn) = 1;
+  comdat_linkage (fn);
+  if (!can_alias_cdtor (fn))
+    return;
+  /* For comdat base and complete cdtors put them into the same,
+     *[CD]5* comdat group instead of *[CD][12]*.  */
+  auto n0 = cgraph_node::get_create (fns[0]);
+  auto n1 = cgraph_node::get_create (fns[1]);
+  auto n2 = fns[2] ? cgraph_node::get_create (fns[1]) : NULL;
+  if (n0->lowered || n1->lowered || (n2 && n2->lowered))
+    return;
+  import_export_decl (fns[0]);
+  n1->definition = false;
+  if (!n0->create_same_body_alias (fns[1], fns[0]))
+    return;
+
+  tree comdat_group = cdtor_comdat_group (fns[1], fns[0]);
+  n1 = cgraph_node::get (fns[1]);
+  n0->set_comdat_group (comdat_group);
+  if (n1->same_comdat_group)
+    n1->remove_from_same_comdat_group ();
+  n1->add_to_same_comdat_group (n0);
+  if (fns[2])
+    n2->add_to_same_comdat_group (n0);
+  import_export_decl (fns[1]);
+  /* Remove the body now that it is an alias.  */
+  release_function_body (fns[1]);
+}
--- gcc/testsuite/g++.dg/lto/pr113208_0.C.jj	2024-04-16 17:45:01.687635682 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208_0.C	2024-04-16 17:45:01.687635682 +0200
@@ -0,0 +1,13 @@ 
+// { dg-lto-do link }
+// { dg-lto-options { {-O1 -std=c++20 -flto}} }
+// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
+// { dg-require-linker-plugin "" }
+
+#define CONSTEXPR constexpr
+#include "pr113208.h"
+
+struct QualityValue;
+struct k : vector<QualityValue> {};
+
+void m(k);
+void n(k i) { m(i); }
--- gcc/testsuite/g++.dg/lto/pr113208_1.C.jj	2024-04-16 17:45:01.687635682 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208_1.C	2024-04-16 17:45:01.687635682 +0200
@@ -0,0 +1,6 @@ 
+#define CONSTEXPR 
+#include "pr113208.h"
+
+struct QualityValue;
+vector<QualityValue> values1;
+vector<QualityValue> values{values1};
--- gcc/testsuite/g++.dg/lto/pr113208.h.jj	2024-04-16 17:45:01.687635682 +0200
+++ gcc/testsuite/g++.dg/lto/pr113208.h	2024-04-16 17:45:01.687635682 +0200
@@ -0,0 +1,10 @@ 
+template <typename _Tp> struct _Vector_base {
+  int g() const;
+  _Vector_base(int, int);
+};
+template <typename _Tp>
+struct vector : _Vector_base<_Tp> {
+  CONSTEXPR vector(const vector &__x)
+      : _Vector_base<_Tp>(1, __x.g()) {}
+ vector() : _Vector_base<_Tp>(1, 2) {}
+};