dblaikie created this revision. 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.
Possible improvement would be to store this bit on the Decl itself, rather than a sidemap - happy to give that a whirl if it seems better. (actually it'd be two bits, since 3 states are required - currently reflected by "not in the map", "in the map with Always", "in the map with Never") 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. https://reviews.llvm.org/D29901 Files: include/clang/AST/ExternalASTSource.h include/clang/Sema/MultiplexExternalSemaSource.h include/clang/Serialization/ASTReader.h lib/AST/ASTContext.cpp lib/AST/ExternalASTSource.cpp lib/Sema/MultiplexExternalSemaSource.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/Modules/Inputs/codegen/foo.h test/Modules/Inputs/codegen/use.cpp test/Modules/codegen.test
Index: test/Modules/codegen.test =================================================================== --- test/Modules/codegen.test +++ test/Modules/codegen.test @@ -3,8 +3,23 @@ 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 (like this dtor) are emitted into both module (if they're used there) and user. +FIXME: Proactively instantiate any valid implicit special members to emit them into the module object. + +FOO: define weak_odr void @_ZN13implicit_dtorD1Ev(%struct.implicit_dtor* %this) unnamed_addr #0 comdat align 2 { +FOO: define weak_odr void @_Z4instIfEvv() #0 comdat { +FOO: define weak_odr void @_ZN13implicit_dtorD2Ev(%struct.implicit_dtor* %this) unnamed_addr #0 comdat align 2 { + +USE: define linkonce_odr void @_ZN20uninst_implicit_dtorD1Ev(%struct.uninst_implicit_dtor* %this) unnamed_addr #0 comdat align 2 { +USE: define linkonce_odr void @_Z4instIiEvv() #0 comdat { +USE: define linkonce_odr void @_ZN20uninst_implicit_dtorD2Ev(%struct.uninst_implicit_dtor* %this) unnamed_addr #0 comdat align 2 { Index: test/Modules/Inputs/codegen/use.cpp =================================================================== --- /dev/null +++ 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>(); +} Index: test/Modules/Inputs/codegen/foo.h =================================================================== --- test/Modules/Inputs/codegen/foo.h +++ test/Modules/Inputs/codegen/foo.h @@ -2,3 +2,29 @@ __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>(); +} Index: lib/Serialization/ASTWriterDecl.cpp =================================================================== --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -2224,6 +2224,8 @@ Writer->ClearSwitchCaseIDs(); assert(FD->doesThisDeclarationHaveABody()); + Record->push_back(Writer->Context->getLangOpts().ModularCodegen && + Writer->WritingModule); if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) { Record->push_back(CD->getNumCtorInitializers()); if (CD->getNumCtorInitializers()) Index: lib/Serialization/ASTReaderDecl.cpp =================================================================== --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -423,6 +423,13 @@ } void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) { + if (Record.readInt()) { + Reader.BodySource[FD] = + Reader.getLocalBitOffset(GetCurrentCursorOffset()).F->Kind == + ModuleKind::MK_MainFile + ? ExternalASTSource::EK_Never + : ExternalASTSource::EK_Always; + } if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) { CD->NumCtorInitializers = Record.readInt(); if (CD->NumCtorInitializers) Index: lib/Serialization/ASTReader.cpp =================================================================== --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -7914,16 +7914,11 @@ 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; } Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) { @@ -8754,9 +8749,9 @@ // 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; } Index: lib/Sema/MultiplexExternalSemaSource.cpp =================================================================== --- lib/Sema/MultiplexExternalSemaSource.cpp +++ lib/Sema/MultiplexExternalSemaSource.cpp @@ -95,9 +95,9 @@ } 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; Index: lib/AST/ExternalASTSource.cpp =================================================================== --- lib/AST/ExternalASTSource.cpp +++ lib/AST/ExternalASTSource.cpp @@ -29,7 +29,7 @@ } ExternalASTSource::ExtKind -ExternalASTSource::hasExternalDefinitions(unsigned ID) { +ExternalASTSource::hasExternalDefinitions(const FunctionDecl *FD) { return EK_ReplyHazy; } Index: lib/AST/ASTContext.cpp =================================================================== --- lib/AST/ASTContext.cpp +++ lib/AST/ASTContext.cpp @@ -8886,7 +8886,7 @@ *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) Index: include/clang/Serialization/ASTReader.h =================================================================== --- include/clang/Serialization/ASTReader.h +++ include/clang/Serialization/ASTReader.h @@ -1103,6 +1103,8 @@ /// predefines buffer may contain additional definitions. std::string SuggestedPredefines; + std::map<const FunctionDecl*, ExternalASTSource::ExtKind> BodySource; + /// \brief Reads a statement from the specified cursor. Stmt *ReadStmtFromStream(ModuleFile &F); @@ -1969,7 +1971,7 @@ /// \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. Index: include/clang/Sema/MultiplexExternalSemaSource.h =================================================================== --- include/clang/Sema/MultiplexExternalSemaSource.h +++ include/clang/Sema/MultiplexExternalSemaSource.h @@ -90,7 +90,7 @@ /// 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. Index: include/clang/AST/ExternalASTSource.h =================================================================== --- include/clang/AST/ExternalASTSource.h +++ include/clang/AST/ExternalASTSource.h @@ -172,7 +172,7 @@ 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.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits