Skip to content

Commit

Permalink
[C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit
Browse files Browse the repository at this point in the history
of dynamic classes

Close llvm#70585 and reflect
itanium-cxx-abi/cxx-abi#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.
  • Loading branch information
ChuanqiXu9 committed Dec 20, 2023
1 parent beb2c7f commit 908a028
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 17 deletions.
32 changes: 25 additions & 7 deletions clang/lib/CodeGen/CGVTables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,15 @@ 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.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
Expand Down Expand Up @@ -1180,6 +1189,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);
Expand All @@ -1189,13 +1213,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
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/CodeGen/ItaniumCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 17 additions & 10 deletions clang/test/CodeGenCXX/modules-vtable.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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
54 changes: 54 additions & 0 deletions clang/test/CodeGenCXX/pr70585.cppm
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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

// 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() {
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

0 comments on commit 908a028

Please sign in to comment.