[llvm-branch-commits] [clang] release/19.x: Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in the mod… (#102287) (PR #102561)

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Aug 20 00:28:28 PDT 2024


https://github.com/tru updated https://github.com/llvm/llvm-project/pull/102561

>From 64b8514e6c1a663660fbb93ec7f623b3e40a2020 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 8 Aug 2024 13:14:09 +0800
Subject: [PATCH] =?UTF-8?q?Reland=20[C++20]=20[Modules]=20[Itanium=20ABI]?=
 =?UTF-8?q?=20Generate=20the=20vtable=20in=20the=20mod=E2=80=A6=20(#102287?=
 =?UTF-8?q?)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reland https://github.com/llvm/llvm-project/pull/75912

The differences of this PR between
https://github.com/llvm/llvm-project/pull/75912 are:

- Fixed a regression in `Decl::isInAnotherModuleUnit()` in DeclBase.cpp
pointed by @mizvekov and add the corresponding test.
- Fixed the regression in windows
https://github.com/llvm/llvm-project/issues/97447. The changes are in
`CodeGenModule::getVTableLinkage` from
`clang/lib/CodeGen/CGVTables.cpp`. According to the feedbacks from MSVC
devs, the linkage of vtables won't affected by modules. So I simply
skipped the case for MSVC.

Given this is more or less fundamental to the use of modules. I hope we
can backport this to 19.x.

(cherry picked from commit 847f9cb0e868c8ec34f9aa86fdf846f8c4e0388b)
---
 clang/include/clang/AST/DeclBase.h            |   7 ++
 .../include/clang/Serialization/ASTBitCodes.h |   3 +
 clang/include/clang/Serialization/ASTReader.h |   6 +
 clang/include/clang/Serialization/ASTWriter.h |   7 ++
 clang/lib/AST/ASTContext.cpp                  |   3 +-
 clang/lib/AST/DeclBase.cpp                    |  34 +++--
 clang/lib/CodeGen/CGVTables.cpp               |  56 +++++----
 clang/lib/CodeGen/ItaniumCXXABI.cpp           |   3 +
 clang/lib/Sema/SemaDecl.cpp                   |   9 ++
 clang/lib/Sema/SemaDeclCXX.cpp                |  14 ++-
 clang/lib/Serialization/ASTReader.cpp         |  11 ++
 clang/lib/Serialization/ASTReaderDecl.cpp     |   7 ++
 clang/lib/Serialization/ASTWriter.cpp         |  33 ++++-
 clang/lib/Serialization/ASTWriterDecl.cpp     |   6 +
 clang/test/CodeGenCXX/modules-vtable.cppm     |  31 +++--
 clang/test/CodeGenCXX/pr70585.cppm            |  47 +++++++
 clang/test/Modules/pr97313.cppm               | 118 ++++++++++++++++++
 .../test/Modules/static-func-in-private.cppm  |   8 ++
 clang/test/Modules/vtable-windows.cppm        |  26 ++++
 19 files changed, 374 insertions(+), 55 deletions(-)
 create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
 create mode 100644 clang/test/Modules/pr97313.cppm
 create mode 100644 clang/test/Modules/static-func-in-private.cppm
 create mode 100644 clang/test/Modules/vtable-windows.cppm

diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 40f01abf384e92..2a4bd0f9c2fda3 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -670,6 +670,13 @@ 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;
+
   /// Whether this declaration comes from explicit global module.
   bool isFromExplicitGlobalModule() const;
 
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 5dd0ba33f8a9c2..9b7e3af0e449b5 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -721,6 +721,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 76e51ac7ab9792..671520a3602b3b 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -790,6 +790,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' 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
@@ -1500,6 +1505,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 a0e475ec9f862c..71a7c28047e318 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -500,6 +500,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.
@@ -857,6 +861,8 @@ class ASTWriter : public ASTDeserializationListener,
     return PredefinedDecls.count(D);
   }
 
+  void handleVTable(CXXRecordDecl *RD);
+
 private:
   // ASTDeserializationListener implementation
   void ReaderInitialized(ASTReader *Reader) override;
@@ -951,6 +957,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/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 7af9ea7105bb08..3da5e888f25175 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12405,8 +12405,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
       !isMSStaticDataMemberInlineDefinition(VD))
     return false;
 
