Skip to content

Commit e395b6f

Browse files
jansvoboda11jansvoboda11
committed
[clang][deps] Skip slow UNHASHED_CONTROL_BLOCK records (llvm#69975)
Deserialization of the `DIAGNOSTIC_OPTIONS` and `HEADER_SEARCH_PATHS` records is slow and done for every transitively loaded PCM. Deserialization of these records cannot be skipped, because the words are VBR6-encoded and we don't store the length of the entire record. We could either turn them into binary blobs that can be skipped during deserialization, or skip writing them altogether. This patch takes the latter approach, since these records are not necessary in scanning PCMs. The scanner doesn't make any guarantees about the accuracy of diagnostics, and we always have the same header search paths due to strict context hashing. The commit that makes the `DIAGNOSTIC_OPTIONS` record skippable was originally implemented by @benlangmuir in a downstream repo. (cherry picked from commit 6c465a2)
1 parent b38f34a commit e395b6f

File tree

7 files changed

+144
-43
lines changed

7 files changed

+144
-43
lines changed

‎clang/include/clang/Driver/Options.td

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,6 +2551,14 @@ def fno_modules_validate_textual_header_includes :
25512551
MarshallingInfoNegativeFlag<LangOpts<"ModulesValidateTextualHeaderIncludes">>,
25522552
HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. "
25532553
"This flag will be removed in a future Clang release.">;
2554+
defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options",
2555+
HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse,
2556+
PosFlag<SetTrue, [], "Disable writing diagnostic options">,
2557+
NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
2558+
defm modules_skip_header_search_paths : BoolFOption<"modules-skip-header-search-paths",
2559+
HeaderSearchOpts<"ModulesSkipHeaderSearchPaths">, DefaultFalse,
2560+
PosFlag<SetTrue, [], "Disable writing header search paths">,
2561+
NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
25542562

25552563
def fincremental_extensions :
25562564
Flag<["-"], "fincremental-extensions">,
@@ -7077,11 +7085,6 @@ defm cache_disable_replay : BoolFOption<"cache-disable-replay",
70777085
PosFlag<SetTrue, [], "Disable replaying a cached compile job">,
70787086
NegFlag<SetFalse>>;
70797087

7080-
defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options",
7081-
HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse,
7082-
PosFlag<SetTrue, [], "Disable writing diagnostic options">,
7083-
NegFlag<SetFalse>>;
7084-
70857088
def fcompilation_caching_service_path : Separate<["-"], "fcompilation-caching-service-path">,
70867089
Group<f_Group>, MetaVarName<"<pathname>">,
70877090
HelpText<"Specify the socket path for connecting to a remote caching service">,

‎clang/include/clang/Lex/HeaderSearchOptions.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,13 @@ class HeaderSearchOptions {
217217
unsigned ModulesValidateDiagnosticOptions : 1;
218218

219219
/// Whether to entirely skip writing diagnostic options.
220+
/// Primarily used to speed up deserialization during dependency scanning.
220221
unsigned ModulesSkipDiagnosticOptions : 1;
221222

223+
/// Whether to entirely skip writing header search paths.
224+
/// Primarily used to speed up deserialization during dependency scanning.
225+
unsigned ModulesSkipHeaderSearchPaths : 1;
226+
222227
unsigned ModulesHashContent : 1;
223228

224229
/// Whether we should include all things that could impact the module in the
@@ -238,7 +243,8 @@ class HeaderSearchOptions {
238243
ModulesValidateSystemHeaders(false),
239244
ValidateASTInputFilesContent(false), UseDebugInfo(false),
240245
ModulesValidateDiagnosticOptions(true),
241-
ModulesSkipDiagnosticOptions(false), ModulesHashContent(false),
246+
ModulesSkipDiagnosticOptions(false),
247+
ModulesSkipHeaderSearchPaths(false), ModulesHashContent(false),
242248
ModulesStrictContextHash(false) {}
243249

244250
/// AddPath - Add the \p Path path to the specified \p Group list.

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,21 @@ namespace {
661661
return false;
662662
}
663663

664+
bool ReadHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
665+
bool Complain) override {
666+
Out.indent(2) << "Header search paths:\n";
667+
Out.indent(4) << "User entries:\n";
668+
for (const auto &Entry : HSOpts.UserEntries)
669+
Out.indent(6) << Entry.Path << "\n";
670+
Out.indent(4) << "System header prefixes:\n";
671+
for (const auto &Prefix : HSOpts.SystemHeaderPrefixes)
672+
Out.indent(6) << Prefix.Prefix << "\n";
673+
Out.indent(4) << "VFS overlay files:\n";
674+
for (const auto &Overlay : HSOpts.VFSOverlayFiles)
675+
Out.indent(6) << Overlay << "\n";
676+
return false;
677+
}
678+
664679
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
665680
bool ReadMacros, bool Complain,
666681
std::string &SuggestedPredefines) override {

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,56 +1211,54 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
12111211
Record.clear();
12121212
}
12131213

1214+
const auto &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
1215+
12141216
// Diagnostic options.
12151217
const auto &Diags = Context.getDiagnostics();
12161218
const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
1217-
if (!PP.getHeaderSearchInfo()
1218-
.getHeaderSearchOpts()
1219-
.ModulesSkipDiagnosticOptions) {
1219+
if (!HSOpts.ModulesSkipDiagnosticOptions) {
12201220
#define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name);
12211221
#define ENUM_DIAGOPT(Name, Type, Bits, Default) \
12221222
Record.push_back(static_cast<unsigned>(DiagOpts.get##Name()));
12231223
#include "clang/Basic/DiagnosticOptions.def"
1224-
Record.push_back(DiagOpts.Warnings.size());
1225-
for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
1226-
AddString(DiagOpts.Warnings[I], Record);
1227-
Record.push_back(DiagOpts.Remarks.size());
1228-
for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
1229-
AddString(DiagOpts.Remarks[I], Record);
1230-
// Note: we don't serialize the log or serialization file names, because they
1231-
// are generally transient files and will almost always be overridden.
1232-
Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
1233-
Record.clear();
1224+
Record.push_back(DiagOpts.Warnings.size());
1225+
for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
1226+
AddString(DiagOpts.Warnings[I], Record);
1227+
Record.push_back(DiagOpts.Remarks.size());
1228+
for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
1229+
AddString(DiagOpts.Remarks[I], Record);
1230+
// Note: we don't serialize the log or serialization file names, because
1231+
// they are generally transient files and will almost always be overridden.
1232+
Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
1233+
Record.clear();
12341234
}
12351235

12361236
// Header search paths.
1237-
Record.clear();
1238-
const HeaderSearchOptions &HSOpts
1239-
= PP.getHeaderSearchInfo().getHeaderSearchOpts();
1240-
1241-
// Include entries.
1242-
Record.push_back(HSOpts.UserEntries.size());
1243-
for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
1244-
const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
1245-
AddString(Entry.Path, Record);
1246-
Record.push_back(static_cast<unsigned>(Entry.Group));
1247-
Record.push_back(Entry.IsFramework);
1248-
Record.push_back(Entry.IgnoreSysRoot);
1249-
}
1237+
if (!HSOpts.ModulesSkipHeaderSearchPaths) {
1238+
// Include entries.
1239+
Record.push_back(HSOpts.UserEntries.size());
1240+
for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
1241+
const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
1242+
AddString(Entry.Path, Record);
1243+
Record.push_back(static_cast<unsigned>(Entry.Group));
1244+
Record.push_back(Entry.IsFramework);
1245+
Record.push_back(Entry.IgnoreSysRoot);
1246+
}
12501247

1251-
// System header prefixes.
1252-
Record.push_back(HSOpts.SystemHeaderPrefixes.size());
1253-
for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
1254-
AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
1255-
Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
1256-
}
1248+
// System header prefixes.
1249+
Record.push_back(HSOpts.SystemHeaderPrefixes.size());
1250+
for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
1251+
AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
1252+
Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
1253+
}
12571254

1258-
// VFS overlay files.
1259-
Record.push_back(HSOpts.VFSOverlayFiles.size());
1260-
for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles)
1261-
AddString(VFSOverlayFile, Record);
1255+
// VFS overlay files.
1256+
Record.push_back(HSOpts.VFSOverlayFiles.size());
1257+
for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles)
1258+
AddString(VFSOverlayFile, Record);
12621259

