c++: Implement modules ABI for vtable emissions

This patch implements the changes described in
https://github.com/itanium-cxx-abi/cxx-abi/pull/171.

One restriction that is lifted in the ABI that hasn't been updated here
is that the ABI no longer requires unique vtables to be emitted with
vague linkage.  I haven't changed this behaviour for this patch, but in
the future we could look into changing the relevant target hook
('class_data_always_comdat') to default to 'false'.  But the current
behaviour is more forgiving to changes in key function identification.

Since the ABI for vtables attached to named modules no longer depends on
key methods, this also resolves the issue described in PR c++/105224.

	PR c++/105224

gcc/cp/ChangeLog:

	* class.cc (finish_struct_1): Also push classes attached to a
	module into the 'keyed_classes' list.
	* decl.cc (record_key_method_defined): Don't push classes
	attached to a named module into the 'keyed_classes' list.
	* module.cc (trees_in::read_class_def): Likewise.
	* decl2.cc (import_export_class): Uniquely emit vtables for
	non-template classes attached to a named module.
	(vtables_uniquely_emitted): New function.
	(import_export_decl): Update comments. Update with knowledge
	about new kinds of uniquely emitted vtables.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/virt-2_a.C: Update linkage requirements.
	* g++.dg/modules/virt-2_b.C: Likewise.
	* g++.dg/modules/virt-2_c.C: Likewise.
	* g++.dg/modules/virt-4_a.C: New test.
	* g++.dg/modules/virt-4_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 5f258729940b6ab062f40b1e6ad2443b9050c6e7..5ef7c71af615b91fd1b457183b0a774c2475d30b 100644 (file)
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -7820,8 +7820,11 @@ finish_struct_1 (tree t)
       /* If a polymorphic class has no key method, we may emit the vtable
         in every translation unit where the class definition appears.  If
         we're devirtualizing, we can look into the vtable even if we
-        aren't emitting it.  */
-      if (!CLASSTYPE_KEY_METHOD (t))
+        aren't emitting it.
+
+        Additionally, if the class is attached to a named module, make sure
+        to always emit the vtable in this TU.  */
+      if (!CLASSTYPE_KEY_METHOD (t) || module_attach_p ())
        vec_safe_push (keyed_classes, t);
     }
 

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index df8553341334575a5cfd7fe1d2b8452f35ccf151..de0c02a39eec826dbb979a5bd69c2fead44492e6 100644 (file)
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -18481,7 +18481,13 @@ record_key_method_defined (tree fndecl)
     {
       tree fnclass = DECL_CONTEXT (fndecl);
       if (fndecl == CLASSTYPE_KEY_METHOD (fnclass))
-       vec_safe_push (keyed_classes, fnclass);
+       {
+         tree classdecl = TYPE_NAME (fnclass);
+         /* Classes attached to a named module are already handled.  */
+         if (!DECL_LANG_SPECIFIC (classdecl)
+             || !DECL_MODULE_ATTACH_P (classdecl))
+           vec_safe_push (keyed_classes, fnclass);
+       }
     }
 }
 

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index b8dc55b51d9dc3a6847c2647d04f2de02e853b97..1f84878b2b9f0feb8d90053d67dd4a8ae7160625 100644 (file)
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2422,17 +2422,26 @@ import_export_class (tree ctype)
       import_export = -1;
   else if (TYPE_POLYMORPHIC_P (ctype))
     {
-      /* The ABI specifies that the virtual table and associated
-        information are emitted with the key method, if any.  */
-      tree method = CLASSTYPE_KEY_METHOD (ctype);
-      /* If weak symbol support is not available, then we must be
-        careful not to emit the vtable when the key function is
-        inline.  An inline function can be defined in multiple
-        translation units.  If we were to emit the vtable in each
-        translation unit containing a definition, we would get
-        multiple definition errors at link-time.  */
-      if (method && (flag_weak || ! DECL_DECLARED_INLINE_P (method)))
-       import_export = (DECL_REALLY_EXTERN (method) ? -1 : 1);
+      tree cdecl = TYPE_NAME (ctype);
+      if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
+       /* For class types attached to a named module, the ABI specifies
+          that the tables are uniquely emitted in the object for the
+          module unit in which it is defined.  */
+       import_export = (DECL_MODULE_IMPORT_P (cdecl) ? -1 : 1);
+      else
+       {
+         /* The ABI specifies that the virtual table and associated
+            information are emitted with the key method, if any.  */
+         tree method = CLASSTYPE_KEY_METHOD (ctype);
+         /* If weak symbol support is not available, then we must be
+            careful not to emit the vtable when the key function is
+            inline.  An inline function can be defined in multiple
+            translation units.  If we were to emit the vtable in each
+            translation unit containing a definition, we would get
+            multiple definition errors at link-time.  */
+         if (method && (flag_weak || ! DECL_DECLARED_INLINE_P (method)))
+           import_export = (DECL_REALLY_EXTERN (method) ? -1 : 1);
+       }
     }
 
   /* When MULTIPLE_SYMBOL_SPACES is set, we cannot count on seeing
@@ -3325,6 +3334,35 @@ tentative_decl_linkage (tree decl)
     }
 }
 
+/* For a polymorphic class type CTYPE, whether its vtables are emitted in a
+   unique object as per the ABI.  */
+
+static bool
+vtables_uniquely_emitted (tree ctype)
+{
+  /* If the class is templated, the tables are emitted in every object that
+     references any of them.  */
+  if (CLASSTYPE_USE_TEMPLATE (ctype))
+    return false;
+
+  /* Otherwise, if the class is attached to a module, the tables are uniquely
+     emitted in the object for the module unit in which it is defined.  */
+  tree cdecl = TYPE_NAME (ctype);
+  if (DECL_LANG_SPECIFIC (cdecl) && DECL_MODULE_ATTACH_P (cdecl))
+    return true;
+
+  /* Otherwise, if the class has a key function, the tables are emitted in the
+     object for the TU containing the definition of the key function.  This is
+     unique if the key function is not inline.  */
+  tree key_method = CLASSTYPE_KEY_METHOD (ctype);
+  if (key_method && !DECL_DECLARED_INLINE_P (key_method))
+    return true;
+
+  /* Otherwise, the tables are emitted in every object that references
+     any of them.  */
+  return false;
+}
+
 /* DECL is a FUNCTION_DECL or VAR_DECL.  If the object file linkage
    for DECL has not already been determined, do so now by setting
    DECL_EXTERNAL, DECL_COMDAT and other related flags.  Until this
@@ -3399,10 +3437,6 @@ import_export_decl (tree decl)
      unit.  */
   import_p = false;
 