-  // Variables in other module units shouldn't be forced to be emitted.
-  if (VD->isInAnotherModuleUnit())
+  if (VD->shouldEmitInExternalSource())
     return false;
 
   // Variables that can be needed in other TUs are required.
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index bc5a9206c0db28..b59f118380ca4b 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1125,20 +1125,36 @@ bool Decl::isInAnotherModuleUnit() const {
   if (!M)
     return false;
 
+  // FIXME or NOTE: maybe we need to be clear about the semantics
+  // of clang header modules. e.g., if this lives in a clang header
+  // module included by the current unit, should we return false
+  // here?
+  //
+  // This is clear for header units as the specification says the
+  // header units live in a synthesised translation unit. So we
+  // can return false here.
   M = M->getTopLevelModule();
-  // FIXME: It is problematic if the header module lives in another module
-  // unit. Consider to fix this by techniques like
-  // ExternalASTSource::hasExternalDefinitions.
-  if (M->isHeaderLikeModule())
+  if (!M->isNamedModule())
     return false;
 
-  // A global module without parent implies that we're parsing the global
-  // module. So it can't be in another module unit.
-  if (M->isGlobalModule())
+  return M != getASTContext().getCurrentNamedModule();
+}
+
+bool Decl::isInCurrentModuleUnit() const {
+  auto *M = getOwningModule();
+
+  if (!M || !M->isNamedModule())
     return false;
 
-  assert(M->isNamedModule() && "New module kind?");
-  return M != getASTContext().getCurrentNamedModule();
+  return M == getASTContext().getCurrentNamedModule();
+}
+
+bool Decl::shouldEmitInExternalSource() const {
+  ExternalASTSource *Source = getASTContext().getExternalSource();
+  if (!Source)
+    return false;
+
+  return Source->hasExternalDefinitions(this) == ExternalASTSource::EK_Always;
 }
 
 bool Decl::isFromExplicitGlobalModule() const {
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 7f729d359b82b3..267bdf09829700 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1078,29 +1078,41 @@ llvm::GlobalVariable::LinkageTypes
 CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
   if (!RD->isExternallyVisible())
     return llvm::GlobalVariable::InternalLinkage;
-
-  // We're at the end of the translation unit, so the current key
-  // function is fully correct.
-  const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
-  if (keyFunction && !RD->hasAttr<DLLImportAttr>()) {
+  
+  // In windows, the linkage of vtable is not related to modules.
+  bool IsInNamedModule = !getTarget().getCXXABI().isMicrosoft() &&
+        RD->isInNamedModule();
+  // If the CXXRecordDecl is not in a module unit, we need to get
+  // its key function. We're at the end of the translation unit, so the current
+  // key function is fully correct.
+  const CXXMethodDecl *keyFunction =
+      IsInNamedModule ? nullptr : Context.getCurrentKeyFunction(RD);
+  if (IsInNamedModule || (keyFunction && !RD->hasAttr<DLLImportAttr>())) {
     // If this class has a key function, use that to determine the
     // linkage of the vtable.
     const FunctionDecl *def = nullptr;
-    if (keyFunction->hasBody(def))
+    if (keyFunction && keyFunction->hasBody(def))
       keyFunction = cast<CXXMethodDecl>(def);
 
-    switch (keyFunction->getTemplateSpecializationKind()) {
-      case TSK_Undeclared:
-      case TSK_ExplicitSpecialization:
+    bool IsExternalDefinition =
+        IsInNamedModule ? RD->shouldEmitInExternalSource() : !def;
+
+    TemplateSpecializationKind Kind =
+        IsInNamedModule ? RD->getTemplateSpecializationKind()
+                        : keyFunction->getTemplateSpecializationKind();
+
+    switch (Kind) {
+    case TSK_Undeclared:
+    case TSK_ExplicitSpecialization:
       assert(
-          (def || CodeGenOpts.OptimizationLevel > 0 ||
+          (IsInNamedModule || def || CodeGenOpts.OptimizationLevel > 0 ||
            CodeGenOpts.getDebugInfo() != llvm::codegenoptions::NoDebugInfo) &&
-          "Shouldn't query vtable linkage without key function, "
-          "optimizations, or debug info");
-      if (!def && CodeGenOpts.OptimizationLevel > 0)
+          "Shouldn't query vtable linkage without the class in module units, "
+          "key function, optimizations, or debug info");
+      if (IsExternalDefinition && CodeGenOpts.OptimizationLevel > 0)
         return llvm::GlobalVariable::AvailableExternallyLinkage;
 
-      if (keyFunction->isInlined())
+      if (keyFunction && keyFunction->isInlined())
         return !Context.getLangOpts().AppleKext
                    ? llvm::GlobalVariable::LinkOnceODRLinkage
                    : llvm::Function::InternalLinkage;
@@ -1119,7 +1131,7 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
 
       case TSK_ExplicitInstantiationDeclaration:
         llvm_unreachable("Should not have been asked to emit this");
-    }
+      }
   }
 
   // -fapple-kext mode does not support weak linkage, so we must use
@@ -1213,22 +1225,20 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) {
       TSK == TSK_ExplicitInstantiationDefinition)
     return false;
 
+  // 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.
+  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);
   if (!keyFunction)
     return false;
 