1263-
Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
1260+
Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
1261+
}
12641262

12651263
// File system options.
12661264
Record.clear();

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,8 @@ class DependencyScanningAction : public tooling::ToolAction {
435435
// TODO: Implement diagnostic bucketing to reduce the impact of strict
436436
// context hashing.
437437
ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
438+
ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
439+
ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
438440

439441
// Avoid some checks and module map parsing when loading PCM files.
440442
ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;

‎clang/test/Modules/diagnostic-options-mismatch.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: rm -rf %t && mkdir -p %t
2+
// RUN: split-file %s %t
3+
4+
//--- module.modulemap
5+
module Mod { header "mod.h" }
6+
//--- mod.h
7+
//--- tu.c
8+
#include "mod.h"
9+
10+
// Without any extra compiler flags, mismatched diagnostic options trigger recompilation of modules.
11+
//
12+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
13+
// RUN: -fsyntax-only %t/tu.c -Wnon-modular-include-in-module
14+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
15+
// RUN: -fsyntax-only %t/tu.c -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
16+
// RUN: | FileCheck %s --check-prefix=DID-REBUILD
17+
// DID-REBUILD: remark: building module 'Mod'
18+
19+
// When skipping serialization of diagnostic options, mismatches cannot be detected, old PCM file gets reused.
20+
//
21+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
22+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Wnon-modular-include-in-module
23+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
24+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
25+
// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
26+
// DID-REUSE-NOT: remark: building module 'Mod'
27+
//
28+
// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-DIAG-OPTS
29+
// NO-DIAG-OPTS-NOT: Diagnostic flags:
30+
31+
// When disabling validation of diagnostic options, mismatches are not checked for, old PCM file gets reused.
32+
//
33+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \
34+
// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Wnon-modular-include-in-module
35+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \
36+
// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
37+
// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
38+
//
39+
// RUN: %clang_cc1 -module-file-info %t/cache3/Mod.pcm | FileCheck %s --check-prefix=OLD-DIAG-OPTS
40+
// OLD-DIAG-OPTS: Diagnostic flags:
41+
// OLD-DIAG-OPTS-NEXT: -Wnon-modular-include-in-module

‎clang/test/Modules/header-search-paths-mismatch.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: rm -rf %t && mkdir -p %t
2+
// RUN: split-file %s %t
3+
4+
//--- module.modulemap
5+
module Mod { header "mod.h" }
6+
//--- mod.h
7+
//--- tu.c
8+
#include "mod.h"
9+
10+
//--- one/foo.h
11+
//--- two/foo.h
12+
13+
// By default, mismatched header search paths are ignored, old PCM file gets reused.
14+
//
15+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
16+
// RUN: -fsyntax-only %t/tu.c -I %t/one
17+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
18+
// RUN: -fsyntax-only %t/tu.c -I %t/two -Rmodule-build 2>&1 \
19+
// RUN: | FileCheck %s --allow-empty --check-prefix=DID-REUSE
20+
// DID-REUSE-NOT: remark: building module 'Mod'
21+
//
22+
// RUN: %clang_cc1 -module-file-info %t/cache1/Mod.pcm | FileCheck %s --check-prefix=HS-PATHS
23+
// HS-PATHS: Header search paths:
24+
// HS-PATHS-NEXT: User entries:
25+
// HS-PATHS-NEXT: one
26+
27+
// When skipping serialization of header search paths, mismatches cannot be detected, old PCM file gets reused.
28+
//
29+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
30+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-header-search-paths -I %t/one
31+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
32+
// RUN: -fsyntax-only %t/tu.c -fmodules-skip-header-search-paths -I %t/two -Rmodule-build 2>&1 \
33+
// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
34+
//
35+
// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-HS-PATHS
36+
// NO-HS-PATHS-NOT: Header search paths:

0 commit comments

Comments
 (0)