From 3e7ae32b9c618b056a827820d8f61d5bca5bdd44 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 24 Aug 2015 21:59:32 +0000 Subject: [PATCH] [modules] Remove unnecessary deserialization of fully-external HeaderFileInfos for all files we've seen in this compilation. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@245881 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Lex/HeaderSearch.h | 11 +++-- include/clang/Lex/ModuleMap.h | 2 +- lib/Lex/HeaderSearch.cpp | 84 +++++++++++++++++++++----------- lib/Lex/ModuleMap.cpp | 9 +++- lib/Serialization/ASTReader.cpp | 6 ++- lib/Serialization/ASTWriter.cpp | 6 +-- 6 files changed, 79 insertions(+), 39 deletions(-) diff --git a/include/clang/Lex/HeaderSearch.h b/include/clang/Lex/HeaderSearch.h index cb61c1436e..60bc3a7394 100644 --- a/include/clang/Lex/HeaderSearch.h +++ b/include/clang/Lex/HeaderSearch.h @@ -570,11 +570,16 @@ public: unsigned header_file_size() const { return FileInfo.size(); } - /// \brief Return the HeaderFileInfo structure for the specified FileEntry. + /// \brief Return the HeaderFileInfo structure for the specified FileEntry, + /// in preparation for updating it in some way. HeaderFileInfo &getFileInfo(const FileEntry *FE); - /// \brief Return the HeaderFileInfo structure for the specified FileEntry. - const HeaderFileInfo *getExistingFileInfo(const FileEntry *FE) const; + /// \brief Return the HeaderFileInfo structure for the specified FileEntry, + /// if it has ever been filled in. + /// \param WantExternal Whether the caller wants purely-external header file + /// info (where \p External is true). + const HeaderFileInfo *getExistingFileInfo(const FileEntry *FE, + bool WantExternal = true) const; // Used by external tools typedef std::vector::const_iterator search_dir_iterator; diff --git a/include/clang/Lex/ModuleMap.h b/include/clang/Lex/ModuleMap.h index a4300553a3..155943e545 100644 --- a/include/clang/Lex/ModuleMap.h +++ b/include/clang/Lex/ModuleMap.h @@ -478,7 +478,7 @@ public: /// \brief Adds this header to the given module. /// \param Role The role of the header wrt the module. void addHeader(Module *Mod, Module::Header Header, - ModuleHeaderRole Role); + ModuleHeaderRole Role, bool Imported = false); /// \brief Marks this header as being excluded from the given module. void excludeHeader(Module *Mod, Module::Header Header); diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp index e1c8fa21a6..dccedea644 100644 --- a/lib/Lex/HeaderSearch.cpp +++ b/lib/Lex/HeaderSearch.cpp @@ -963,21 +963,22 @@ LookupSubframeworkHeader(StringRef Filename, /// header file info (\p HFI) static void mergeHeaderFileInfo(HeaderFileInfo &HFI, const HeaderFileInfo &OtherHFI) { + assert(OtherHFI.External && "expected to merge external HFI"); + HFI.isImport |= OtherHFI.isImport; HFI.isPragmaOnce |= OtherHFI.isPragmaOnce; HFI.isModuleHeader |= OtherHFI.isModuleHeader; HFI.NumIncludes += OtherHFI.NumIncludes; - + if (!HFI.ControllingMacro && !HFI.ControllingMacroID) { HFI.ControllingMacro = OtherHFI.ControllingMacro; HFI.ControllingMacroID = OtherHFI.ControllingMacroID; } - - if (OtherHFI.External) { - HFI.DirInfo = OtherHFI.DirInfo; - HFI.External = OtherHFI.External && (!HFI.IsValid || HFI.External); - HFI.IndexHeaderMapHeader = OtherHFI.IndexHeaderMapHeader; - } + + HFI.DirInfo = OtherHFI.DirInfo; + HFI.External = (!HFI.IsValid || HFI.External); + HFI.IsValid = true; + HFI.IndexHeaderMapHeader = OtherHFI.IndexHeaderMapHeader; if (HFI.Framework.empty()) HFI.Framework = OtherHFI.Framework; @@ -989,42 +990,58 @@ HeaderFileInfo &HeaderSearch::getFileInfo(const FileEntry *FE) { if (FE->getUID() >= FileInfo.size()) FileInfo.resize(FE->getUID() + 1); - HeaderFileInfo &HFI = FileInfo[FE->getUID()]; + HeaderFileInfo *HFI = &FileInfo[FE->getUID()]; // FIXME: Use a generation count to check whether this is really up to date. - if (ExternalSource && !HFI.Resolved) { - HFI.Resolved = true; - mergeHeaderFileInfo(HFI, ExternalSource->GetHeaderFileInfo(FE)); + if (ExternalSource && !HFI->Resolved) { + HFI->Resolved = true; + auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE); + + HFI = &FileInfo[FE->getUID()]; + if (ExternalHFI.External) + mergeHeaderFileInfo(*HFI, ExternalHFI); } - HFI.IsValid = true; + HFI->IsValid = true; // We have local information about this header file, so it's no longer // strictly external. - HFI.External = false; - return HFI; + HFI->External = false; + return *HFI; } const HeaderFileInfo * -HeaderSearch::getExistingFileInfo(const FileEntry *FE) const { +HeaderSearch::getExistingFileInfo(const FileEntry *FE, + bool WantExternal) const { // If we have an external source, ensure we have the latest information. // FIXME: Use a generation count to check whether this is really up to date. - if (ExternalSource && - (FE->getUID() >= FileInfo.size() || !FileInfo[FE->getUID()].Resolved)) { - auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE); - if (ExternalHFI.External) { - if (FE->getUID() >= FileInfo.size()) - FileInfo.resize(FE->getUID() + 1); - mergeHeaderFileInfo(FileInfo[FE->getUID()], ExternalHFI); + HeaderFileInfo *HFI; + if (ExternalSource) { + if (FE->getUID() >= FileInfo.size()) { + if (!WantExternal) + return nullptr; + FileInfo.resize(FE->getUID() + 1); } - } - if (FE->getUID() >= FileInfo.size()) + HFI = &FileInfo[FE->getUID()]; + if (!WantExternal && (!HFI->IsValid || HFI->External)) + return nullptr; + if (!HFI->Resolved) { + HFI->Resolved = true; + auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE); + + HFI = &FileInfo[FE->getUID()]; + if (ExternalHFI.External) + mergeHeaderFileInfo(*HFI, ExternalHFI); + } + } else if (FE->getUID() >= FileInfo.size()) { return nullptr; + } else { + HFI = &FileInfo[FE->getUID()]; + } - HeaderFileInfo &HFI = FileInfo[FE->getUID()]; - if (!HFI.IsValid) + if (!HFI->IsValid || (HFI->External && !WantExternal)) return nullptr; - return &HFI; + return HFI; } bool HeaderSearch::isFileMultipleIncludeGuarded(const FileEntry *File) { @@ -1038,8 +1055,19 @@ bool HeaderSearch::isFileMultipleIncludeGuarded(const FileEntry *File) { void HeaderSearch::MarkFileModuleHeader(const FileEntry *FE, ModuleMap::ModuleHeaderRole Role, bool isCompilingModuleHeader) { + bool isModularHeader = !(Role & ModuleMap::TextualHeader); + + // Don't mark the file info as non-external if there's nothing to change. + if (!isCompilingModuleHeader) { + if (!isModularHeader) + return; + auto *HFI = getExistingFileInfo(FE); + if (HFI && HFI->isModuleHeader) + return; + } + auto &HFI = getFileInfo(FE); - HFI.isModuleHeader |= !(Role & ModuleMap::TextualHeader); + HFI.isModuleHeader |= isModularHeader; HFI.isCompilingModuleHeader |= isCompilingModuleHeader; } diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp index d619b52bef..e1594eee36 100644 --- a/lib/Lex/ModuleMap.cpp +++ b/lib/Lex/ModuleMap.cpp @@ -796,7 +796,7 @@ static Module::HeaderKind headerRoleToKind(ModuleMap::ModuleHeaderRole Role) { } void ModuleMap::addHeader(Module *Mod, Module::Header Header, - ModuleHeaderRole Role) { + ModuleHeaderRole Role, bool Imported) { KnownHeader KH(Mod, Role); // Only add each header to the headers list once. @@ -811,7 +811,12 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header, Mod->Headers[headerRoleToKind(Role)].push_back(std::move(Header)); bool isCompilingModuleHeader = Mod->getTopLevelModule() == CompilingModule; - HeaderInfo.MarkFileModuleHeader(Header.Entry, Role, isCompilingModuleHeader); + if (!Imported || isCompilingModuleHeader) { + // When we import HeaderFileInfo, the external source is expected to + // set the isModuleHeader flag itself. + HeaderInfo.MarkFileModuleHeader(Header.Entry, Role, + isCompilingModuleHeader); + } } void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) { diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 8e19c708f7..7c94ca435d 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -1603,11 +1603,13 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d, // going to use this information to rebuild the module, so it doesn't make // a lot of difference. Module::Header H = { key.Filename, FileMgr.getFile(Filename) }; - ModMap.addHeader(Mod, H, HeaderRole); + ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true); + HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader); } // This HeaderFileInfo was externally loaded. HFI.External = true; + HFI.IsValid = true; return HFI; } @@ -2790,7 +2792,7 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { RemapBuilder DeclRemap(F.DeclRemap); RemapBuilder TypeRemap(F.TypeRemap); - while(Data < DataEnd) { + while (Data < DataEnd) { using namespace llvm::support; uint16_t Len = endian::readNext(Data); StringRef Name = StringRef((const char*)Data, Len); diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 678258eb44..7058ba3eb9 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -1736,9 +1736,9 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) { // changed since it was loaded. Also skip it if it's for a modular header // from a different module; in that case, we rely on the module(s) // containing the header to provide this information. - const HeaderFileInfo *HFI = HS.getExistingFileInfo(File); - if (!HFI || (HFI->External && Chain) || - (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) + const HeaderFileInfo *HFI = + HS.getExistingFileInfo(File, /*WantExternal*/!Chain); + if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) continue; // Massage the file path into an appropriate form. -- 2.40.0