Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?

One slight concern I have is why we end up in 'maybe_thunk_body' to
start with: the imported constructor isn't DECL_ONE_ONLY (as its
external) and so 'can_alias_cdtor' returns false.  The change in
write_function_def (which I believe is necessary regardless) hides this
because we never actually emit the function definitions, but I worry
that this might somehow affect LTO, though I haven't been able to
construct a testcase which fails.  To avoid the potential of that we
could do something like this to mark such functions as DECL_ONE_ONLY
just in case; thoughts?

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index e7782627a49..c96e81aceef 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -16738,9 +16738,15 @@ module_state::read_cluster (unsigned snum)
 
       /* Make sure we emit explicit instantiations.
          FIXME do we want to do this in expand_or_defer_fn instead?  */
-      if (DECL_EXPLICIT_INSTANTIATION (decl)
-          && !DECL_EXTERNAL (decl))
-        setup_explicit_instantiation_definition_linkage (decl);
+      if (DECL_EXPLICIT_INSTANTIATION (decl))
+        {
+          if (DECL_DECLARED_INLINE_P (decl)
+              && TREE_PUBLIC (decl)
+              && DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
+            maybe_make_one_only (decl);
+          if (!DECL_EXTERNAL (decl))
+            setup_explicit_instantiation_definition_linkage (decl);
+        }
 
       if (abstract)
         ;


This bug also affects 14 after r14-11743-g6d5a6a26e28d15; a minimal fix
for the ICE that seems straight-forwardly correct is to just do the
optimize.cc hunk, and I think that might be more appropriate given we
didn't handle explicit instantiations specially then anyway.  Would such
a change (with appropriate xfailed tests) be OK?  Maybe this would even
be better for 15 also?

-- >8 --

The attached testcase ICEs in maybe_thunk_body because we haven't
created a node in the cgraph for an imported explicit instantiation yet.

We in fact really shouldn't be emitting calls at all, since an imported
explicit instantiation always exists in the TU we imported it from.  So
this patch adjusts DECL_NOT_REALLY_EXTERN handling to account for this.

        PR c++/120125

gcc/cp/ChangeLog:

        * module.cc (trees_out::write_function_def): Only set
        DECL_NOT_REALLY_EXTERN if the importer might need to emit it.
        * optimize.cc (maybe_thunk_body): Don't assume 'fn' has a cgraph
        node created.

gcc/testsuite/ChangeLog:

        * g++.dg/modules/clone-4_a.C: New test.
        * g++.dg/modules/clone-4_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
 gcc/cp/module.cc                         |  6 +++++-
 gcc/cp/optimize.cc                       |  4 ++--
 gcc/testsuite/g++.dg/modules/clone-4_a.C | 12 ++++++++++++
 gcc/testsuite/g++.dg/modules/clone-4_b.C | 12 ++++++++++++
 4 files changed, 31 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/clone-4_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/clone-4_b.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f562bf8cd91..e7782627a49 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12638,7 +12638,11 @@ trees_out::write_function_def (tree decl)
     {
       unsigned flags = 0;
 
-      flags |= 1 * DECL_NOT_REALLY_EXTERN (decl);
+      /* Whether the importer should emit this definition, if used.  */
+      flags |= 1 * (DECL_NOT_REALLY_EXTERN (decl)
+                   && (get_importer_interface (decl)
+                       != importer_interface::always_import));
+
       if (f)
        {
          flags |= 2;
diff --git a/gcc/cp/optimize.cc b/gcc/cp/optimize.cc
index 6f9a77f407a..fc4d6c2e351 100644
--- a/gcc/cp/optimize.cc
+++ b/gcc/cp/optimize.cc
@@ -309,8 +309,8 @@ maybe_thunk_body (tree fn, bool force)
       defer_mangling_aliases = save_defer_mangling_aliases;
       cgraph_node::get_create (fns[0])->set_comdat_group (comdat_group);
       cgraph_node::get_create (fns[1])->add_to_same_comdat_group
-       (cgraph_node::get_create (fns[0]));
-      symtab_node::get (fn)->add_to_same_comdat_group
+       (cgraph_node::get (fns[0]));
+      symtab_node::get_create (fn)->add_to_same_comdat_group
        (symtab_node::get (fns[0]));
       if (fns[2])
        /* If *[CD][12]* dtors go into the *[CD]5* comdat group and dtor is
diff --git a/gcc/testsuite/g++.dg/modules/clone-4_a.C 
b/gcc/testsuite/g++.dg/modules/clone-4_a.C
new file mode 100644
index 00000000000..3ee6109f23e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/clone-4_a.C
@@ -0,0 +1,12 @@
+// PR c++/120125
+// { dg-additional-options "-fmodules -fdeclone-ctor-dtor" }
+// { dg-module-cmi M }
+
+export module M;
+
+void foo();
+export template <typename _Tp> struct __shared_ptr {
+  inline __shared_ptr() { foo(); }
+};
+
+template class __shared_ptr<int>;
diff --git a/gcc/testsuite/g++.dg/modules/clone-4_b.C 
b/gcc/testsuite/g++.dg/modules/clone-4_b.C
new file mode 100644
index 00000000000..1b36cb477c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/clone-4_b.C
@@ -0,0 +1,12 @@
+// PR c++/120125
+// { dg-additional-options "-fmodules -fdeclone-ctor-dtor" }
+
+import M;
+
+int main() {
+  __shared_ptr<int> s1;
+  __shared_ptr<double> s2;
+}
+
+// { dg-final { scan-assembler-not {_ZNW1M12__shared_ptrIiEC[1-4]Ev:} } }
+// { dg-final { scan-assembler {_ZNW1M12__shared_ptrIdEC2Ev:} } }
-- 
2.47.0

Reply via email to