[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
Tue Dec 19 01:04:30 PST 2023
https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/75912
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.
>From 38b3178137b5bfafd8cd0a3194111029b2398298 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 | 27 ++++++++----
clang/lib/CodeGen/ItaniumCXXABI.cpp | 6 +++
clang/lib/CodeGen/ModuleBuilder.cpp | 2 +-
clang/test/CodeGenCXX/modules-vtable.cppm | 27 +++++++-----
clang/test/CodeGenCXX/pr70585.cppm | 50 +++++++++++++++++++++++
5 files changed, 94 insertions(+), 18 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..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
More information about the cfe-commits
mailing list