[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 19 01:04:55 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: Chuanqi Xu (ChuanqiXu9)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/75912.diff
5 Files Affected:
- (modified) clang/lib/CodeGen/CGVTables.cpp (+20-7)
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+6)
- (modified) clang/lib/CodeGen/ModuleBuilder.cpp (+1-1)
- (modified) clang/test/CodeGenCXX/modules-vtable.cppm (+17-10)
- (added) clang/test/CodeGenCXX/pr70585.cppm (+50)
``````````diff
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 27a2cab4f75319..06b02df93b26d8 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1045,6 +1045,15 @@ llvm::GlobalVariable::LinkageTypes
CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+
+ // Previously we'll decide the linkage of the vtable by the linkage
+ // of the key function. But within modules, the concept of key functions
+ // becomes meaningless. So the linkage of the vtable should always be
+ // external if the class is externally visible.
+ //
+ // TODO: How about the case of AppleKext, DLLExportAttr and DLLImportAttr.
+ 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.
@@ -1180,6 +1189,16 @@ 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 +1208,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/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index d173806ec8ce53..7e2c1191071c84 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 are 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/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp
index 3594f4c66e6774..76175a157ba7bb 100644
--- a/clang/lib/CodeGen/ModuleBuilder.cpp
+++ b/clang/lib/CodeGen/ModuleBuilder.cpp
@@ -317,7 +317,7 @@ namespace {
void HandleVTable(CXXRecordDecl *RD) override {
if (Diags.hasErrorOccurred())
return;
-
+
Builder->EmitVTable(RD);
}
};
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..5c8245c0efbf80
--- /dev/null
+++ b/clang/test/CodeGenCXX/pr70585.cppm
@@ -0,0 +1,50 @@
+// 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
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+ virtual ~Fruit() = default;
+ virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+ Banana() {}
+ void eval() override;
+};
+
+// CHECK-DAG: @_ZTVW3foo6Banana = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo6Banana = constant
+// CHECK-DAG: @_ZTIW3foo6Banana = constant
+//
+// CHECK-DAG: @_ZTVW3foo5Fruit = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo5Fruit = constant
+// CHECK-DAG: @_ZTIW3foo5Fruit = constant
+
+//--- layer2.cppm
+export module foo:layer2;
+import :layer1;
+export void layer2_fun() {
+ Banana *b = new Banana();
+ b->eval();
+}
+void Banana::eval() {
+}
+
+// CHECK-NOT: @_ZTVW3foo6Banana
+// CHECK-NOT: @_ZTSW3foo6Banana
+// CHECK-NOT: @_ZTIW3foo6Banana
+// CHECK-NOT: @_ZTVW3foo5Fruit
+// CHECK-NOT: @_ZTSW3foo5Fruit
+// CHECK-NOT: @_ZTIW3foo5Fruit
``````````
</details>
https://github.com/llvm/llvm-project/pull/75912
More information about the cfe-commits
mailing list