-  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();
 }
 
 /// 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 cd76f8406e7b72..0be92fb2e27579 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2315,6 +2315,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()) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fa4bfe13053d8e..d608dd92a4b479 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18073,6 +18073,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 where 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 92c47be67339e9..4e4f91de8cd5a5 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18517,11 +18517,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 =
@@ -18566,7 +18570,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/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 3cb96df12e4da0..29aec144aec1a8 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3921,6 +3921,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(ReadDeclID(F, Record, I));
+      break;
+
     case IMPORTED_MODULES:
       if (!F.isModule()) {
         // If we aren't loading a module (which has its own exports), make
@@ -8110,6 +8117,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 ccc97f65526d6c..c118f3818467d9 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4242,6 +4242,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) {
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index c78d8943d6d92e..7c0636962459e9 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);
@@ -3961,6 +3962,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
 //===----------------------------------------------------------------------===//
@@ -5163,6 +5168,13 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) {
   // Write all of the DeclsToCheckForDeferredDiags.
   for (auto *D : SemaRef.DeclsToCheckForDeferredDiags)
     GetDeclRef(D);
+
+  // Write all classes that need to emit the vtable definitions if required.
+  if (isWritingStdCXXNamedModules())
+    for (CXXRecordDecl *RD : PendingEmittingVTables)
+      GetDeclRef(RD);
+  else
+    PendingEmittingVTables.clear();
 }
 
 void ASTWriter::WriteSpecialDeclRecords(Sema &SemaRef) {
@@ -5317,6 +5329,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,
@@ -6559,10 +6582,12 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   // computed.
   Record->push_back(D->getODRHash());
 
-  bool ModulesDebugInfo =
-      Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
-  Record->push_back(ModulesDebugInfo);
-  if (ModulesDebugInfo)
+  bool ModulesCodegen =
+      !D->isDependentType() &&
+     (Writer->Context->getLangOpts().ModulesDebugInfo ||
+      D->isInNamedModule());
+  Record->push_back(ModulesCodegen);
+  if (ModulesCodegen)
     Writer->AddDeclRef(D, Writer->ModularCodegenDecls);
 
   // IsLambda bit is already saved.
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 17c774038571ef..8a4ca54349e38f 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1529,8 +1529,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 in that case the key function is meaningless.
   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 fb179b1de4880b..5cc3504d72628f 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 00000000000000..ad4e13589d86e4
--- /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
diff --git a/clang/test/Modules/pr97313.cppm b/clang/test/Modules/pr97313.cppm
new file mode 100644
index 00000000000000..ebbd0ee4e2c655
--- /dev/null
+++ b/clang/test/Modules/pr97313.cppm
@@ -0,0 +1,118 @@
+// REQUIRES: !system-windows
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Base.cppm \
+// RUN:     -emit-module-interface -o %t/Base.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Sub.cppm \
+// RUN:     -emit-module-interface -o %t/Sub.pcm -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Sub.pcm \
+// RUN:     -emit-llvm -o %t/Sub.pcm -o - -fprebuilt-module-path=%t | \
+// RUN:     FileCheck %t/Sub.cppm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/main.cpp \
+// RUN:     -emit-llvm -fprebuilt-module-path=%t -o - | FileCheck %t/main.cpp
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Mod.cppm \
+// RUN:     -emit-module-interface -o %t/Mod.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Mod.pcm \
+// RUN:     -emit-llvm -o - | FileCheck %t/Mod.cppm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Use.cpp \
+// RUN:     -emit-llvm -fprebuilt-module-path=%t -o - | \
+// RUN:     FileCheck %t/Use.cpp
+
+//--- Base.cppm
+export module Base;
+
+export template <class>
+class Base
+{
+public:
+    constexpr Base();
+    constexpr virtual ~Base();
+};
+
+template <class X>
+constexpr Base<X>::Base() = default;
+
+template <class X>
+constexpr Base<X>::~Base() = default;
+
+//--- Sub.cppm
+export module Sub;
+export import Base;
+
+export class Sub : public Base<int>
+{
+};
+
+// CHECK: @_ZTIW4Base4BaseIiE = {{.*}}linkonce_odr
+
+//--- main.cpp
+import Sub;
+
+int main()
+{
+    Base<int> *b = new Sub();
+    delete b;
+}
+
+// CHECK: @_ZTIW4Base4BaseIiE = {{.*}}linkonce_odr
+
+//--- Mod.cppm
+export module Mod;
+
+export class NonTemplate {
+public:
+    virtual ~NonTemplate();
+};
+
+// CHECK: @_ZTIW3Mod11NonTemplate = {{.*}}constant
+
+export template <class C>
+class Template {
+public:
+    virtual ~Template();
+};
+
+export template<>
+class Template<char> {
+public:
+    virtual ~Template();
+};
+
+// CHECK: @_ZTIW3Mod8TemplateIcE = {{.*}}constant
+
+export template class Template<unsigned>;
+
+// CHECK: @_ZTIW3Mod8TemplateIjE = {{.*}}weak_odr
+
+export extern template class Template<double>;
+
+auto v = new Template<signed int>();
+
+// CHECK: @_ZTIW3Mod8TemplateIiE = {{.*}}linkonce_odr
+
+//--- Use.cpp
+import Mod;
+
+auto v1 = new NonTemplate();
+auto v2 = new Template<char>();
+auto v3 = new Template<unsigned>();
+auto v4 = new Template<double>();
+auto v5 = new Template<signed int>();
+auto v6 = new Template<NonTemplate>();
+
+// CHECK: @_ZTVW3Mod11NonTemplate = {{.*}}external
+// CHECK: @_ZTVW3Mod8TemplateIcE = {{.*}}external
+// CHECK: @_ZTVW3Mod8TemplateIjE = {{.*}}weak_odr
+// CHECK: @_ZTSW3Mod8TemplateIjE = {{.*}}weak_odr
+// CHECK: @_ZTIW3Mod8TemplateIjE = {{.*}}weak_odr
+// CHECK: @_ZTVW3Mod8TemplateIdE = {{.*}}external
+// CHECK: @_ZTVW3Mod8TemplateIiE = {{.*}}linkonce_odr
+// CHECK: @_ZTSW3Mod8TemplateIiE = {{.*}}linkonce_odr
+// CHECK: @_ZTIW3Mod8TemplateIiE = {{.*}}linkonce_odr
+// CHECK: @_ZTVW3Mod8TemplateIS_11NonTemplateE = {{.*}}linkonce_odr
+// CHECK: @_ZTSW3Mod8TemplateIS_11NonTemplateE = {{.*}}linkonce_odr
+// CHECK: @_ZTIW3Mod8TemplateIS_11NonTemplateE = {{.*}}linkonce_odr
diff --git a/clang/test/Modules/static-func-in-private.cppm b/clang/test/Modules/static-func-in-private.cppm
new file mode 100644
index 00000000000000..d7ce663f16d525
--- /dev/null
+++ b/clang/test/Modules/static-func-in-private.cppm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify -fsyntax-only
+// expected-no-diagnostics
+export module a;
+module :private;
+static void f() {}
+void g() {
+  f();
+}
diff --git a/clang/test/Modules/vtable-windows.cppm b/clang/test/Modules/vtable-windows.cppm
new file mode 100644
index 00000000000000..dbde24c8a9bdd2
--- /dev/null
+++ b/clang/test/Modules/vtable-windows.cppm
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple i686-pc-windows-msvc %t/foo.cppm -emit-module-interface \
+// RUN:   -o %t/foo.pcm
+// RUN: %clang_cc1 -std=c++20 -triple i686-pc-windows-msvc %t/user.cc -fmodule-file=foo=%t/foo.pcm \
+// RUN:   -emit-llvm -o - -disable-llvm-passes | FileCheck %t/user.cc
+
+//--- foo.cppm
+export module foo;
+export struct Fruit {
+    virtual ~Fruit() = default;
+    virtual void eval();
+};
+
+//--- user.cc
+import foo;
+void test() {
+  Fruit *f = new Fruit();
+  f->eval();
+}
+
+// Check that the virtual table is an unnamed_addr constant in comdat that can
+// be merged with the virtual table with other TUs.
+// CHECK: unnamed_addr constant {{.*}}[ptr @"??_R4Fruit@@6B@", ptr @"??_GFruit@@UAEPAXI at Z", ptr @"?eval at Fruit@@UAEXXZ"{{.*}}comdat($"??_7Fruit@@6B@")



More information about the llvm-branch-commits mailing list