[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 25 00:47:06 PST 2024
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912
>From 630e59738990c3dd570065b8b7a050d822d68df0 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Tue, 19 Dec 2023 17:00:59 +0800
Subject: [PATCH] [C++20] [Modules] [Itanium ABI] Generate the vtable in the
module unit of dynamic classes
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
clang/lib/CodeGen/CGVTables.cpp | 28 ++++++++++----
clang/lib/CodeGen/CodeGenModule.cpp | 7 ++++
clang/lib/CodeGen/ItaniumCXXABI.cpp | 6 +++
clang/lib/Sema/SemaDecl.cpp | 9 +++++
clang/lib/Sema/SemaDeclCXX.cpp | 13 +++++--
clang/lib/Serialization/ASTReaderDecl.cpp | 15 +++++++-
clang/lib/Serialization/ASTWriterDecl.cpp | 15 +++++---
clang/test/CodeGenCXX/modules-vtable.cppm | 27 ++++++++-----
clang/test/CodeGenCXX/pr70585.cppm | 47 +++++++++++++++++++++++
9 files changed, 139 insertions(+), 28 deletions(-)
create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 27a2cab4f75319..5c8499d2594a1d 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1046,6 +1046,11 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+ // V-tables for non-template classes with an owning module are always
+ // uniquely emitted in that module.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+ return llvm::GlobalVariable::ExternalLinkage;
+
// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
@@ -1180,6 +1185,21 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - 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.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+ return M != CGM.getContext().getCurrentNamedModule();
+
// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction = CGM.getContext().getCurrentKeyFunction(RD);
@@ -1189,13 +1209,7 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
- if (!keyFunction->hasBody(Def))
- return true;
-
- assert(Def && "The body of the key function is not assigned to Def?");
- // If the non-inline key function comes from another module unit, the vtable
- // must be defined there.
- return Def->isInAnotherModuleUnit() && !Def->isInlineSpecified();
+ return !keyFunction->hasBody(Def);
}
/// Given that we're currently at the end of the translation unit, and
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index d78f2594a23764..b133695d90905b 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6738,6 +6738,13 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) {
if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
DI->completeUnusedClass(*CRD);
}
+ // If we're emitting a dynamic class from the importable module we're emitting,
+ // we always need to emit the virtual table according to the ABI requirement.
+ if (CRD->getOwningModule() && CRD->getOwningModule()->isInterfaceOrPartition() &&
+ CRD->getDefinition() && CRD->isDynamicClass() &&
+ CRD->getOwningModule() == getContext().getCurrentNamedModule())
+ EmitVTable(CRD);
+
// Emit any static data members, they may be definitions.
for (auto *I : CRD->decls())
if (isa<VarDecl>(I) || isa<CXXRecordDecl>(I))
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index d173806ec8ce53..1acf8b3e0659ff 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1801,6 +1801,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
if (VTable->hasInitializer())
return;
+ // If the class is attached to a C++ named module other than the one
+ // we're currently compiling, the vtable should be defined there.
+ if (Module *M = RD->getOwningModule();
+ M && M->isNamedModule() && M != CGM.getContext().getCurrentNamedModule())
+ return;
+
ItaniumVTableContext &VTContext = CGM.getItaniumVTableContext();
const VTableLayout &VTLayout = VTContext.getVTableLayout(RD);
llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index ffbe317d559995..aaf4a3d0baf8eb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18127,6 +18127,15 @@ void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
if (NumInitMethods > 1 || !Def->hasInitMethod())
Diag(RD->getLocation(), diag::err_sycl_special_type_num_init_method);
}
+
+ // If we're defining a dynamic class in a module interface unit, we always
+ // need to produce the vtable for it even if the vtable is not used in the
+ // current TU.
+ //
+ // The case that the current class is not dynamic is handled in MarkVTableUsed.
+ if (getCurrentModule() &&
+ getCurrentModule()->isInterfaceOrPartition())
+ MarkVTableUsed(RD->getLocation(), RD, /*DefinitionRequired=*/true);
}
// Exit this scope of this tag's definition.
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 36e53c684ac4dc..5d1524d7db1385 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18697,11 +18697,16 @@ bool Sema::DefineUsedVTables() {
bool DefineVTable = true;
- // If this class has a key function, but that key function is
- // defined in another translation unit, we don't need to emit the
- // vtable even though we're using it.
const CXXMethodDecl *KeyFunction = Context.getCurrentKeyFunction(Class);
- if (KeyFunction && !KeyFunction->hasBody()) {
+ // V-tables for non-template classes with an owning module are always
+ // uniquely emitted in that module.
+ if (Class->getOwningModule() &&
+ Class->getOwningModule()->isInterfaceOrPartition())
+ DefineVTable = true;
+ else if (KeyFunction && !KeyFunction->hasBody()) {
+ // If this class has a key function, but that key function is
+ // defined in another translation unit, we don't need to emit the
+ // vtable even though we're using it.
// The key function is in another translation unit.
DefineVTable = false;
TemplateSpecializationKind TSK =
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 547eb77930b4ee..f5ff9115aaf8cf 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2253,7 +2253,10 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) {
// Lazily load the key function to avoid deserializing every method so we can
// compute it.
- if (WasDefinition) {
+ //
+ // The key function in named module is meaningless.
+ if (WasDefinition && (!D->getOwningModule() ||
+ !D->getOwningModule()->isInterfaceOrPartition())) {
DeclID KeyFn = readDeclID();
if (KeyFn && D->isCompleteDefinition())
// FIXME: This is wrong for the ARM ABI, where some other module may have
@@ -3207,11 +3210,19 @@ static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D, bool HasBody) {
OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var));
if (const auto *Func = dyn_cast<FunctionDecl>(D))
return Func->doesThisDeclarationHaveABody() || HasBody;
-
+
if (auto *ES = D->getASTContext().getExternalSource())
if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
return true;
+ // The dynamic class defined in a named module is interesting.
+ // The code generator needs to emit its vtable there.
+ if (const auto *Class = dyn_cast<CXXRecordDecl>(D))
+ return Class->getOwningModule() &&
+ Class->getOwningModule()->isInterfaceOrPartition() &&
+ Class->getOwningModule() == Ctx.getCurrentNamedModule() &&
+ Class->getDefinition() && Class->isDynamicClass();
+
return false;
}
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 9e3299f0491848..33b2c1d3923ba2 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1482,11 +1482,16 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
Record.push_back(D->isThisDeclarationADefinition());
if (D->isThisDeclarationADefinition())
Record.AddCXXDefinitionData(D);
-
- // Store (what we currently believe to be) the key function to avoid
- // deserializing every method so we can compute it.
- if (D->isCompleteDefinition())
- Record.AddDeclRef(Context.getCurrentKeyFunction(D));
+
+ if (D->isCompleteDefinition()) {
+ if (D->getOwningModule() && D->getOwningModule()->isInterfaceOrPartition())
+ Writer.ModularCodegenDecls.push_back(Writer.GetDeclRef(D));
+ else {
+ // Store (what we currently believe to be) the key function to avoid
+ // deserializing every method so we can compute it.
+ Record.AddDeclRef(Context.getCurrentKeyFunction(D));
+ }
+ }
Code = serialization::DECL_CXX_RECORD;
}
diff --git a/clang/test/CodeGenCXX/modules-vtable.cppm b/clang/test/CodeGenCXX/modules-vtable.cppm
index fb179b1de4880b..abbbc403ace243 100644
--- a/clang/test/CodeGenCXX/modules-vtable.cppm
+++ b/clang/test/CodeGenCXX/modules-vtable.cppm
@@ -24,6 +24,8 @@
// RUN: %t/M-A.cppm -o %t/M-A.pcm
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -fmodule-file=M:A=%t/M-A.pcm \
// RUN: %t/M-B.cppm -emit-llvm -o - | FileCheck %t/M-B.cppm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 \
+// RUN: %t/M-A.pcm -emit-llvm -o - | FileCheck %t/M-A.cppm
//--- Mod.cppm
export module Mod;
@@ -41,9 +43,10 @@ Base::~Base() {}
// CHECK: @_ZTSW3Mod4Base = constant
// CHECK: @_ZTIW3Mod4Base = constant
-// CHECK-INLINE: @_ZTVW3Mod4Base = linkonce_odr {{.*}}unnamed_addr constant
-// CHECK-INLINE: @_ZTSW3Mod4Base = linkonce_odr {{.*}}constant
-// CHECK-INLINE: @_ZTIW3Mod4Base = linkonce_odr {{.*}}constant
+// With the new Itanium C++ ABI, the linkage of vtables in modules don't need to be linkonce ODR.
+// CHECK-INLINE: @_ZTVW3Mod4Base = {{.*}}unnamed_addr constant
+// CHECK-INLINE: @_ZTSW3Mod4Base = {{.*}}constant
+// CHECK-INLINE: @_ZTIW3Mod4Base = {{.*}}constant
module :private;
int private_use() {
@@ -60,11 +63,11 @@ int use() {
// CHECK-NOT: @_ZTSW3Mod4Base = constant
// CHECK-NOT: @_ZTIW3Mod4Base = constant
-// CHECK: @_ZTVW3Mod4Base = external unnamed_addr
+// CHECK: @_ZTVW3Mod4Base = external
-// CHECK-INLINE: @_ZTVW3Mod4Base = linkonce_odr {{.*}}unnamed_addr constant
-// CHECK-INLINE: @_ZTSW3Mod4Base = linkonce_odr {{.*}}constant
-// CHECK-INLINE: @_ZTIW3Mod4Base = linkonce_odr {{.*}}constant
+// CHECK-INLINE-NOT: @_ZTSW3Mod4Base = constant
+// CHECK-INLINE-NOT: @_ZTIW3Mod4Base = constant
+// CHECK-INLINE: @_ZTVW3Mod4Base = external
// Check the case that the declaration of the key function comes from another
// module unit but the definition of the key function comes from the current
@@ -82,6 +85,10 @@ int a_use() {
return 43;
}
+// CHECK: @_ZTVW1M1C = unnamed_addr constant
+// CHECK: @_ZTSW1M1C = constant
+// CHECK: @_ZTIW1M1C = constant
+
//--- M-B.cppm
export module M:B;
import :A;
@@ -93,6 +100,6 @@ int b_use() {
return 43;
}
-// CHECK: @_ZTVW1M1C = unnamed_addr constant
-// CHECK: @_ZTSW1M1C = constant
-// CHECK: @_ZTIW1M1C = constant
+// CHECK: @_ZTVW1M1C = external
+// CHECK-NOT: @_ZTSW1M1C = constant
+// CHECK-NOT: @_ZTIW1M1C = constant
diff --git a/clang/test/CodeGenCXX/pr70585.cppm b/clang/test/CodeGenCXX/pr70585.cppm
new file mode 100644
index 00000000000000..a2d6e02b5833ee
--- /dev/null
+++ b/clang/test/CodeGenCXX/pr70585.cppm
@@ -0,0 +1,47 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck %t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck %t/layer2.cppm
+//
+// Check the case about emitting object files from sources directly.
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -S -emit-llvm -o - | FileCheck %t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple -S -emit-llvm \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm -o - | FileCheck %t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+ virtual ~Fruit() = default;
+ virtual void eval();
+};
+
+// CHECK-DAG: @_ZTVW3foo5Fruit = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo5Fruit = constant
+// CHECK-DAG: @_ZTIW3foo5Fruit = constant
+
+// Testing that:
+// (1) The use of virtual functions won't produce the vtable.
+// (2) The definition of key functions won't produce the vtable.
+//
+//--- layer2.cppm
+export module foo:layer2;
+import :layer1;
+export void layer2_fun() {
+ Fruit *b = new Fruit();
+ b->eval();
+}
+void Fruit::eval() {}
+// CHECK: @_ZTVW3foo5Fruit = external unnamed_addr constant
+// CHECK-NOT: @_ZTSW3foo5Fruit
+// CHECK-NOT: @_ZTIW3foo5Fruit
More information about the cfe-commits
mailing list