Remove non-affecting module maps from PCM files.

Problem: PCM file includes references to all module maps used in compilation which created PCM. This problem leads to PCM-rebuilds in distributed compilations as some module maps could be missing in isolated compilation. (For example in our distributed build system we create a temp folder for every compilation with only modules and headers that are needed for that particular command). Solution: Add only affecting module map files to a PCM-file. Reviewed By: rsmith Differential Revision: https://reviews.llvm.org/D106876
llvm · Nov 18, 2021 · 8a4fcfc · 8a4fcfc
1 parent af6ee58
commit 8a4fcfc
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 19 deletions.
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
@@ -456,6 +456,9 @@ class ASTWriter : public ASTDeserializationListener,
   std::vector<std::unique_ptr<ModuleFileExtensionWriter>>
       ModuleFileExtensionWriters;
 
+  /// User ModuleMaps skipped when writing control block.
+  std::set<const FileEntry *> SkippedModuleMaps;
+
   /// Retrieve or create a submodule ID for this module.
   unsigned getSubmoduleID(Module *Mod);
 
@@ -475,7 +478,7 @@ class ASTWriter : public ASTDeserializationListener,
   createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
 
   void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts,
-                       bool Modules);
+                       std::set<const FileEntry *> &AffectingModuleMaps);
   void WriteSourceManagerBlock(SourceManager &SourceMgr,
                                const Preprocessor &PP);
   void WritePreprocessor(const Preprocessor &PP, bool IsModule);

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
@@ -161,6 +161,59 @@ static TypeCode getTypeCodeForTypeClass(Type::TypeClass id) {
 
 namespace {
 
+std::set<const FileEntry *> GetAllModuleMaps(const HeaderSearch &HS,
+                                             Module *RootModule) {
+  std::set<const FileEntry *> ModuleMaps{};
+  std::set<Module *> ProcessedModules;
+  SmallVector<Module *> ModulesToProcess{RootModule};
+
+  SmallVector<const FileEntry *, 16> FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+    FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+    const FileEntry *File = FilesByUID[UID];
+    if (!File)
+      continue;
+
+    const HeaderFileInfo *HFI =
+        HS.getExistingFileInfo(File, /*WantExternal*/ false);
+    if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+      continue;
+
+    for (const auto &KH : HS.findAllModulesForHeader(File)) {
+      if (!KH.getModule())
+        continue;
+      ModulesToProcess.push_back(KH.getModule());
+    }
+  }
+
+  while (!ModulesToProcess.empty()) {
+    auto *CurrentModule = ModulesToProcess.pop_back_val();
+    ProcessedModules.insert(CurrentModule);
+
+    auto *ModuleMapFile =
+        HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+    if (!ModuleMapFile) {
+      continue;
+    }
+
+    ModuleMaps.insert(ModuleMapFile);
+
+    for (auto *ImportedModule : (CurrentModule)->Imports) {
+      if (!ImportedModule ||
+          ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+        continue;
+      }
+      ModulesToProcess.push_back(ImportedModule);
+    }
+  }
+
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter &Writer;
   ASTWriter::RecordData Record;
@@ -1424,9 +1477,15 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
     Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set<const FileEntry *> AffectingModuleMaps;
+  if (WritingModule) {
+    AffectingModuleMaps =
+        GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
                   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-                  PP.getLangOpts().Modules);
+                  AffectingModuleMaps);
   Stream.ExitBlock();
 }
 
@@ -1444,9 +1503,9 @@ struct InputFileEntry {
 
 } // namespace
 
-void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
-                                HeaderSearchOptions &HSOpts,
-                                bool Modules) {
+void ASTWriter::WriteInputFiles(
+    SourceManager &SourceMgr, HeaderSearchOptions &HSOpts,
+    std::set<const FileEntry *> &AffectingModuleMaps) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1486,6 +1545,16 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
     if (!Cache->OrigEntry)
       continue;
 
+    if (isModuleMap(File.getFileCharacteristic()) &&
+        !isSystem(File.getFileCharacteristic()) &&
+        !AffectingModuleMaps.empty() &&
+        AffectingModuleMaps.find(Cache->OrigEntry) ==
+            AffectingModuleMaps.end()) {
+      SkippedModuleMaps.insert(Cache->OrigEntry);
+      // Do not emit modulemaps that do not affect current module.
+      continue;
+    }
+
     InputFileEntry Entry;
     Entry.File = Cache->OrigEntry;
     Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -1999,11 +2068,17 @@ void ASTWriter::WriteSourceManagerBlock(SourceManager &SourceMgr,
     Record.push_back(SLoc->getOffset() - 2);
     if (SLoc->isFile()) {
       const SrcMgr::FileInfo &File = SLoc->getFile();
+      const SrcMgr::ContentCache *Content = &File.getContentCache();
+      if (Content->OrigEntry && !SkippedModuleMaps.empty() &&
+          SkippedModuleMaps.find(Content->OrigEntry) !=
+              SkippedModuleMaps.end()) {
+        // Do not emit files that were not listed as inputs.
+        continue;
+      }
       AddSourceLocation(File.getIncludeLoc(), Record);
       Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
       Record.push_back(File.hasLineDirectives());
 
-      const SrcMgr::ContentCache *Content = &File.getContentCache();
       bool EmitBlob = false;
       if (Content->OrigEntry) {
         assert(Content->OrigEntry == Content->ContentsEntry &&

diff --git a/clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap b/clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
diff --git a/clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap b/clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
diff --git a/clang/test/Modules/add-remove-irrelevant-mobile-map.m b/clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
diff --git a/clang/test/SemaCXX/Inputs/compare.modulemap b/clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+    header "std-compare.h"
+  }
+  explicit module other {}
+}
diff --git a/clang/test/SemaCXX/compare-modules-cxx2a.cpp b/clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@ auto va = A() <=> A(); // expected-note {{required here}}
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}