Skip to content

Commit

Permalink
[C++20] [Modules] [Itanium ABI] Generate the vtable in the module uni…
Browse files Browse the repository at this point in the history
…t of dynamic classes (llvm#75912)

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 authored Jun 17, 2024
1 parent ef18986 commit 15bb026
Show file tree
Hide file tree
Showing 15 changed files with 183 additions and 24 deletions.
3 changes: 3 additions & 0 deletions clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,9 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;

/// Whether this declaration comes from the same module unit being compiled.
bool isInCurrentModuleUnit() const;

/// Whether the definition of the declaration should be emitted in external
/// sources.
bool shouldEmitInExternalSource() const;
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,9 @@ enum ASTRecordTypes {

/// Record code for \#pragma clang unsafe_buffer_usage begin/end
PP_UNSAFE_BUFFER_USAGE = 69,

/// Record code for vtables to emit.
VTABLES_TO_EMIT = 70,
};

/// Record types used within a source manager block.
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,11 @@ class ASTReader
/// the consumer eagerly.
SmallVector<GlobalDeclID, 16> EagerlyDeserializedDecls;

/// The IDs of all vtables to emit. The referenced declarations are passed
/// to the consumers's HandleVTable eagerly after passing
/// EagerlyDeserializedDecls.
SmallVector<GlobalDeclID, 16> VTablesToEmit;

/// The IDs of all tentative definitions stored in the chain.
///
/// Sema keeps track of all tentative definitions in a TU because it has to
Expand Down Expand Up @@ -1514,6 +1519,7 @@ class ASTReader
bool isConsumerInterestedIn(Decl *D);
void PassInterestingDeclsToConsumer();
void PassInterestingDeclToConsumer(Decl *D);
void PassVTableToConsumer(CXXRecordDecl *RD);

void finishPendingActions();
void diagnoseOdrViolations();
Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/Serialization/ASTWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,10 @@ class ASTWriter : public ASTDeserializationListener,
std::vector<SourceRange> NonAffectingRanges;
std::vector<SourceLocation::UIntTy> NonAffectingOffsetAdjustments;

/// A list of classes which need to emit the VTable in the corresponding
/// object file.
llvm::SmallVector<CXXRecordDecl *> PendingEmittingVTables;

/// Computes input files that didn't affect compilation of the current module,
/// and initializes data structures necessary for leaving those files out
/// during \c SourceManager serialization.
Expand Down Expand Up @@ -849,6 +853,8 @@ class ASTWriter : public ASTDeserializationListener,

bool getDoneWritingDeclsAndTypes() const { return DoneWritingDeclsAndTypes; }

void handleVTable(CXXRecordDecl *RD);

private:
// ASTDeserializationListener implementation
void ReaderInitialized(ASTReader *Reader) override;
Expand Down Expand Up @@ -943,6 +949,7 @@ class PCHGenerator : public SemaConsumer {

void InitializeSema(Sema &S) override { SemaPtr = &S; }
void HandleTranslationUnit(ASTContext &Ctx) override;
void HandleVTable(CXXRecordDecl *RD) override { Writer.handleVTable(RD); }
ASTMutationListener *GetASTMutationListener() override;
ASTDeserializationListener *GetASTDeserializationListener() override;
bool hasEmittedPCH() const { return Buffer->IsComplete; }
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,15 @@ bool Decl::isInAnotherModuleUnit() const {
return M != getASTContext().getCurrentNamedModule();
}

bool Decl::isInCurrentModuleUnit() const {
auto *M = getOwningModule();

if (!M || !M->isNamedModule())
return false;

return M == getASTContext().getCurrentNamedModule();
}

bool Decl::shouldEmitInExternalSource() const {
ExternalASTSource *Source = getASTContext().getExternalSource();
if (!Source)
Expand Down
28 changes: 21 additions & 7 deletions clang/lib/CodeGen/CGVTables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,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 (RD->isInNamedModule())
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 @@ -1185,6 +1190,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 (RD->isInNamedModule())
return RD->shouldEmitInExternalSource();

// 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 @@ -1194,13 +1214,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->shouldEmitInExternalSource() && !Def->isInlineSpecified();
return !keyFunction->hasBody(Def);
}

/// Given that we're currently at the end of the translation unit, and
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/CodeGen/ItaniumCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2130,6 +2130,9 @@ bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
if (!canSpeculativelyEmitVTableAsBaseClass(RD))
return false;

if (RD->shouldEmitInExternalSource())
return false;

// For a complete-object vtable (or more specifically, for the VTT), we need
// to be able to speculatively emit the vtables of all dynamic virtual bases.
for (const auto &B : RD->vbases()) {
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18333,6 +18333,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.
Expand Down
14 changes: 9 additions & 5 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18706,11 +18706,15 @@ 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->isInCurrentModuleUnit())
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 =
Expand Down Expand Up @@ -18755,7 +18759,7 @@ bool Sema::DefineUsedVTables() {
DefinedAnything = true;
MarkVirtualMembersReferenced(Loc, Class);
CXXRecordDecl *Canonical = Class->getCanonicalDecl();
if (VTablesUsed[Canonical])
if (VTablesUsed[Canonical] && !Class->shouldEmitInExternalSource())
Consumer.HandleVTable(Class);

// Warn if we're emitting a weak vtable. The vtable will be weak if there is
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3903,6 +3903,13 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
}
break;

case VTABLES_TO_EMIT:
if (F.Kind == MK_MainFile ||
getContext().getLangOpts().BuildingPCHWithObjectFile)
for (unsigned I = 0, N = Record.size(); I != N;)
VTablesToEmit.push_back(getGlobalDeclID(F, LocalDeclID(Record[I++])));
break;

case IMPORTED_MODULES:
if (!F.isModule()) {
// If we aren't loading a module (which has its own exports), make
Expand Down Expand Up @@ -8067,6 +8074,10 @@ void ASTReader::PassInterestingDeclToConsumer(Decl *D) {
Consumer->HandleInterestingDecl(DeclGroupRef(D));
}

void ASTReader::PassVTableToConsumer(CXXRecordDecl *RD) {
Consumer->HandleVTable(RD);
}

void ASTReader::StartTranslationUnit(ASTConsumer *Consumer) {
this->Consumer = Consumer;

Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4219,6 +4219,13 @@ void ASTReader::PassInterestingDeclsToConsumer() {

// If we add any new potential interesting decl in the last call, consume it.
ConsumingPotentialInterestingDecls();

for (GlobalDeclID ID : VTablesToEmit) {
auto *RD = cast<CXXRecordDecl>(GetDecl(ID));
assert(!RD->shouldEmitInExternalSource());
PassVTableToConsumer(RD);
}
VTablesToEmit.clear();
}

void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
Expand Down
23 changes: 23 additions & 0 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@ void ASTWriter::WriteBlockInfoBlock() {
RECORD(DECLS_TO_CHECK_FOR_DEFERRED_DIAGS);
RECORD(PP_ASSUME_NONNULL_LOC);
RECORD(PP_UNSAFE_BUFFER_USAGE);
RECORD(VTABLES_TO_EMIT);

// SourceManager Block.
BLOCK(SOURCE_MANAGER_BLOCK);
Expand Down Expand Up @@ -3957,6 +3958,10 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP,
Stream.EmitRecord(INTERESTING_IDENTIFIERS, InterestingIdents);
}

void ASTWriter::handleVTable(CXXRecordDecl *RD) {
PendingEmittingVTables.push_back(RD);
}

//===----------------------------------------------------------------------===//
// DeclContext's Name Lookup Table Serialization
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -5141,6 +5146,13 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) {
// Write all of the DeclsToCheckForDeferredDiags.
for (auto *D : SemaRef.DeclsToCheckForDeferredDiags)
GetDeclRef(D);

// Write all classes need to emit the vtable definitions if required.
if (isWritingStdCXXNamedModules())
for (CXXRecordDecl *RD : PendingEmittingVTables)
GetDeclRef(RD);
else
PendingEmittingVTables.clear();
}

void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) {
Expand Down Expand Up @@ -5295,6 +5307,17 @@ void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) {
}
if (!DeleteExprsToAnalyze.empty())
Stream.EmitRecord(DELETE_EXPRS_TO_ANALYZE, DeleteExprsToAnalyze);

RecordData VTablesToEmit;
for (CXXRecordDecl *RD : PendingEmittingVTables) {
if (!wasDeclEmitted(RD))
continue;

AddDeclRef(RD, VTablesToEmit);
}

if (!VTablesToEmit.empty())
Stream.EmitRecord(VTABLES_TO_EMIT, VTablesToEmit);
}

ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Serialization/ASTWriterDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1537,8 +1537,14 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
if (D->isThisDeclarationADefinition())
Record.AddCXXDefinitionData(D);

if (D->isCompleteDefinition() && D->isInNamedModule())
Writer.AddDeclRef(D, Writer.ModularCodegenDecls);

// Store (what we currently believe to be) the key function to avoid
// deserializing every method so we can compute it.
//
// FIXME: Avoid adding the key function if the class is defined in
// module purview since the key function is meaningless in module purview.
if (D->isCompleteDefinition())
Record.AddDeclRef(Context.getCurrentKeyFunction(D));

Expand Down
31 changes: 19 additions & 12 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 @@ -58,13 +61,13 @@ int use() {
return 43;
}

// CHECK-NOT: @_ZTSW3Mod4Base = constant
// CHECK-NOT: @_ZTIW3Mod4Base = constant
// CHECK: @_ZTVW3Mod4Base = external unnamed_addr
// CHECK-NOT: @_ZTSW3Mod4Base
// CHECK-NOT: @_ZTIW3Mod4Base
// 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
// CHECK-INLINE-NOT: @_ZTIW3Mod4Base
// 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
// CHECK-NOT: @_ZTIW1M1C
Loading

0 comments on commit 15bb026

Please sign in to comment.