https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912
>From 4b595ee5b7a5fefb1e9ec0a152bce587ba463b4b Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/8] [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/include/clang/AST/DeclBase.h | 3 ++ clang/lib/AST/DeclBase.cpp | 9 +++++ clang/lib/CodeGen/CGVTables.cpp | 28 ++++++++++---- clang/lib/CodeGen/CodeGenModule.cpp | 7 ++++ clang/lib/CodeGen/ItaniumCXXABI.cpp | 3 ++ clang/lib/Sema/SemaDecl.cpp | 9 +++++ clang/lib/Sema/SemaDeclCXX.cpp | 12 ++++-- clang/lib/Serialization/ASTReaderDecl.cpp | 6 +++ clang/lib/Serialization/ASTWriterDecl.cpp | 6 +++ clang/test/CodeGenCXX/modules-vtable.cppm | 31 +++++++++------ clang/test/CodeGenCXX/pr70585.cppm | 47 +++++++++++++++++++++++ 11 files changed, 138 insertions(+), 23 deletions(-) create mode 100644 clang/test/CodeGenCXX/pr70585.cppm diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 5f19af1891b74..3310f57acc683 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -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; diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index e64a8326e8d5d..78768792f1032 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -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) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 001633453f242..55c3032dc9332 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -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); @@ -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); @@ -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 diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index dd4a665ebc78b..2f09abcc15fbf 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -6867,6 +6867,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->getDefinition() && CRD->isDynamicClass() && + CRD->isInCurrentModuleUnit()) + 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 8427286dee887..2cd6bb7eb3f89 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -1830,6 +1830,9 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT, if (VTable->hasInitializer()) return; + if (RD->shouldEmitInExternalSource()) + 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 95a6fe66babae..8d4c639c1c30f 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -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. diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 37f0df2a6a27d..6053177515f70 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -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->isInNamedModule()) + 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 cf2dc32e30b91..b458e74543364 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -3239,6 +3239,12 @@ bool ASTReader::isConsumerInterestedIn(Decl *D) { 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->isInCurrentModuleUnit() && + Class->getDefinition() && Class->isDynamicClass(); + return false; } diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 5a6ab4408eb2b..49b2f9bc1e6cf 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -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)); diff --git a/clang/test/CodeGenCXX/modules-vtable.cppm b/clang/test/CodeGenCXX/modules-vtable.cppm index fb179b1de4880..5cc3504d72628 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() { @@ -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 @@ -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 +// CHECK-NOT: @_ZTIW1M1C diff --git a/clang/test/CodeGenCXX/pr70585.cppm b/clang/test/CodeGenCXX/pr70585.cppm new file mode 100644 index 0000000000000..ad4e13589d86e --- /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 -emit-llvm -o - | FileCheck %t/layer1.cppm +// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -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: -emit-llvm -o - | FileCheck %t/layer1.cppm +// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple -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 >From 4c654cecd5f9cc4fe3ffd7f47933ebfb1b1ce83f Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Thu, 6 Jun 2024 14:04:05 +0800 Subject: [PATCH 2/8] update --- clang/lib/Serialization/ASTReaderDecl.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index b458e74543364..cf2dc32e30b91 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -3239,12 +3239,6 @@ bool ASTReader::isConsumerInterestedIn(Decl *D) { 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->isInCurrentModuleUnit() && - Class->getDefinition() && Class->isDynamicClass(); - return false; } >From 5a65491f835f01588c6a6186847aa57a4050d931 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Fri, 7 Jun 2024 11:12:05 +0800 Subject: [PATCH 3/8] Update --- clang/lib/CodeGen/CGVTables.cpp | 3 ++- clang/lib/CodeGen/CodeGenModule.cpp | 3 ++- clang/lib/CodeGen/ItaniumCXXABI.cpp | 3 --- clang/lib/CodeGen/ModuleBuilder.cpp | 3 +++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 55c3032dc9332..f25b43c18205c 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1241,7 +1241,8 @@ void CodeGenModule::EmitDeferredVTables() { #endif for (const CXXRecordDecl *RD : DeferredVTables) - if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD)) + if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD) && + !RD->shouldEmitInExternalSource()) VTables.GenerateClassData(RD); else if (shouldOpportunisticallyEmitVTables()) OpportunisticVTables.push_back(RD); diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 2f09abcc15fbf..17d8fedf1bf41 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -3222,7 +3222,8 @@ void CodeGenModule::EmitVTablesOpportunistically() { for (const CXXRecordDecl *RD : OpportunisticVTables) { assert(getVTables().isVTableExternal(RD) && "This queue should only contain external vtables"); - if (getCXXABI().canSpeculativelyEmitVTable(RD)) + if (getCXXABI().canSpeculativelyEmitVTable(RD) && + !RD->shouldEmitInExternalSource()) VTables.GenerateClassData(RD); } OpportunisticVTables.clear(); diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 2cd6bb7eb3f89..8427286dee887 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -1830,9 +1830,6 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT, if (VTable->hasInitializer()) return; - if (RD->shouldEmitInExternalSource()) - 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 df85295cfb2e2..9e3de7949a104 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -318,6 +318,9 @@ namespace { if (Diags.hasUnrecoverableErrorOccurred()) return; + if (RD->shouldEmitInExternalSource()) + return; + Builder->EmitVTable(RD); } }; >From de4b8423b7b67ec77abd2faeeec4d5d0f18a8273 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Tue, 11 Jun 2024 14:36:13 +0800 Subject: [PATCH 4/8] Update --- clang/lib/CodeGen/CodeGenModule.cpp | 3 +-- clang/lib/CodeGen/ItaniumCXXABI.cpp | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 17d8fedf1bf41..2f09abcc15fbf 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -3222,8 +3222,7 @@ void CodeGenModule::EmitVTablesOpportunistically() { for (const CXXRecordDecl *RD : OpportunisticVTables) { assert(getVTables().isVTableExternal(RD) && "This queue should only contain external vtables"); - if (getCXXABI().canSpeculativelyEmitVTable(RD) && - !RD->shouldEmitInExternalSource()) + if (getCXXABI().canSpeculativelyEmitVTable(RD)) VTables.GenerateClassData(RD); } OpportunisticVTables.clear(); diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 8427286dee887..c15176463866a 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -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()) { >From aecb47365702825e5b5800b02b974871b11104c5 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Wed, 12 Jun 2024 17:30:44 +0800 Subject: [PATCH 5/8] update --- .../include/clang/Serialization/ASTBitCodes.h | 3 +++ clang/include/clang/Serialization/ASTReader.h | 6 +++++ clang/include/clang/Serialization/ASTWriter.h | 7 ++++++ clang/lib/CodeGen/CGVTables.cpp | 3 +-- clang/lib/CodeGen/CodeGenModule.cpp | 6 ----- clang/lib/Serialization/ASTReader.cpp | 11 +++++++++ clang/lib/Serialization/ASTReaderDecl.cpp | 4 ++++ clang/lib/Serialization/ASTWriter.cpp | 23 +++++++++++++++++++ 8 files changed, 55 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 4ce6cd74dd834..a4728b1c06b3f 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -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. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 0a9006223dcbd..08f302c01f538 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -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 @@ -1514,6 +1519,7 @@ class ASTReader bool isConsumerInterestedIn(Decl *D); void PassInterestingDeclsToConsumer(); void PassInterestingDeclToConsumer(Decl *D); + void PassVTableToConsumer(CXXRecordDecl *RD); void finishPendingActions(); void diagnoseOdrViolations(); diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index fcc007d6f8637..e66b675510179 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -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. @@ -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; @@ -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; } diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index f25b43c18205c..55c3032dc9332 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1241,8 +1241,7 @@ void CodeGenModule::EmitDeferredVTables() { #endif for (const CXXRecordDecl *RD : DeferredVTables) - if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD) && - !RD->shouldEmitInExternalSource()) + if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD)) VTables.GenerateClassData(RD); else if (shouldOpportunisticallyEmitVTables()) OpportunisticVTables.push_back(RD); diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 2f09abcc15fbf..0de0fc94eb07b 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -6867,12 +6867,6 @@ 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->getDefinition() && CRD->isDynamicClass() && - CRD->isInCurrentModuleUnit()) - EmitVTable(CRD); // Emit any static data members, they may be definitions. for (auto *I : CRD->decls()) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 89bab014c86ba..a2c322087fd1e 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -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 @@ -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; diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index cf2dc32e30b91..9e7c52ad98dec 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -4219,6 +4219,10 @@ void ASTReader::PassInterestingDeclsToConsumer() { // If we add any new potential interesting decl in the last call, consume it. ConsumingPotentialInterestingDecls(); + + for (GlobalDeclID ID : VTablesToEmit) + PassVTableToConsumer(cast<CXXRecordDecl>(GetDecl(ID))); + VTablesToEmit.clear(); } void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index ef165979f9a9e..713091e070805 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -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); @@ -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 //===----------------------------------------------------------------------===// @@ -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) { @@ -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, >From c136838b5343f8f71c4eb4d5bfe3d8c4bf52ab57 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Thu, 13 Jun 2024 10:18:20 +0800 Subject: [PATCH 6/8] Use isInCurrentModuleUnit --- clang/lib/Sema/SemaDeclCXX.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 6053177515f70..412381e5e51fd 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18709,7 +18709,7 @@ bool Sema::DefineUsedVTables() { const CXXMethodDecl *KeyFunction = Context.getCurrentKeyFunction(Class); // V-tables for non-template classes with an owning module are always // uniquely emitted in that module. - if (Class->isInNamedModule()) + if (Class->isInCurrentModuleUnit()) DefineVTable = true; else if (KeyFunction && !KeyFunction->hasBody()) { // If this class has a key function, but that key function is >From 65524b6f67783d4c6d42e2adb06dded5964f2ddd Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Fri, 14 Jun 2024 10:34:25 +0800 Subject: [PATCH 7/8] Update --- clang/lib/CodeGen/ModuleBuilder.cpp | 3 --- clang/lib/Sema/SemaDeclCXX.cpp | 2 +- clang/lib/Serialization/ASTReaderDecl.cpp | 7 +++++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp index 9e3de7949a104..df85295cfb2e2 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -318,9 +318,6 @@ namespace { if (Diags.hasUnrecoverableErrorOccurred()) return; - if (RD->shouldEmitInExternalSource()) - return; - Builder->EmitVTable(RD); } }; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 412381e5e51fd..7718b24955b06 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18759,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 diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 9e7c52ad98dec..eb0c8c6c099b1 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -4220,8 +4220,11 @@ void ASTReader::PassInterestingDeclsToConsumer() { // If we add any new potential interesting decl in the last call, consume it. ConsumingPotentialInterestingDecls(); - for (GlobalDeclID ID : VTablesToEmit) - PassVTableToConsumer(cast<CXXRecordDecl>(GetDecl(ID))); + for (GlobalDeclID ID : VTablesToEmit) { + auto *RD = cast<CXXRecordDecl>(GetDecl(ID)); + assert(!RD->shouldEmitInExternalSource()); + PassVTableToConsumer(RD); + } VTablesToEmit.clear(); } >From 5401fe635fd30d4c16edd4ca7db5d5e65234e4fa Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Mon, 17 Jun 2024 10:23:52 +0800 Subject: [PATCH 8/8] Remove additional blank lines --- clang/lib/CodeGen/CodeGenModule.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 0de0fc94eb07b..dd4a665ebc78b 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -6867,7 +6867,6 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) { if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never) DI->completeUnusedClass(*CRD); } - // Emit any static data members, they may be definitions. for (auto *I : CRD->decls()) if (isa<VarDecl>(I) || isa<CXXRecordDecl>(I)) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
