On Thu, Apr 25, 2024 at 11:30:48AM -0400, Jason Merrill wrote:
> Hmm, maybe maybe_clone_body shouldn't clear DECL_SAVED_TREE for aliases, but
> rather set it to some stub like void_node?
I'll try that in stage1.
> Though with all these changes, it's probably better to go with your first
> patch for GCC 14 and delay this approach to 15. Your v1 patch is OK for 14.
Just to record, following patch passed bootstrap/regtest on x86_64-linux and
i686-linux. But I've committed the v1 version instead with the addition of
comdat2.C and comdat5.C testcases from this patch now and in stage1 will
post an incremental diff.
Thanks.
2024-04-25 Jakub Jelinek <ja...@redhat.com>
Jason Merrill <ja...@redhat.com>
PR lto/113208
* decl2.cc (tentative_decl_linkage): Call maybe_make_one_only
for implicit instantiations of maybe in charge ctors/dtors
declared inline.
(c_parse_final_cleanups): Don't skip used same body aliases which
have non-NULL DECL_SAVED_TREE on the alias target. 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.
* decl.cc (cxx_comdat_group): For DECL_CLONED_FUNCTION_P
functions if SUPPORTS_ONE_ONLY return DECL_COMDAT_GROUP if already
set.
* g++.dg/abi/comdat2.C: New test.
* g++.dg/abi/comdat3.C: New test.
* g++.dg/abi/comdat4.C: New test.
* g++.dg/abi/comdat5.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-24 18:28:22.299513620 +0200
+++ gcc/cp/decl2.cc 2024-04-25 16:19:17.385547357 +0200
@@ -3312,16 +3312,23 @@ tentative_decl_linkage (tree decl)
linkage of all functions, and as that causes writes to
the data mapped in from the PCH file, it's advantageous
to mark the functions at this point. */
- if (DECL_DECLARED_INLINE_P (decl)
- && (!DECL_IMPLICIT_INSTANTIATION (decl)
- || DECL_DEFAULTED_FN (decl)))
+ if (DECL_DECLARED_INLINE_P (decl))
{
- /* This function must have external linkage, as
- otherwise DECL_INTERFACE_KNOWN would have been
- set. */
- gcc_assert (TREE_PUBLIC (decl));
- comdat_linkage (decl);
- DECL_INTERFACE_KNOWN (decl) = 1;
+ if (!DECL_IMPLICIT_INSTANTIATION (decl)
+ || DECL_DEFAULTED_FN (decl))
+ {
+ /* This function must have external linkage, as
+ otherwise DECL_INTERFACE_KNOWN would have been
+ set. */
+ gcc_assert (TREE_PUBLIC (decl));
+ comdat_linkage (decl);
+ DECL_INTERFACE_KNOWN (decl) = 1;
+ }
+ else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
+ /* For implicit instantiations of cdtors try to make
+ it comdat, so that maybe_clone_body can use aliases.
+ See PR113208. */
+ maybe_make_one_only (decl);
}
}
else if (VAR_P (decl))
@@ -5264,7 +5271,19 @@ c_parse_final_cleanups (void)
generate_tls_wrapper (decl);
if (!DECL_SAVED_TREE (decl))
- continue;
+ {
+ cgraph_node *node;
+ tree tgt;
+ /* Even when maybe_clone_body created same body alias
+ has no DECL_SAVED_TREE, if its alias target does,
+ don't skip it. */
+ if (!DECL_CLONED_FUNCTION (decl)
+ || !(node = cgraph_node::get (decl))
+ || !node->cpp_implicit_alias
+ || !(tgt = node->get_alias_target_tree ())
+ || !DECL_SAVED_TREE (tgt))
+ continue;
+ }
cgraph_node *node = cgraph_node::get_create (decl);
@@ -5292,7 +5311,7 @@ c_parse_final_cleanups (void)
node = node->get_alias_target ();
node->call_for_symbol_thunks_and_aliases (clear_decl_external,
- NULL, true);
+ NULL, true);
/* If we mark !DECL_EXTERNAL one of the symbols in some comdat
group, we need to mark all symbols in the same comdat group
that way. */
@@ -5302,7 +5321,7 @@ c_parse_final_cleanups (void)
next != node;
next = dyn_cast<cgraph_node *> (next->same_comdat_group))
next->call_for_symbol_thunks_and_aliases (clear_decl_external,
- NULL, true);
+ NULL, true);
}
/* If we're going to need to write this function out, and
--- gcc/cp/optimize.cc.jj 2024-04-25 09:47:24.936736833 +0200
+++ gcc/cp/optimize.cc 2024-04-25 10:19:25.338332564 +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)));
}
/* FN is a [cd]tor, fns is a pointer to an array of length 3. Fill fns
--- gcc/cp/decl.cc.jj 2024-04-24 18:28:22.231514532 +0200
+++ gcc/cp/decl.cc 2024-04-25 09:47:06.855991224 +0200
@@ -19254,6 +19254,14 @@ cxx_comdat_group (tree decl)
else
break;
}
+ /* If a ctor/dtor has already set the comdat group by
+ maybe_clone_body, don't override it. */
+ if (SUPPORTS_ONE_ONLY
+ && TREE_CODE (decl) == FUNCTION_DECL
+ && DECL_CLONED_FUNCTION_P (decl)
+ && SUPPORTS_ONE_ONLY)
+ if (tree comdat = DECL_COMDAT_GROUP (decl))
+ return comdat;
}
return decl;
--- gcc/testsuite/g++.dg/abi/comdat2.C.jj 2024-04-25 09:47:06.855991224
+0200
+++ gcc/testsuite/g++.dg/abi/comdat2.C 2024-04-25 09:47:06.855991224 +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/abi/comdat3.C.jj 2024-04-25 09:47:06.855991224
+0200
+++ gcc/testsuite/g++.dg/abi/comdat3.C 2024-04-25 09:47:06.855991224 +0200
@@ -0,0 +1,22 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2" }
+// { dg-final { scan-assembler "_ZN1M1SINS_1P1TELN1N1LE2EEC5Ev,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1M1SINS_1P1TELN1N1LE2EEC1Ev,comdat" } }
+// { dg-final { scan-assembler-not "_ZN1M1SINS_1P1TELN1N1LE2EEC2Ev,comdat" } }
+
+namespace N { enum L { L1, L2, L3 } const O = L3; }
+namespace M {
+ using N::O;
+ using N::L;
+ template <typename, L = O>
+ struct S { constexpr S () {} };
+ namespace P {
+ struct T;
+ struct U { S<T> u; };
+ void foo () { U (); }
+ }
+ extern template class S<P::T>;
+}
+namespace p = M::P;
+template class M::S<p::T>;
--- gcc/testsuite/g++.dg/abi/comdat4.C.jj 2024-04-25 10:27:54.141172257
+0200
+++ gcc/testsuite/g++.dg/abi/comdat4.C 2024-04-25 10:28:22.493773334 +0200
@@ -0,0 +1,28 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2" }
+// { 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);
+}
+
+template struct B<C>;
--- gcc/testsuite/g++.dg/abi/comdat5.C.jj 2024-04-25 10:28:35.842585513
+0200
+++ gcc/testsuite/g++.dg/abi/comdat5.C 2024-04-25 10:28:52.413352362 +0200
@@ -0,0 +1,28 @@
+// PR lto/113208
+// { dg-do compile { target { c++11 && { *-*-*gnu* } } } }
+// { dg-additional-options "-O2" }
+// { dg-final { scan-assembler-not "_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);
+}
+
+extern template struct B<C>;
--- gcc/testsuite/g++.dg/lto/pr113208_0.C.jj 2024-04-25 09:47:06.856991210
+0200
+++ gcc/testsuite/g++.dg/lto/pr113208_0.C 2024-04-25 09:47:06.855991224
+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-25 09:47:06.856991210
+0200
+++ gcc/testsuite/g++.dg/lto/pr113208_1.C 2024-04-25 09:47:06.856991210
+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-25 09:47:06.856991210
+0200
+++ gcc/testsuite/g++.dg/lto/pr113208.h 2024-04-25 09:47:06.856991210 +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) {}
+};
Jakub
