Skip to content

Commit d28b524

Browse files
jansvoboda11jansvoboda11
authored
Merge pull request #7746 from apple/jan_svoboda/20230725-skip-slow-pcm-records
2 parents b38f34a + e395b6f commit d28b524

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)