[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
Wed Jun 12 19:18:54 PDT 2024


https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912

>From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 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 1/6] [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 600ce73c7f019..f38386381853b 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 1e9c879e371bc..153dc3351dae5 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1106,6 +1106,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 be7bf0b72dc0c..5b69e4e063bf1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6853,6 +6853,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 08483e6ebd67f..9eff1a4c4f586 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18327,6 +18327,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 631fd4e354927..e7e9ff54107a0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18684,11 +18684,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 765c083ce8df8..9a8e7417b1985 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 f8f1ed2c7fddbd83cb3a855295d52862ff787f1c Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 6 Jun 2024 14:04:05 +0800
Subject: [PATCH 2/6] 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 9a8e7417b1985..765c083ce8df8 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 8f4807f7dc32fe96908de3aafea01ca377ec2db4 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Fri, 7 Jun 2024 11:12:05 +0800
Subject: [PATCH 3/6] 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 5b69e4e063bf1..afd43b8016bc8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3219,7 +3219,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 63a03d8d94d29c1fea591f663a5657e9fcd532bc Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Tue, 11 Jun 2024 14:36:13 +0800
Subject: [PATCH 4/6] 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 afd43b8016bc8..5b69e4e063bf1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3219,8 +3219,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 8833f7e16f078f01398275b96360823261948b1c Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Wed, 12 Jun 2024 17:30:44 +0800
Subject: [PATCH 5/6] 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 fe1bd47348be1..a54d016a0dba0 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -688,6 +688,9 @@ enum ASTRecordTypes {
   /// Record code for lexical and visible block for delayed namespace in
   /// reduced BMI.
   DELAYED_NAMESPACE_LEXICAL_VISIBLE_RECORD = 68,
+
+  /// Record code for vtables to emit.
+  VTABLES_TO_EMIT = 69,
 };
 
 /// 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 bea58d60d36c4..049fa0d7e4b49 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -814,6 +814,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
@@ -1522,6 +1527,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 5b69e4e063bf1..6c35de3fe1bff 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6853,12 +6853,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 2bc00e1bb3e38..a935fb47be4e8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3907,6 +3907,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
@@ -8044,6 +8051,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 765c083ce8df8..c55dbb02b9856 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4221,6 +4221,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 d18dbad983d75..653a0565340a8 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -926,6 +926,7 @@ void ASTWriter::WriteBlockInfoBlock() {
   RECORD(PP_CONDITIONAL_STACK);
   RECORD(DECLS_TO_CHECK_FOR_DEFERRED_DIAGS);
   RECORD(PP_ASSUME_NONNULL_LOC);
+  RECORD(VTABLES_TO_EMIT);
 
   // SourceManager Block.
   BLOCK(SOURCE_MANAGER_BLOCK);
@@ -3952,6 +3953,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
 //===----------------------------------------------------------------------===//
@@ -5136,6 +5141,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) {
@@ -5290,6 +5302,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 f0cc8499cfb0c41207031bda3e0f23c0f7da9d05 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 13 Jun 2024 10:18:20 +0800
Subject: [PATCH 6/6] 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 e7e9ff54107a0..ae1c5f6e11fbc 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18687,7 +18687,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



More information about the cfe-commits mailing list