Modular Codegen: Add/use a bit in serialized function definitions to track whether...

Some decls are created not where they are written, but in other module
files/users (implicit special members and function template implicit
specializations). To correctly identify them, use a bit next to the definition
to track the modular codegen property.

Discussed whether the module file bit could be omitted in favor of
reconstituting from the modular codegen decls list - best guess today is that
the efficiency improvement of not having to deserialize the whole list whenever
any function is queried by a module user is worth it for the small size
increase of this redundant (list + bit-on-def) representation.

Reviewers: rsmith

Differential Revision: https://reviews.llvm.org/D29901

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@299982 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h
index f83c515b1c2a0594acb573d833f29cf8abd42120..474cf2c0e3f32600229f473f00dbe49c5e73aede 100644 (file)
--- a/include/clang/AST/ASTContext.h
+++ b/include/clang/AST/ASTContext.h
@@ -2510,7 +2510,7 @@ public:
   ///
   /// \returns true if the function/var must be CodeGen'ed/deserialized even if
   /// it is not used.
-  bool DeclMustBeEmitted(const Decl *D, bool ForModularCodegen = false);
+  bool DeclMustBeEmitted(const Decl *D);
 
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD);

diff --git a/include/clang/AST/ExternalASTSource.h b/include/clang/AST/ExternalASTSource.h
index 40c54b2e8d6a1baa9fcdbcada6e33a0e8e4851fd..f2b29cd9a73616d971d02eb5cb5a7c649f4964a8 100644 (file)
--- a/include/clang/AST/ExternalASTSource.h
+++ b/include/clang/AST/ExternalASTSource.h
@@ -172,7 +172,7 @@ public:
 
   enum ExtKind { EK_Always, EK_Never, EK_ReplyHazy };
 
-  virtual ExtKind hasExternalDefinitions(unsigned ID);
+  virtual ExtKind hasExternalDefinitions(const FunctionDecl *FD);
 
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.

diff --git a/include/clang/Basic/Module.h b/include/clang/Basic/Module.h
index 51ad1090887ef76d568da68434d0481d332bfe16..8fcb0e8b056a9bbf8ba16a2710fba16dfec21b6c 100644 (file)
--- a/include/clang/Basic/Module.h
+++ b/include/clang/Basic/Module.h
@@ -215,8 +215,6 @@ public:
   /// and headers from used modules.
   unsigned NoUndeclaredIncludes : 1;
 