-  /* FIXME: Since https://github.com/itanium-cxx-abi/cxx-abi/pull/171,
-     the ABI specifies that classes attached to named modules should
-     have their vtables uniquely emitted in the object for the module
-     unit in which it is defined.  And similarly for RTTI structures.  */
   if (VAR_P (decl) && DECL_VTABLE_OR_VTT_P (decl))
     {
       class_type = DECL_CONTEXT (decl);
@@ -3411,15 +3445,13 @@ import_export_decl (tree decl)
          && CLASSTYPE_INTERFACE_ONLY (class_type))
        import_p = true;
       else if ((!flag_weak || TARGET_WEAK_NOT_IN_ARCHIVE_TOC)
-              && !CLASSTYPE_USE_TEMPLATE (class_type)
-              && CLASSTYPE_KEY_METHOD (class_type)
-              && !DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD (class_type)))
-       /* The ABI requires that all virtual tables be emitted with
-          COMDAT linkage.  However, on systems where COMDAT symbols
-          don't show up in the table of contents for a static
-          archive, or on systems without weak symbols (where we
-          approximate COMDAT linkage by using internal linkage), the
-          linker will report errors about undefined symbols because
+              && !vtables_uniquely_emitted (class_type))
+       /* The ABI historically required that all virtual tables be
+          emitted with COMDAT linkage.  However, on systems where
+          COMDAT symbols don't show up in the table of contents for
+          a static archive, or on systems without weak symbols (where
+          we approximate COMDAT linkage by using internal linkage),
+          the linker will report errors about undefined symbols because
           it will not see the virtual table definition.  Therefore,
           in the case that we know that the virtual table will be
           emitted in only one translation unit, we make the virtual
@@ -3440,16 +3472,17 @@ import_export_decl (tree decl)
            DECL_EXTERNAL (decl) = 0;
          else
            {
-             /* The generic C++ ABI says that class data is always
-                COMDAT, even if there is a key function.  Some
-                variants (e.g., the ARM EABI) says that class data
-                only has COMDAT linkage if the class data might be
-                emitted in more than one translation unit.  When the
-                key method can be inline and is inline, we still have
-                to arrange for comdat even though
+             /* The generic C++ ABI used to say that class data is always
+                COMDAT, even if emitted in a unique object.  This is no
+                longer true, but for now we continue to do so for
+                compatibility with programs that are not strictly valid.
+                However, some variants (e.g., the ARM EABI) explicitly say
+                that class data only has COMDAT linkage if the class data
+                might be emitted in more than one translation unit.
+                When the key method can be inline and is inline, we still
+                have to arrange for comdat even though
                 class_data_always_comdat is false.  */
-             if (!CLASSTYPE_KEY_METHOD (class_type)
-                 || DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD (class_type))
+             if (!vtables_uniquely_emitted (class_type)
                  || targetm.cxx.class_data_always_comdat ())
                {
                  /* The ABI requires COMDAT linkage.  Normally, we
@@ -3489,8 +3522,7 @@ import_export_decl (tree decl)
                  && !CLASSTYPE_INTERFACE_ONLY (type))
                {
                  comdat_p = (targetm.cxx.class_data_always_comdat ()
-                             || (CLASSTYPE_KEY_METHOD (type)
-                                 && DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD (type))));
+                             || !vtables_uniquely_emitted (class_type));
                  mark_needed (decl);
                  if (!flag_weak)
                    {

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index fac0301d80e7ee9a038332ec5663b898b8cf8fa3..f7725091f60fc5746aa99479354dc1b6873131f4 100644 (file)
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12502,10 +12502,14 @@ trees_in::read_class_def (tree defn, tree maybe_template)
 
          if (vtables)
            {
-             if (!CLASSTYPE_KEY_METHOD (type)
-                 /* Sneaky user may have defined it inline
-                    out-of-class.  */
-                 || DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD (type)))
+             if ((!CLASSTYPE_KEY_METHOD (type)
+                  /* Sneaky user may have defined it inline
+                     out-of-class.  */
+                  || DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD (type)))
+                 /* An imported non-template class attached to a module
+                    doesn't need to have its vtables emitted here.  */
+                 && (CLASSTYPE_USE_TEMPLATE (type)
+                     || !DECL_MODULE_ATTACH_P (defn)))
                vec_safe_push (keyed_classes, type);
              unsigned len = vtables->length ();
              tree *chain = &CLASSTYPE_VTABLES (type);

diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C b/gcc/testsuite/g++.dg/modules/virt-2_a.C
index b5050445c3f15e7bf3d54c0e81dd9e1ce6d97b97..580552be5a0d845904f6d9914050333f460ad9ca 100644 (file)
--- a/gcc/testsuite/g++.dg/modules/virt-2_a.C
+++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C
@@ -1,6 +1,3 @@
-// AAPCS overrides TARGET_CXX_KEY_METHOD_MAY_BE_INLINE,
-// in a way that invalidates this test.
-// { dg-skip-if "!TARGET_CXX_KEY_METHOD_MAY_BE_INLINE" { arm_eabi } } 
 // { dg-module-do run }
 // { dg-additional-options -fmodules-ts }
 export module foo;

diff --git a/gcc/testsuite/g++.dg/modules/virt-2_b.C b/gcc/testsuite/g++.dg/modules/virt-2_b.C
index 2bc5eced0135a9f737e56689d97ea2c338a08e85..305ef4725334010ad42843aba1b76289c5fd32f1 100644 (file)
--- a/gcc/testsuite/g++.dg/modules/virt-2_b.C
+++ b/gcc/testsuite/g++.dg/modules/virt-2_b.C
@@ -21,7 +21,8 @@ int main ()
   return !(Visit (&me) == 1);
 }
 
-// Again, we emit Visitor vtable and rtti here
-// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } }
-// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } }
-// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } }
+// Since https://github.com/itanium-cxx-abi/cxx-abi/pull/171
+// we only emit Visitor vtables and RTTI in its module unit
+// { dg-final { scan-assembler-not {_ZTVW3foo7Visitor:} } }
+// { dg-final { scan-assembler-not {_ZTIW3foo7Visitor:} } }
+// { dg-final { scan-assembler-not {_ZTSW3foo7Visitor:} } }

