r245881 - [modules] Remove unnecessary deserialization of fully-external HeaderFileInfos for all files we've seen in this compilation.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 24 14:59:32 PDT 2015


Author: rsmith
Date: Mon Aug 24 16:59:32 2015
New Revision: 245881

URL: http://llvm.org/viewvc/llvm-project?rev=245881&view=rev
Log:
[modules] Remove unnecessary deserialization of fully-external HeaderFileInfos for all files we've seen in this compilation.

Modified:
    cfe/trunk/include/clang/Lex/HeaderSearch.h
    cfe/trunk/include/clang/Lex/ModuleMap.h
    cfe/trunk/lib/Lex/HeaderSearch.cpp
    cfe/trunk/lib/Lex/ModuleMap.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=245881&r1=245880&r2=245881&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Mon Aug 24 16:59:32 2015
@@ -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<DirectoryLookup>::const_iterator search_dir_iterator;

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=245881&r1=245880&r2=245881&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Mon Aug 24 16:59:32 2015
@@ -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);

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=245881&r1=245880&r2=245881&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Mon Aug 24 16:59:32 2015
@@ -963,21 +963,22 @@ LookupSubframeworkHeader(StringRef Filen
 /// 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::getFileInf
   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::isFileMultipleInclude
 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;
 }
 

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=245881&r1=245880&r2=245881&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Mon Aug 24 16:59:32 2015
@@ -796,7 +796,7 @@ static Module::HeaderKind headerRoleToKi
 }
 
 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, M
   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) {

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=245881&r1=245880&r2=245881&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Aug 24 16:59:32 2015
@@ -1603,11 +1603,13 @@ HeaderFileInfoTrait::ReadData(internal_k
     // 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, u
       RemapBuilder DeclRemap(F.DeclRemap);
       RemapBuilder TypeRemap(F.TypeRemap);
 
-      while(Data < DataEnd) {
+      while (Data < DataEnd) {
         using namespace llvm::support;
         uint16_t Len = endian::readNext<uint16_t, little, unaligned>(Data);
         StringRef Name = StringRef((const char*)Data, Len);

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=245881&r1=245880&r2=245881&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Mon Aug 24 16:59:32 2015
@@ -1736,9 +1736,9 @@ void ASTWriter::WriteHeaderSearch(const
     // 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.




More information about the cfe-commits mailing list