Skip to content

Commit

Permalink
c++: Implement modules ABI for vtable emissions
Browse files Browse the repository at this point in the history
This patch implements the changes described in
itanium-cxx-abi/cxx-abi#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>
  • Loading branch information
wreien committed May 2, 2024
1 parent fd48e67 commit ad30265
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 54 deletions.
7 changes: 5 additions & 2 deletions gcc/cp/class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 7 additions & 1 deletion gcc/cp/decl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down
102 changes: 67 additions & 35 deletions gcc/cp/decl2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down
12 changes: 8 additions & 4 deletions gcc/cp/module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions gcc/testsuite/g++.dg/modules/virt-2_a.C
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
9 changes: 5 additions & 4 deletions gcc/testsuite/g++.dg/modules/virt-2_b.C
Original file line number Diff line number Diff line change
Expand Up @@ -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:} } }
10 changes: 5 additions & 5 deletions gcc/testsuite/g++.dg/modules/virt-2_c.C
Original file line number Diff line number Diff line change
Expand Up @@ -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:} } }
31 changes: 31 additions & 0 deletions gcc/testsuite/g++.dg/modules/virt-4_a.C
Original file line number Diff line number Diff line change
@@ -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;
23 changes: 23 additions & 0 deletions gcc/testsuite/g++.dg/modules/virt-4_b.C
Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit ad30265

Please sign in to comment.