diff --git a/gcc/testsuite/g++.dg/modules/virt-2_c.C b/gcc/testsuite/g++.dg/modules/virt-2_c.C
index 7b3eeebe5089bcfb2e5d2deb9aed26104e190824..fed29cf4d54e8c087541308ff19efba4a05312f2 100644 (file)
--- a/gcc/testsuite/g++.dg/modules/virt-2_c.C
+++ b/gcc/testsuite/g++.dg/modules/virt-2_c.C
@@ -9,8 +9,8 @@ int Foo ()
   return !(Visit (&v) == 0);
 }
 
-// We do emit Visitor vtable
-// andl also we do emit rtti here
-// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } }
-// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } }
-// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } }
+// Since https://github.com/itanium-cxx-abi/cxx-abi/pull/171
+// we only emit Visitor vtables and RTTI in its module unit
+// { dg-final { scan-assembler-not {_ZTVW3foo7Visitor:} } }
+// { dg-final { scan-assembler-not {_ZTIW3foo7Visitor:} } }
+// { dg-final { scan-assembler-not {_ZTSW3foo7Visitor:} } }

diff --git a/gcc/testsuite/g++.dg/modules/virt-4_a.C b/gcc/testsuite/g++.dg/modules/virt-4_a.C
new file mode 100644 (file)
index 0000000..a269163
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/virt-4_a.C
@@ -0,0 +1,31 @@
+// { dg-additional-options "-fmodules-ts" }
+// Test changes for https://github.com/itanium-cxx-abi/cxx-abi/pull/171
+
+export module M;
+
+// Attached to module, should only be emitted in this TU
+// { dg-final { scan-assembler {_ZTVW1M8Attached:} } }
+// { dg-final { scan-assembler {_ZTIW1M8Attached:} } }
+// { dg-final { scan-assembler {_ZTSW1M8Attached:} } }
+export struct Attached {
+  virtual void key();
+};
+
+// Not attached to module, should be emitted where key function is
+// { dg-final { scan-assembler-not {_ZTV10Unattached:} } }
+// { dg-final { scan-assembler-not {_ZTI10Unattached:} } }
+// { dg-final { scan-assembler-not {_ZTS10Unattached:} } }
+export extern "C++" struct Unattached {
+  // Key function not defined here
+  virtual void key();
+};
+
+// Template, should be emitted everywhere it's used
+export template <typename T> struct Templated {
+  virtual void key() {}
+};
+
+// { dg-final { scan-assembler {_ZTVW1M9TemplatedIiE:} } }
+// { dg-final { scan-assembler {_ZTIW1M9TemplatedIiE:} } }
+// { dg-final { scan-assembler {_ZTSW1M9TemplatedIiE:} } }
+static Templated<int> x;

diff --git a/gcc/testsuite/g++.dg/modules/virt-4_b.C b/gcc/testsuite/g++.dg/modules/virt-4_b.C
new file mode 100644 (file)
index 0000000..999d48b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/virt-4_b.C
@@ -0,0 +1,23 @@
+// { dg-additional-options "-fmodules-ts" }
+
+module M;
+
+// Attached to module, shouldn't be defined in this TU
+// (was already emitted in interface unit)
+// { dg-final { scan-assembler-not {_ZTVW1M8Attached:} } }
+// { dg-final { scan-assembler-not {_ZTIW1M8Attached:} } }
+// { dg-final { scan-assembler-not {_ZTSW1M8Attached:} } }
+void Attached::key() {}
+
+// Not attached to module and this is the key function,
+// so vtables and RTTI should be emitted here
+// { dg-final { scan-assembler {_ZTV10Unattached:} } }
+// { dg-final { scan-assembler {_ZTI10Unattached:} } }
+// { dg-final { scan-assembler {_ZTS10Unattached:} } }
+extern "C++" void Unattached::key() {}
+
+// Template vtables should be emitted wherever it's used
+// { dg-final { scan-assembler {_ZTVW1M9TemplatedIiE:} } }
+// { dg-final { scan-assembler {_ZTIW1M9TemplatedIiE:} } }
+// { dg-final { scan-assembler {_ZTSW1M9TemplatedIiE:} } }
+static Templated<int> y;