-  unsigned WithCodegen : 1;
-
   /// \brief Describes the visibility of the various names within a
   /// particular module.
   enum NameVisibilityKind {

diff --git a/include/clang/Sema/MultiplexExternalSemaSource.h b/include/clang/Sema/MultiplexExternalSemaSource.h
index 93e83dc026d96c17c3b5c67ff4cdf46b9369af51..0386552dbe7e3a1326d9adfd617a2c3049466739 100644 (file)
--- a/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -90,7 +90,7 @@ public:
   /// initializers themselves.
   CXXCtorInitializer **GetExternalCXXCtorInitializers(uint64_t Offset) override;
 
-  ExtKind hasExternalDefinitions(unsigned ID) override;
+  ExtKind hasExternalDefinitions(const FunctionDecl *FD) override;
 
   /// \brief Find all declarations with the given name in the
   /// given context.

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index e955ef9565f25477bb5e3df86e630474d3375c09..4e1c1cf57da244334e6866027269a39a75dc6403 100644 (file)
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -1115,6 +1115,8 @@ private:
   /// predefines buffer may contain additional definitions.
   std::string SuggestedPredefines;
 
+  llvm::DenseMap<const FunctionDecl *, bool> BodySource;
+
   /// \brief Reads a statement from the specified cursor.
   Stmt *ReadStmtFromStream(ModuleFile &F);
 
@@ -1997,7 +1999,7 @@ public:
   /// \brief Return a descriptor for the corresponding module.
   llvm::Optional<ASTSourceDescriptor> getSourceDescriptor(unsigned ID) override;
 
-  ExtKind hasExternalDefinitions(unsigned ID) override;
+  ExtKind hasExternalDefinitions(const FunctionDecl *FD) override;
 
   /// \brief Retrieve a selector from the given module with its local ID
   /// number.

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 0c069cc80b117f0f458a58a2a2f519f1e3908ae6..7b337b061a031eb1ce9add4d37a3003f2222ca69 100644 (file)
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -8897,7 +8897,7 @@ GVALinkage ASTContext::GetGVALinkageForFunction(const FunctionDecl *FD) const {
       *this, basicGVALinkageForFunction(*this, FD), FD);
   auto EK = ExternalASTSource::EK_ReplyHazy;
   if (auto *Ext = getExternalSource())
-    EK = Ext->hasExternalDefinitions(FD->getOwningModuleID());
+    EK = Ext->hasExternalDefinitions(FD);
   switch (EK) {
   case ExternalASTSource::EK_Never:
     if (L == GVA_DiscardableODR)
@@ -8993,7 +8993,7 @@ GVALinkage ASTContext::GetGVALinkageForVariable(const VarDecl *VD) {
       *this, basicGVALinkageForVariable(*this, VD), VD);
 }
 
-bool ASTContext::DeclMustBeEmitted(const Decl *D, bool ForModularCodegen) {
+bool ASTContext::DeclMustBeEmitted(const Decl *D) {
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
     if (!VD->isFileVarDecl())
       return false;
@@ -9059,9 +9059,6 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D, bool ForModularCodegen) {
 
     GVALinkage Linkage = GetGVALinkageForFunction(FD);
 
-    if (Linkage == GVA_DiscardableODR && ForModularCodegen)
-      return true;
-
     // static, static inline, always_inline, and extern inline functions can
     // always be deferred.  Normal inline functions can be deferred in C99/C++.
     // Implicit template instantiations can also be deferred in C++.

diff --git a/lib/AST/ExternalASTSource.cpp b/lib/AST/ExternalASTSource.cpp
index 26b1aef0a8fdf6e76b174c6cede10c9e0fe1d0f8..958a67843b9bc138127e91d9d439414d381f0928 100644 (file)
--- a/lib/AST/ExternalASTSource.cpp
+++ b/lib/AST/ExternalASTSource.cpp
@@ -29,7 +29,7 @@ ExternalASTSource::getSourceDescriptor(unsigned ID) {
 }
 
 ExternalASTSource::ExtKind
-ExternalASTSource::hasExternalDefinitions(unsigned ID) {
+ExternalASTSource::hasExternalDefinitions(const FunctionDecl *FD) {
   return EK_ReplyHazy;
 }
 

diff --git a/lib/Basic/Module.cpp b/lib/Basic/Module.cpp
index ad814fda9abb5c91d0dd29226a5443df52ff3fce..a6fd931cb174c2871cbcf30fa56e846dc6c29840 100644 (file)
--- a/lib/Basic/Module.cpp
+++ b/lib/Basic/Module.cpp
@@ -33,7 +33,7 @@ Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
       IsExplicit(IsExplicit), IsSystem(false), IsExternC(false),
       IsInferred(false), InferSubmodules(false), InferExplicitSubmodules(false),
       InferExportWildcard(false), ConfigMacrosExhaustive(false),
-      NoUndeclaredIncludes(false), WithCodegen(false), NameVisibility(Hidden) {
+      NoUndeclaredIncludes(false), NameVisibility(Hidden) {
   if (Parent) {
     if (!Parent->isAvailable())
       IsAvailable = false;

diff --git a/lib/Sema/MultiplexExternalSemaSource.cpp b/lib/Sema/MultiplexExternalSemaSource.cpp
index c97e4dfdd6d3aca936aa19d7fd106d4d37bcfc49..f749b7d139450cc8ceee68aee3d212adef96119b 100644 (file)
--- a/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -95,9 +95,9 @@ MultiplexExternalSemaSource::GetExternalCXXCtorInitializers(uint64_t Offset) {
 }
 
 ExternalASTSource::ExtKind
-MultiplexExternalSemaSource::hasExternalDefinitions(unsigned int ID) {
+MultiplexExternalSemaSource::hasExternalDefinitions(const FunctionDecl *FD) {
   for (const auto &S : Sources)
-    if (auto EK = S->hasExternalDefinitions(ID))
+    if (auto EK = S->hasExternalDefinitions(FD))
       if (EK != EK_ReplyHazy)
         return EK;
   return EK_ReplyHazy;

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 9ef1378972efb12fda6fb446bf030be247232268..a51f144ae069ed1cbec6ae8f8c9eea176f47a129 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -4834,7 +4834,6 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
       bool InferExplicitSubmodules = Record[Idx++];
       bool InferExportWildcard = Record[Idx++];
       bool ConfigMacrosExhaustive = Record[Idx++];
-      bool WithCodegen = Record[Idx++];
 
       Module *ParentModule = nullptr;
       if (Parent)
@@ -4880,7 +4879,6 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
       CurrentModule->InferExplicitSubmodules = InferExplicitSubmodules;
       CurrentModule->InferExportWildcard = InferExportWildcard;
       CurrentModule->ConfigMacrosExhaustive = ConfigMacrosExhaustive;
-      CurrentModule->WithCodegen = WithCodegen;
       if (DeserializationListener)
         DeserializationListener->ModuleRead(GlobalID, CurrentModule);
 
@@ -8149,16 +8147,12 @@ ASTReader::getSourceDescriptor(unsigned ID) {
   return None;
 }
 
-ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(unsigned ID) {
-  const Module *M = getSubmodule(ID);
-  if (!M || !M->WithCodegen)
+ExternalASTSource::ExtKind
+ASTReader::hasExternalDefinitions(const FunctionDecl *FD) {
+  auto I = BodySource.find(FD);
+  if (I == BodySource.end())
     return EK_ReplyHazy;
-
-  ModuleFile *MF = ModuleMgr.lookup(M->getASTFile());
-  assert(MF); // ?
-  if (MF->Kind == ModuleKind::MK_MainFile)
-    return EK_Never;
-  return EK_Always;
+  return I->second ? EK_Never : EK_Always;
 }
 
 Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
@@ -8992,9 +8986,9 @@ void ASTReader::finishPendingActions() {
       // FIXME: Check for =delete/=default?
       // FIXME: Complain about ODR violations here?
       const FunctionDecl *Defn = nullptr;
-      if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn))
+      if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn)) {
         FD->setLazyBody(PB->second);
-      else
+      } else
         mergeDefinitionVisibility(const_cast<FunctionDecl*>(Defn), FD);
       continue;
     }

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 5cd59177d7d1f1e40162e956cfd4944500c4c080..9b9b41a104f1258102723741c56c7d81761a58bd 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -424,6 +424,8 @@ uint64_t ASTDeclReader::GetCurrentCursorOffset() {
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
+  if (Record.readInt())
+    Reader.BodySource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
   if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     CD->NumCtorInitializers = Record.readInt();
     if (CD->NumCtorInitializers)

diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 9b479c24e203c2b116954f3056bdf5eb4d9fb249..23859c241d5280b916660cc6f29643c31164e915 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -2627,7 +2627,6 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExplicit...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExportWild...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ConfigMacrosExh...
-  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // WithCodegen
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
   unsigned DefinitionAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
 
@@ -2726,8 +2725,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
                                          Mod->InferSubmodules,
                                          Mod->InferExplicitSubmodules,
                                          Mod->InferExportWildcard,
-                                         Mod->ConfigMacrosExhaustive,
-                                         Context->getLangOpts().ModularCodegen && WritingModule};
+                                         Mod->ConfigMacrosExhaustive};
       Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name);
     }
 

diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp
index 41b9a97c78f6aa1a1ba0ed2e123adc43f38b72f8..8196c8d02cd28857c7641f55a27f7cc2a7a0422b 100644 (file)
--- a/lib/Serialization/ASTWriterDecl.cpp
+++ b/lib/Serialization/ASTWriterDecl.cpp
@@ -2159,7 +2159,7 @@ void ASTWriter::WriteDeclAbbrevs() {
 /// relatively painless since they would presumably only do it for top-level
 /// decls.
 static bool isRequiredDecl(const Decl *D, ASTContext &Context,
-                           bool WritingModule, bool ModularCode) {
+                           bool WritingModule) {
   // An ObjCMethodDecl is never considered as "required" because its
   // implementation container always is.
 
@@ -2175,7 +2175,7 @@ static bool isRequiredDecl(const Decl *D, ASTContext &Context,
     return false;
   }
 
-  return Context.DeclMustBeEmitted(D, ModularCode);
+  return Context.DeclMustBeEmitted(D);
 }
 
 void ASTWriter::WriteDecl(ASTContext &Context, Decl *D) {
@@ -2219,11 +2219,8 @@ void ASTWriter::WriteDecl(ASTContext &Context, Decl *D) {
 
   // Note declarations that should be deserialized eagerly so that we can add
   // them to a record in the AST file later.
-  if (isRequiredDecl(D, Context, WritingModule, false))
+  if (isRequiredDecl(D, Context, WritingModule))
     EagerlyDeserializedDecls.push_back(ID);
-  else if (Context.getLangOpts().ModularCodegen && WritingModule &&
-           isRequiredDecl(D, Context, true, true))
-    ModularCodegenDecls.push_back(ID);
 }
 
 void ASTRecordWriter::AddFunctionDefinition(const FunctionDecl *FD) {
@@ -2231,6 +2228,11 @@ void ASTRecordWriter::AddFunctionDefinition(const FunctionDecl *FD) {
   Writer->ClearSwitchCaseIDs();
 
   assert(FD->doesThisDeclarationHaveABody());
+  bool ModularCodegen = Writer->Context->getLangOpts().ModularCodegen &&
+                        Writer->WritingModule && !FD->isDependentContext();
+  Record->push_back(ModularCodegen);
+  if (ModularCodegen)
+    Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(FD));
   if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     Record->push_back(CD->getNumCtorInitializers());
     if (CD->getNumCtorInitializers())

diff --git a/test/Modules/Inputs/codegen-nodep/foo.h b/test/Modules/Inputs/codegen-nodep/foo.h
new file mode 100644 (file)
index 0000000..af91e8d
--- /dev/null
+++ b/test/Modules/Inputs/codegen-nodep/foo.h
@@ -0,0 +1,5 @@
+template <typename T>
+void ftempl() {
+}
+inline void f() {
+}

diff --git a/test/Modules/Inputs/codegen-nodep/foo.modulemap b/test/Modules/Inputs/codegen-nodep/foo.modulemap
new file mode 100644 (file)
index 0000000..2e095d2
--- /dev/null
+++ b/test/Modules/Inputs/codegen-nodep/foo.modulemap
@@ -0,0 +1 @@
+module foo { header "foo.h" }

diff --git a/test/Modules/Inputs/codegen/foo.h b/test/Modules/Inputs/codegen/foo.h
index 3fcab7185731b9aa07d258f948573a2cf95ea496..e77e8d1824e5105de182c8c353a9b06f201733e4 100644 (file)
--- a/test/Modules/Inputs/codegen/foo.h
+++ b/test/Modules/Inputs/codegen/foo.h
@@ -2,3 +2,29 @@ inline void f1(const char* fmt, ...) {
   __builtin_va_list args;
   __builtin_va_start(args, fmt);
 }
+
+struct non_trivial_dtor {
+  ~non_trivial_dtor();
+};
+
+struct implicit_dtor {
+  non_trivial_dtor d;
+};
+
+struct uninst_implicit_dtor {
+  non_trivial_dtor d;
+};
+
+inline void use_implicit_dtor() {
+  implicit_dtor d;
+}
+
+template <typename T>
+void inst() {
+}
+
+inline void inst_decl() {
+  // cause inst<int>'s declaration to be instantiated, without a definition.
+  (void)sizeof(&inst<int>);
+  inst<float>();
+}

diff --git a/test/Modules/Inputs/codegen/use.cpp b/test/Modules/Inputs/codegen/use.cpp
new file mode 100644 (file)
index 0000000..cd1a4a6
--- /dev/null
+++ b/test/Modules/Inputs/codegen/use.cpp
@@ -0,0 +1,8 @@
+#include "foo.h"
+void non_modular_use_of_implicit_dtor() {
+  implicit_dtor d1;
+  uninst_implicit_dtor d2;
+}
+void use_of_instantiated_declaration_without_definition() {
+  inst<int>();
+}

diff --git a/test/Modules/codegen-nodep.test b/test/Modules/codegen-nodep.test
new file mode 100644 (file)
index 0000000..cb2b4e3
--- /dev/null
+++ b/test/Modules/codegen-nodep.test
@@ -0,0 +1,13 @@
+RUN: rm -rf %t
+REQUIRES: x86-registered-target
+
+RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++ -fmodules \
+RUN:            -emit-module -fmodule-name=foo \
+RUN:            %S/Inputs/codegen-nodep/foo.modulemap -o - \
+RUN:          | llvm-bcanalyzer - -dump \
+RUN:          | FileCheck %s
+
+Ensure there's only one modular codegen decl - the sentinel plain inline
+function, not any for the function template.
+
+CHECK: <MODULAR_CODEGEN_DECLS op0={{[0-9]+}}/>

diff --git a/test/Modules/codegen.test b/test/Modules/codegen.test
index f1823d55bafc94c5eeb84b1560081604c15e4ed2..6807640e603d653f792d0265cbbc3bff7ee0ac48 100644 (file)
--- a/test/Modules/codegen.test
+++ b/test/Modules/codegen.test
@@ -3,8 +3,25 @@ REQUIRES: x86-registered-target
 
 RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++ -fmodules -emit-module -fmodule-name=foo %S/Inputs/codegen/foo.modulemap -o %t/foo.pcm
 
-RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %t/foo.pcm | FileCheck %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %t/foo.pcm | FileCheck --check-prefix=FOO --check-prefix=BOTH %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - -fmodules -fmodule-file=%t/foo.pcm %S/Inputs/codegen/use.cpp | FileCheck --check-prefix=BOTH --check-prefix=USE %s
 
-CHECK: $_Z2f1PKcz = comdat any
-CHECK: define weak_odr void @_Z2f1PKcz(i8* %fmt, ...) #{{[0-9]+}} comdat
-CHECK:   call void @llvm.va_start(i8* %{{[a-zA-Z0-9]*}})
+FOO: $_Z2f1PKcz = comdat any
+FOO: $_ZN13implicit_dtorD1Ev = comdat any
+USE: $_Z4instIiEvv = comdat any
+FOO: $_ZN13implicit_dtorD2Ev = comdat any
+FOO: define weak_odr void @_Z2f1PKcz(i8* %fmt, ...) #{{[0-9]+}} comdat
+FOO:   call void @llvm.va_start(i8* %{{[a-zA-Z0-9]*}})
+
+Test that implicit special members are emitted into the FOO module if they're
+ODR used there, otherwise emit them linkonce_odr as usual in the use.
+
+FIXME: Proactively instantiate any valid implicit special members to emit them into the module object.
+
+FOO: define weak_odr void @_ZN13implicit_dtorD1Ev
+FOO: define weak_odr void @_Z4instIfEvv
+FOO: define weak_odr void @_ZN13implicit_dtorD2Ev
+
+USE: define linkonce_odr void @_ZN20uninst_implicit_dtorD1Ev
+USE: define linkonce_odr void @_Z4instIiEvv
+USE: define linkonce_odr void @_ZN20uninst_implicit_dtorD2Ev