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?
> 
> 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.

Ok, here is an updated patch, which sets DECL_SAVED_TREE to void_node for
the aliases together with reversion of the earlier committed patch.

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

2024-05-09  Jakub Jelinek  <ja...@redhat.com>
            Jason Merrill  <ja...@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.
        (maybe_clone_body): Don't clear DECL_SAVED_TREE, instead set it
        to void_node.
        (maybe_clone_body): Remove.
        * 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/comdat3.C: New test.
        * g++.dg/abi/comdat4.C: New test.

--- gcc/cp/cp-tree.h.jj 2024-05-09 10:30:54.775505524 +0200
+++ gcc/cp/cp-tree.h    2024-05-09 17:07:01.246206288 +0200
@@ -7451,7 +7451,6 @@ 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-05-02 09:31:17.753298180 +0200
+++ gcc/cp/decl2.cc     2024-05-09 17:11:11.676836268 +0200
@@ -3325,16 +3325,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))
@@ -3604,9 +3611,6 @@ 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
@@ -5331,7 +5335,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.  */
@@ -5341,7 +5345,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 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)));
 }
 
 /* FN is a [cd]tor, fns is a pointer to an array of length 3.  Fill fns
@@ -712,7 +710,7 @@ maybe_clone_body (tree fn)
          if (expand_or_defer_fn_1 (clone))
            emit_associated_thunks (clone);
          /* We didn't generate a body, so remove the empty one.  */
-         DECL_SAVED_TREE (clone) = NULL_TREE;
+         DECL_SAVED_TREE (clone) = void_node;
        }
       else
        expand_or_defer_fn (clone);
@@ -723,58 +721,3 @@ 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/cp/decl.cc.jj   2024-05-09 10:30:54.804505130 +0200
+++ gcc/cp/decl.cc      2024-05-09 17:07:08.400110018 +0200
@@ -19280,6 +19280,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/comdat3.C.jj       2024-05-09 17:07:27.345855068 
+0200
+++ gcc/testsuite/g++.dg/abi/comdat3.C  2024-05-09 17:07:08.400110018 +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-05-09 17:07:31.961792950 
+0200
+++ gcc/testsuite/g++.dg/abi/comdat4.C  2024-05-09 17:07:08.400110018 +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>;


        Jakub

Reply via email to