From 94c1c6bb447e920a3e82b24347115b7f855994b2 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 11 Apr 2017 20:46:34 +0000 Subject: [PATCH] Modular Codegen: Add/use a bit in serialized function definitions to track whether they are the subject of modular codegen 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 --- include/clang/AST/ASTContext.h | 2 +- include/clang/AST/ExternalASTSource.h | 2 +- include/clang/Basic/Module.h | 2 -- .../clang/Sema/MultiplexExternalSemaSource.h | 2 +- include/clang/Serialization/ASTReader.h | 4 ++- lib/AST/ASTContext.cpp | 7 ++--- lib/AST/ExternalASTSource.cpp | 2 +- lib/Basic/Module.cpp | 2 +- lib/Sema/MultiplexExternalSemaSource.cpp | 4 +-- lib/Serialization/ASTReader.cpp | 20 +++++--------- lib/Serialization/ASTReaderDecl.cpp | 2 ++ lib/Serialization/ASTWriter.cpp | 4 +-- lib/Serialization/ASTWriterDecl.cpp | 14 +++++----- test/Modules/Inputs/codegen-nodep/foo.h | 5 ++++ .../Inputs/codegen-nodep/foo.modulemap | 1 + test/Modules/Inputs/codegen/foo.h | 26 +++++++++++++++++++ test/Modules/Inputs/codegen/use.cpp | 8 ++++++ test/Modules/codegen-nodep.test | 13 ++++++++++ test/Modules/codegen.test | 25 +++++++++++++++--- 19 files changed, 104 insertions(+), 41 deletions(-) create mode 100644 test/Modules/Inputs/codegen-nodep/foo.h create mode 100644 test/Modules/Inputs/codegen-nodep/foo.modulemap create mode 100644 test/Modules/Inputs/codegen/use.cpp create mode 100644 test/Modules/codegen-nodep.test diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h index f83c515b1c..474cf2c0e3 100644 --- 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 40c54b2e8d..f2b29cd9a7 100644 --- 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 51ad109088..8fcb0e8b05 100644 --- 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 93e83dc026..0386552dbe 100644 --- 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 e955ef9565..4e1c1cf57d 100644 --- 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 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 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 0c069cc80b..7b337b061a 100644 --- 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(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 26b1aef0a8..958a67843b 100644 --- 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 ad814fda9a..a6fd931cb1 100644 --- 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 c97e4dfdd6..f749b7d139 100644 --- 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 9ef1378972..a51f144ae0 100644 --- 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(Defn), FD); continue; } diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 5cd59177d7..9b9b41a104 100644 --- 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(FD)) { CD->NumCtorInitializers = Record.readInt(); if (CD->NumCtorInitializers) diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 9b479c24e2..23859c241d 100644 --- 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 41b9a97c78..8196c8d02c 100644 --- 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(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 index 0000000000..af91e8d263 --- /dev/null +++ b/test/Modules/Inputs/codegen-nodep/foo.h @@ -0,0 +1,5 @@ +template +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 index 0000000000..2e095d2794 --- /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 3fcab71857..e77e8d1824 100644 --- 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 +void inst() { +} + +inline void inst_decl() { + // cause inst's declaration to be instantiated, without a definition. + (void)sizeof(&inst); + inst(); +} diff --git a/test/Modules/Inputs/codegen/use.cpp b/test/Modules/Inputs/codegen/use.cpp new file mode 100644 index 0000000000..cd1a4a642d --- /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(); +} diff --git a/test/Modules/codegen-nodep.test b/test/Modules/codegen-nodep.test new file mode 100644 index 0000000000..cb2b4e37e9 --- /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: diff --git a/test/Modules/codegen.test b/test/Modules/codegen.test index f1823d55ba..6807640e60 100644 --- 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 -- 2.40.0