Skip to content

Commit ff328f2

Browse files
ilya-biryukovilya-biryukov
committed
[ASTWriter] Do not allocate source location space for module maps used only for textual headers
This is a follow up to llvm#112015 and it reduces the duplication of source locations further. We do not need to allocate source location space in the serialized PCMs for module maps used only to find textual headers. Those module maps are never referenced from anywhere in the serialized ASTs and are re-read in other compilations. We do need the InputFile entry for the in the serialized AST because clang-scan-deps relies on it. The previous patch introduced a mechanism to do exactly that.
1 parent 469f9d5 commit ff328f2

File tree

2 files changed

+136
-14
lines changed

2 files changed

+136
-14
lines changed

‎clang/lib/Serialization/ASTWriter.cpp‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,29 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
184184
const SourceManager &SM = PP.getSourceManager();
185185
const ModuleMap &MM = HS.getModuleMap();
186186

187-
llvm::DenseSet<FileID> ModuleMaps;
188-
189-
llvm::DenseSet<const Module *> ProcessedModules;
190-
auto CollectModuleMapsForHierarchy = [&](const Module *M) {
187+
// Module maps used only by textual headers are special. Their FileID is
188+
// non-affecting, but their FileEntry is (i.e. must be written as InputFile).
189+
enum AffectedReason : bool {
190+
ARTextualHeader = 0,
191+
ARImportOrTextualHeader = 1,
192+
};
193+
auto AssignMostImportant = [](AffectedReason &L, AffectedReason R) {
194+
L = std::max(L, R);
195+
};
196+
llvm::DenseMap<FileID, AffectedReason> ModuleMaps;
197+
llvm::DenseMap<const Module *, AffectedReason> ProcessedModules;
198+
auto CollectModuleMapsForHierarchy = [&](const Module *M, AffectedReason Reason) {
191199
M = M->getTopLevelModule();
192200

193-
if (!ProcessedModules.insert(M).second)
201+
// We need to process the header either when it was not present of when we
202+
// previously flagged module map as textual headers and now we found a
203+
// proper import.
204+
if (auto [It, Inserted] = ProcessedModules.insert({M, Reason});
205+
!Inserted && Reason <= It->second) {
194206
return;
207+
} else {
208+
It->second = Reason;
209+
}
195210

196211
std::queue<const Module *> Q;
197212
Q.push(M);
@@ -202,12 +217,12 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
202217
// The containing module map is affecting, because it's being pointed
203218
// into by Module::DefinitionLoc.
204219
if (auto F = MM.getContainingModuleMapFileID(Mod); F.isValid())
205-
ModuleMaps.insert(F);
220+
AssignMostImportant(ModuleMaps[F], Reason);
206221
// For inferred modules, the module map that allowed inferring is not
207222
// related to the virtual containing module map file. It did affect the
208223
// compilation, though.
209224
if (auto UniqF = MM.getModuleMapFileIDForUniquing(Mod); UniqF.isValid())
210-
ModuleMaps.insert(UniqF);
225+
AssignMostImportant(ModuleMaps[UniqF], Reason);
211226

212227
for (auto *SubM : Mod->submodules())
213228
Q.push(SubM);
@@ -216,7 +231,7 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
216231

217232
// Handle all the affecting modules referenced from the root module.
218233

219-
CollectModuleMapsForHierarchy(RootModule);
234+
CollectModuleMapsForHierarchy(RootModule, ARImportOrTextualHeader);
220235

221236
std::queue<const Module *> Q;
222237
Q.push(RootModule);
@@ -225,9 +240,9 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
225240
Q.pop();
226241

227242
for (const Module *ImportedModule : CurrentModule->Imports)
228-
CollectModuleMapsForHierarchy(ImportedModule);
243+
CollectModuleMapsForHierarchy(ImportedModule, ARImportOrTextualHeader);
229244
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
230-
CollectModuleMapsForHierarchy(UndeclaredModule);
245+
CollectModuleMapsForHierarchy(UndeclaredModule, ARImportOrTextualHeader);
231246

232247
for (auto *M : CurrentModule->submodules())
233248
Q.push(M);
@@ -256,7 +271,7 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
256271

257272
for (const auto &KH : HS.findResolvedModulesForHeader(*File))
258273
if (const Module *M = KH.getModule())
259-
CollectModuleMapsForHierarchy(M);
274+
CollectModuleMapsForHierarchy(M, ARTextualHeader);
260275
}
261276

262277
// FIXME: This algorithm is not correct for module map hierarchies where
@@ -278,13 +293,16 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
278293
// includes a module map defining a module that's not a submodule of X.
279294

280295
llvm::DenseSet<const FileEntry *> ModuleFileEntries;
281-
for (FileID MM : ModuleMaps) {
282-
if (auto *FE = SM.getFileEntryForID(MM))
296+
llvm::DenseSet<FileID> ModuleFileIDs;
297+
for (auto [FID, Reason] : ModuleMaps) {
298+
if (Reason == ARImportOrTextualHeader)
299+
ModuleFileIDs.insert(FID);
300+
if (auto *FE = SM.getFileEntryForID(FID))
283301
ModuleFileEntries.insert(FE);
284302
}
285303

286304
AffectingModuleMaps R;
287-
R.DefinitionFileIDs = std::move(ModuleMaps);
305+
R.DefinitionFileIDs = std::move(ModuleFileIDs);
288306
R.DefinitionFiles = std::move(ModuleFileEntries);
289307
return std::move(R);
290308
}

‎clang/test/Modules/prune-non-affecting-module-map-repeated-textual.cpp‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// Same as prune-non-affecting-module-map-repeated.cpp, but check that textual-only
2+
// inclusions do not cause duplication of the module map files they are defined in.
3+
4+
// RUN: rm -rf %t && mkdir %t
5+
// RUN: split-file %s %t
6+
7+
// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
8+
// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mixed.map\
9+
// RUN: -fmodule-map-file=%t/mod1.map \
10+
// RUN: -fmodule-name=mod1 -emit-module %t/mod1.map -o %t/mod1.pcm
11+
// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
12+
// RUN: -fmodule-map-file=%t/mixed.map\
13+
// RUN: -fmodule-name=mixed -emit-module %t/mixed.map -o %t/mixed.pcm
14+
// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
15+
// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod1.map -fmodule-map-file=%t/mod2.map \
16+
// RUN: -fmodule-file=%t/mod1.pcm -fmodule-file=%t/mixed.pcm \
17+
// RUN: -fmodule-name=mod2 -emit-module %t/mod2.map -o %t/mod2.pcm
18+
// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
19+
// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod2.map -fmodule-map-file=%t/mod3.map \
20+
// RUN: -fmodule-file=%t/mod2.pcm -fmodule-file=%t/mixed.pcm \
21+
// RUN: -fmodule-name=mod3 -emit-module %t/mod3.map -o %t/mod3.pcm
22+
// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
23+
// RUN: -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod3.map -fmodule-map-file=%t/mod4.map \
24+
// RUN: -fmodule-file=%t/mod3.pcm \
25+
// RUN: -fmodule-name=mod4 -emit-module %t/mod4.map -o %t/mod4.pcm
26+
// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules -fmodule-map-file=%t/base.map -fmodule-map-file=%t/mod4.map -fmodule-file=%t/mod4.pcm -fsyntax-only -verify %t/check_slocs.cc
27+
28+
//--- base.map
29+
module base { textual header "vector.h" }
30+
//--- mixed.map
31+
module mixed { textual header "mixed_text.h" header "mixed_mod.h"}
32+
//--- mod1.map
33+
module mod1 { header "mod1.h" }
34+
//--- mod2.map
35+
module mod2 { header "mod2.h" }
36+
//--- mod3.map
37+
module mod3 { header "mod3.h" }
38+
//--- mod4.map
39+
module mod4 { header "mod4.h" }
40+
//--- check_slocs.cc
41+
#include "mod4.h"
42+
#include "vector.h"
43+
#pragma clang __debug sloc_usage // expected-remark {{source manager location address space usage}}
44+
// expected-note@* {{% of available space}}
45+
46+
// base.map must only be present once, despite being used in each module.
47+
// Because its location in every module compile should be non-affecting.
48+
49+
// [email protected]:1 {{file entered 1 time}}
50+
51+
// different modules use either only textual header from mixed.map or both textual and modular
52+
// headers. Either combination must lead to only 1 use at the end, because the module is ultimately
53+
// in the import chain and any textual uses should not change that.
54+
55+
// [email protected]:1 {{file entered 1 time}}
56+
57+
// expected-note@* + {{file entered}}
58+
59+
60+
//--- vector.h
61+
#ifndef VECTOR_H
62+
#define VECTOR_H
63+
#endif
64+
65+
//--- mixed_text.h
66+
#ifndef MIXED_TEXT_H
67+
#define MIXED_TEXT_H
68+
#endif
69+
//--- mixed_mod.h
70+
#ifndef MIXED_MOD_H
71+
#define MIXED_MOD_H
72+
#endif
73+
74+
//--- mod1.h
75+
#ifndef MOD1
76+
#define MOD1
77+
#include "vector.h"
78+
#include "mixed_text.h"
79+
int mod1();
80+
#endif
81+
//--- mod2.h
82+
#ifndef MOD2
83+
#define MOD2
84+
#include "vector.h"
85+
#include "mod1.h"
86+
#include "mixed_mod.h"
87+
int mod2();
88+
#endif
89+
//--- mod3.h
90+
#ifndef MOD3
91+
#define MOD3
92+
#include "vector.h"
93+
#include "mod2.h"
94+
#include "mixed_text.h"
95+
#include "mixed_mod.h"
96+
int mod3();
97+
#endif
98+
//--- mod4.h
99+
#ifndef MOD4
100+
#define MOD4
101+
#include "vector.h"
102+
#include "mod3.h"
103+
int mod4();
104+
#endif

0 commit comments

Comments
 (0)