Skip to content

Commit 280a62f

Browse files
committed
[clang][modules][deps] Avoid checks for relocated modules
Currently, `ASTReader` performs some checks to diagnose relocated modules. This can add quite a bit of overhead to the scanner: it requires looking up, parsing and resolving module maps for all transitively loaded module files (and all the module maps encountered in the search paths on the way). Most of those checks are not really useful in the scanner anyway, since it uses strict context hash and immutable filesystem, which prevent those scenarios in the first place. This can speed up scanning by up to 30%. Depends on D150292. Reviewed By: benlangmuir Differential Revision: https://reviews.llvm.org/D150320 (cherry picked from commit 227f719)
1 parent b4ee045 commit 280a62f

File tree

4 files changed

+24
-16
lines changed

4 files changed

+24
-16
lines changed

clang/include/clang/Lex/PreprocessorOptions.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ class PreprocessorOptions {
8080
std::vector<std::string> Includes;
8181
std::vector<std::string> MacroIncludes;
8282

83+
/// Perform extra checks when loading PCM files for mutable file systems.
84+
bool ModulesCheckRelocated = true;
85+
8386
/// Initialize the preprocessor with the compiler and target specific
8487
/// predefines.
8588
bool UsePredefines = true;

clang/lib/Serialization/ASTReader.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2949,6 +2949,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
29492949
BaseDirectoryAsWritten = Blob;
29502950
assert(!F.ModuleName.empty() &&
29512951
"MODULE_DIRECTORY found before MODULE_NAME");
2952+
F.BaseDirectory = std::string(Blob);
2953+
if (!PP.getPreprocessorOpts().ModulesCheckRelocated)
2954+
break;
29522955
// If we've already loaded a module map file covering this module, we may
29532956
// have a better path for it (relative to the current build).
29542957
Module *M = PP.getHeaderSearchInfo().lookupModule(
@@ -2970,8 +2973,6 @@ ASTReader::ReadControlBlock(ModuleFile &F,
29702973
}
29712974
}
29722975
F.BaseDirectory = std::string(M->Directory->getName());
2973-
} else {
2974-
F.BaseDirectory = std::string(Blob);
29752976
}
29762977
break;
29772978
}
@@ -3949,7 +3950,8 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
39493950
// usable header search context.
39503951
assert(!F.ModuleName.empty() &&
39513952
"MODULE_NAME should come before MODULE_MAP_FILE");
3952-
if (F.Kind == MK_ImplicitModule && ModuleMgr.begin()->Kind != MK_MainFile) {
3953+
if (PP.getPreprocessorOpts().ModulesCheckRelocated &&
3954+
F.Kind == MK_ImplicitModule && ModuleMgr.begin()->Kind != MK_MainFile) {
39533955
// An implicitly-loaded module file should have its module listed in some
39543956
// module map file that we've already loaded.
39553957
Module *M =

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,9 @@ class DependencyScanningAction : public tooling::ToolAction {
432432
// context hashing.
433433
ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
434434

435+
// Avoid some checks and module map parsing when loading PCM files.
436+
ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;
437+
435438
std::unique_ptr<FrontendAction> Action;
436439

437440
if (ModuleName)

clang/test/ClangScanDeps/header-search-pruning-transitive.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ module X { header "X.h" }
7272
// CHECK: ],
7373
// CHECK-NEXT: "context-hash": "[[HASH_X:.*]]",
7474
// CHECK-NEXT: "file-deps": [
75-
// CHECK-NEXT: "[[PREFIX]]/./X.h",
76-
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
75+
// CHECK-NEXT: "[[PREFIX]]/X.h",
76+
// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
7777
// CHECK-NEXT: ],
7878
// CHECK-NEXT: "name": "X"
7979
// CHECK-NEXT: },
@@ -84,11 +84,11 @@ module X { header "X.h" }
8484
// CHECK: ],
8585
// CHECK-NEXT: "context-hash": "[[HASH_Y_WITH_A]]",
8686
// CHECK-NEXT: "file-deps": [
87-
// CHECK-NEXT: "[[PREFIX]]/./Y.h",
88-
// CHECK-NEXT: "[[PREFIX]]/./a/a.h",
89-
// CHECK-NEXT: "[[PREFIX]]/./begin/begin.h",
90-
// CHECK-NEXT: "[[PREFIX]]/./end/end.h",
91-
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
87+
// CHECK-NEXT: "[[PREFIX]]/Y.h",
88+
// CHECK-NEXT: "[[PREFIX]]/a/a.h",
89+
// CHECK-NEXT: "[[PREFIX]]/begin/begin.h",
90+
// CHECK-NEXT: "[[PREFIX]]/end/end.h",
91+
// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
9292
// CHECK-NEXT: ],
9393
// CHECK-NEXT: "name": "Y"
9494
// CHECK-NEXT: }
@@ -126,8 +126,8 @@ module X { header "X.h" }
126126
// also has a different context hash from the first version of module X.
127127
// CHECK-NOT: "context-hash": "[[HASH_X]]",
128128
// CHECK: "file-deps": [
129-
// CHECK-NEXT: "[[PREFIX]]/./X.h",
130-
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
129+
// CHECK-NEXT: "[[PREFIX]]/X.h",
130+
// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
131131
// CHECK-NEXT: ],
132132
// CHECK-NEXT: "name": "X"
133133
// CHECK-NEXT: },
@@ -138,10 +138,10 @@ module X { header "X.h" }
138138
// CHECK: ],
139139
// CHECK-NEXT: "context-hash": "[[HASH_Y_WITHOUT_A]]",
140140
// CHECK-NEXT: "file-deps": [
141-
// CHECK-NEXT: "[[PREFIX]]/./Y.h",
142-
// CHECK-NEXT: "[[PREFIX]]/./begin/begin.h",
143-
// CHECK-NEXT: "[[PREFIX]]/./end/end.h",
144-
// CHECK-NEXT: "[[PREFIX]]/./module.modulemap"
141+
// CHECK-NEXT: "[[PREFIX]]/Y.h",
142+
// CHECK-NEXT: "[[PREFIX]]/begin/begin.h",
143+
// CHECK-NEXT: "[[PREFIX]]/end/end.h",
144+
// CHECK-NEXT: "[[PREFIX]]/module.modulemap"
145145
// CHECK-NEXT: ],
146146
// CHECK-NEXT: "name": "Y"
147147
// CHECK-NEXT: }

0 commit comments

Comments
 (0)