]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c++/modules: Fix modules and LTO with header units [PR118961]
authorNathaniel Shead <nathanieloshead@gmail.com>
Fri, 28 Mar 2025 12:38:26 +0000 (23:38 +1100)
committerNathaniel Shead <nathanieloshead@gmail.com>
Sat, 29 Mar 2025 09:14:06 +0000 (20:14 +1100)
This patch makes some adjustments required to get a simple modules
testcase working with LTO.  There are two main issues fixed.

Firstly, modules only streams the maybe-in-charge constructor, and any
clones are recreated on stream-in.  These clones are copied from the
existing function decl and then adjusted.  This caused issues because
the clones were getting incorrectly marked as abstract, since after
clones have been created (in the imported file) the maybe-in-charge decl
gets marked as abstract.  So this patch just ensures that clones are
always created as non-abstract.

The second issue is that we need to explicitly tell cgraph that explicit
instantiations need to be emitted, otherwise LTO will elide them (as
they don't necessarily appear to be used directly) and cause link
errors.  Additionally, expand_or_defer_fn doesn't setup comdat groups
for explicit instantiations, so we need to do that here as well.
Currently this is all handled in 'mark_decl_instantiated'; this patch
splits out the linkage handling into a separate function that we can
call from modules code, maybe in GCC16 we could move this somewhere more
central.

PR c++/118961

gcc/cp/ChangeLog:

* class.cc (copy_fndecl_with_name): Mark clones as non-abstract.
* cp-tree.h (setup_explicit_instantiation_definition_linkage):
Declare new function.
* module.cc (trees_in::read_var_def): Use it.
(module_state::read_cluster): Likewise.
* pt.cc (setup_explicit_instantiation_definition_linkage): New
function.
(mark_decl_instantiated): Use it.

gcc/testsuite/ChangeLog:

* g++.dg/modules/lto-1.h: New test.
* g++.dg/modules/lto-1_a.H: New test.
* g++.dg/modules/lto-1_b.C: New test.
* g++.dg/modules/lto-1_c.C: New test.
* g++.dg/modules/lto-2_a.H: New test.
* g++.dg/modules/lto-2_b.C: New test.
* g++.dg/modules/lto-3_a.H: New test.
* g++.dg/modules/lto-3_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>
12 files changed:
gcc/cp/class.cc
gcc/cp/cp-tree.h
gcc/cp/module.cc
gcc/cp/pt.cc
gcc/testsuite/g++.dg/modules/lto-1.h [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-1_a.H [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-1_b.C [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-1_c.C [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-2_a.H [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-2_b.C [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-3_a.H [new file with mode: 0644]
gcc/testsuite/g++.dg/modules/lto-3_b.C [new file with mode: 0644]

index d5ae69b0fdfad6be026c9311d78682d517997e0e..2b694b98e56539b469da89ff2ff1aae5d44d2de0 100644 (file)
@@ -5169,6 +5169,7 @@ copy_fndecl_with_name (tree fn, tree name, tree_code code,
       set_constraints (clone, copy_node (ci));
 
   SET_DECL_ASSEMBLER_NAME (clone, NULL_TREE);
+  DECL_ABSTRACT_P (clone) = false;
   /* There's no pending inline data for this function.  */
   DECL_PENDING_INLINE_INFO (clone) = NULL;
   DECL_PENDING_INLINE_P (clone) = 0;
index f0b4484ec2a898809b23443ebefc384e012c2a6f..2f2122dcf241c97ceb33e99fed7503ec4136e22c 100644 (file)
@@ -7692,6 +7692,7 @@ extern tree fn_type_unification                   (tree, tree, tree,
                                                 tree, unification_kind_t, int,
                                                 struct conversion **,
                                                 bool, bool);
+extern void setup_explicit_instantiation_definition_linkage (tree);
 extern void mark_decl_instantiated             (tree, int);
 extern int more_specialized_fn                 (tree, tree, int);
 extern tree type_targs_deducible_from          (tree, tree);
index 214fb918e8d8bfedd54a5319b3fa7ec3abf09c2d..894c70f7225f346a31df4481d9976c92462f24a5 100644 (file)
@@ -12737,6 +12737,9 @@ trees_in::read_var_def (tree decl, tree maybe_template)
          if (maybe_dup && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (maybe_dup))
            DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = true;
          tentative_decl_linkage (decl);
+         if (DECL_EXPLICIT_INSTANTIATION (decl)
+             && !DECL_EXTERNAL (decl))
+           setup_explicit_instantiation_definition_linkage (decl);
          if (DECL_IMPLICIT_INSTANTIATION (decl)
              || (DECL_EXPLICIT_INSTANTIATION (decl)
                  && !DECL_EXTERNAL (decl))
@@ -16657,6 +16660,12 @@ module_state::read_cluster (unsigned snum)
       cfun->language->returns_abnormally = pdata.returns_abnormally;
       cfun->language->infinite_loop = pdata.infinite_loop;
 
+      /* 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 (abstract)
        ;
       else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl))
index 1f5ab4e3f71e48ec54bf0fd4e6132c2ba383dc26..417d107d69937abac037a0618761ef915c907a20 100644 (file)
@@ -25968,6 +25968,23 @@ mark_definable (tree decl)
     DECL_NOT_REALLY_EXTERN (clone) = 1;
 }
 
+/* DECL is an explicit instantiation definition, ensure that it will
+   be written out here and that it won't clash with other instantiations
+   in other translation units.  */
+
+void
+setup_explicit_instantiation_definition_linkage (tree decl)
+{
+  mark_definable (decl);
+  mark_needed (decl);
+  /* Always make artificials weak.  */
+  if (DECL_ARTIFICIAL (decl) && flag_weak)
+    comdat_linkage (decl);
+  /* We also want to put explicit instantiations in linkonce sections.  */
+  else if (TREE_PUBLIC (decl))
+    maybe_make_one_only (decl);
+}
+
 /* Called if RESULT is explicitly instantiated, or is a member of an
    explicitly instantiated class.  */
 
@@ -26005,16 +26022,8 @@ mark_decl_instantiated (tree result, int extern_p)
     }
   else
     {
-      mark_definable (result);
-      mark_needed (result);
       set_instantiating_module (result);
-      /* Always make artificials weak.  */
-      if (DECL_ARTIFICIAL (result) && flag_weak)
-       comdat_linkage (result);
-      /* For WIN32 we also want to put explicit instantiations in
-        linkonce sections.  */
-      else if (TREE_PUBLIC (result))
-       maybe_make_one_only (result);
+      setup_explicit_instantiation_definition_linkage (result);
       if (TREE_CODE (result) == FUNCTION_DECL
          && DECL_TEMPLATE_INSTANTIATED (result))
        /* If the function has already been instantiated, clear DECL_EXTERNAL,
diff --git a/gcc/testsuite/g++.dg/modules/lto-1.h b/gcc/testsuite/g++.dg/modules/lto-1.h
new file mode 100644 (file)
index 0000000..935f1de
--- /dev/null
@@ -0,0 +1,13 @@
+template <typename> struct S {
+  S() {}
+};
+template <typename> inline int x = 0;
+
+extern template struct S<char>;
+extern template int x<char>;
+
+template <typename> int* foo() {
+  static int x;
+  return &x;
+};
+extern template int* foo<char>();
diff --git a/gcc/testsuite/g++.dg/modules/lto-1_a.H b/gcc/testsuite/g++.dg/modules/lto-1_a.H
new file mode 100644 (file)
index 0000000..6ea294d
--- /dev/null
@@ -0,0 +1,9 @@
+// PR c++/118961
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+// Test explicit instantiations get emitted with LTO
+
+#include "lto-1.h"
+template struct S<char>;
+template int x<char>;
+template int* foo<char>();
diff --git a/gcc/testsuite/g++.dg/modules/lto-1_b.C b/gcc/testsuite/g++.dg/modules/lto-1_b.C
new file mode 100644 (file)
index 0000000..75d9a80
--- /dev/null
@@ -0,0 +1,9 @@
+// PR c++/118961
+// { dg-require-effective-target lto }
+// { dg-additional-options "-fmodules -flto" }
+
+#include "lto-1.h"
+
+S<char> s;
+int y = x<char>;
+int* p = foo<char>();
diff --git a/gcc/testsuite/g++.dg/modules/lto-1_c.C b/gcc/testsuite/g++.dg/modules/lto-1_c.C
new file mode 100644 (file)
index 0000000..ffd4595
--- /dev/null
@@ -0,0 +1,8 @@
+// PR c++/118961
+// { dg-module-do link }
+// { dg-require-effective-target lto }
+// { dg-additional-options "-fmodules -fno-module-lazy -flto" }
+
+#include "lto-1_a.H"
+
+int main() {}
diff --git a/gcc/testsuite/g++.dg/modules/lto-2_a.H b/gcc/testsuite/g++.dg/modules/lto-2_a.H
new file mode 100644 (file)
index 0000000..f817329
--- /dev/null
@@ -0,0 +1,11 @@
+// PR c++/118961
+// { dg-additional-options "-fmodule-header -std=c++20" }
+// { dg-module-cmi {} }
+// Test we correctly emit the bodies of cloned constructors.
+
+template <typename>
+struct S {
+  S() requires true {}
+};
+
+inline S<int> foo() { return {}; }
diff --git a/gcc/testsuite/g++.dg/modules/lto-2_b.C b/gcc/testsuite/g++.dg/modules/lto-2_b.C
new file mode 100644 (file)
index 0000000..340ff48
--- /dev/null
@@ -0,0 +1,9 @@
+// PR c++/118961
+// { dg-module-do link }
+// { dg-require-effective-target lto }
+// { dg-additional-options "-fmodules -flto -std=c++20" }
+
+import "lto-2_a.H";
+int main() {
+  foo();
+}
diff --git a/gcc/testsuite/g++.dg/modules/lto-3_a.H b/gcc/testsuite/g++.dg/modules/lto-3_a.H
new file mode 100644 (file)
index 0000000..be63699
--- /dev/null
@@ -0,0 +1,6 @@
+// PR c++/118961
+// { dg-additional-options "-fmodule-header -std=c++20" }
+// { dg-module-cmi {} }
+// We shouldn't ICE when linking against the standard library.
+
+#include <string>
diff --git a/gcc/testsuite/g++.dg/modules/lto-3_b.C b/gcc/testsuite/g++.dg/modules/lto-3_b.C
new file mode 100644 (file)
index 0000000..f459596
--- /dev/null
@@ -0,0 +1,10 @@
+// PR c++/118961
+// { dg-module-do link }
+// { dg-require-effective-target lto }
+// { dg-additional-options "-fmodules -flto -static -std=c++20" }
+
+import "lto-3_a.H";
+
+int main() {
+  std::string m_message;
+}