[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 23 14:48:29 PDT 2023
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/69975
Deserialization of the `DIAGNOSTIC_OPTIONS` and `HEADER_SEARCH_PATHS` records is slow and done for every transitively loaded PCM. These records cannot be skipped, because the words are VBR6-encoded and we don't store the length. We could either make them binary blobs that can be skipped during deserialiazation, or skip writing them altogether. This patch takes the latter approach, since these records are not necessary in the dependency scanner. We don't make any guarantees about diagnostics reported by the scanner, and we always have the same header search paths due to strict context hashing.
>From 0270c76e779457486ee89f100db2b7151832c290 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 10 Oct 2023 14:16:13 -0700
Subject: [PATCH 1/3] [clang][modules] Make `DIAGNOSTIC_OPTIONS` skippable
---
clang/include/clang/Driver/Options.td | 4 ++
clang/include/clang/Lex/HeaderSearchOptions.h | 6 ++-
clang/lib/Serialization/ASTWriter.cpp | 4 ++
.../Modules/diagnostic-options-mismatch.c | 41 +++++++++++++++++++
4 files changed, 54 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Modules/diagnostic-options-mismatch.c
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 5415b18d3f406df..68ec32bdf56a555 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2944,6 +2944,10 @@ def fno_modules_validate_textual_header_includes :
MarshallingInfoNegativeFlag<LangOpts<"ModulesValidateTextualHeaderIncludes">>,
HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. "
"This flag will be removed in a future Clang release.">;
+defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options",
+ HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse,
+ PosFlag<SetTrue, [], [], "Disable writing diagnostic options">,
+ NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>;
def fincremental_extensions :
Flag<["-"], "fincremental-extensions">,
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index c7d95006bb779ad..d6dd94fe9466299 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -219,6 +219,9 @@ class HeaderSearchOptions {
unsigned ModulesValidateDiagnosticOptions : 1;
+ /// Whether to entirely skip writing diagnostic options.
+ unsigned ModulesSkipDiagnosticOptions : 1;
+
unsigned ModulesHashContent : 1;
/// Whether we should include all things that could impact the module in the
@@ -238,7 +241,8 @@ class HeaderSearchOptions {
ModulesValidateSystemHeaders(false),
ValidateASTInputFilesContent(false),
ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false),
- ModulesValidateDiagnosticOptions(true), ModulesHashContent(false),
+ ModulesValidateDiagnosticOptions(true),
+ ModulesSkipDiagnosticOptions(false), ModulesHashContent(false),
ModulesStrictContextHash(false) {}
/// AddPath - Add the \p Path path to the specified \p Group list.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 27700c711d52fdd..e063f8d6acc295a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1215,6 +1215,9 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
// Diagnostic options.
const auto &Diags = Context.getDiagnostics();
const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
+ if (!PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .ModulesSkipDiagnosticOptions) {
#define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name);
#define ENUM_DIAGOPT(Name, Type, Bits, Default) \
Record.push_back(static_cast<unsigned>(DiagOpts.get##Name()));
@@ -1229,6 +1232,7 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
// are generally transient files and will almost always be overridden.
Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
Record.clear();
+ }
// Header search paths.
Record.clear();
diff --git a/clang/test/Modules/diagnostic-options-mismatch.c b/clang/test/Modules/diagnostic-options-mismatch.c
new file mode 100644
index 000000000000000..cdd6f897729a9d8
--- /dev/null
+++ b/clang/test/Modules/diagnostic-options-mismatch.c
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: split-file %s %t
+
+//--- module.modulemap
+module Mod { header "mod.h" }
+//--- mod.h
+//--- tu.c
+#include "mod.h"
+
+// Without any extra compiler flags, mismatched diagnostic options trigger recompilation of modules.
+//
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -Wnon-modular-include-in-module
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
+// RUN: | FileCheck %s --check-prefix=DID-REBUILD
+// DID-REBUILD: remark: building module 'Mod'
+
+// When skipping serialization of diagnostic options, mismatches cannot be detected, old PCM file gets reused.
+//
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Wnon-modular-include-in-module
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
+// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
+// DID-REUSE-NOT: remark: building module 'Mod'
+//
+// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-DIAG-OPTS
+// NO-DIAG-OPTS-NOT: Diagnostic flags:
+
+// When disabling validation of diagnostic options, mismatches are not checked for, old PCM file gets reused.
+//
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Wnon-modular-include-in-module
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache3 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -fmodules-disable-diagnostic-validation -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \
+// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
+//
+// RUN: %clang_cc1 -module-file-info %t/cache3/Mod.pcm | FileCheck %s --check-prefix=OLD-DIAG-OPTS
+// OLD-DIAG-OPTS: Diagnostic flags:
+// OLD-DIAG-OPTS-NEXT: -Wnon-modular-include-in-module
>From b4bdbaf98b0fd2308d38a9334fa045a88885a115 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 10 Oct 2023 14:18:33 -0700
Subject: [PATCH 2/3] [clang][modules] Make `HEADER_SEARCH_PATHS` skippable
---
clang/include/clang/Lex/HeaderSearchOptions.h | 6 ++-
clang/lib/Serialization/ASTWriter.cpp | 52 +++++++++----------
2 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index d6dd94fe9466299..766b1919a5b5eab 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -222,6 +222,9 @@ class HeaderSearchOptions {
/// Whether to entirely skip writing diagnostic options.
unsigned ModulesSkipDiagnosticOptions : 1;
+ /// Whether to entirely skip writing header search paths.
+ unsigned ModulesSkipHeaderSearchPaths : 1;
+
unsigned ModulesHashContent : 1;
/// Whether we should include all things that could impact the module in the
@@ -242,7 +245,8 @@ class HeaderSearchOptions {
ValidateASTInputFilesContent(false),
ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false),
ModulesValidateDiagnosticOptions(true),
- ModulesSkipDiagnosticOptions(false), ModulesHashContent(false),
+ ModulesSkipDiagnosticOptions(false),
+ ModulesSkipHeaderSearchPaths(false), ModulesHashContent(false),
ModulesStrictContextHash(false) {}
/// AddPath - Add the \p Path path to the specified \p Group list.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index e063f8d6acc295a..d5689cb3c97cb39 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1212,12 +1212,12 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
Record.clear();
}
+ const auto &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
+
// Diagnostic options.
const auto &Diags = Context.getDiagnostics();
const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
- if (!PP.getHeaderSearchInfo()
- .getHeaderSearchOpts()
- .ModulesSkipDiagnosticOptions) {
+ if (!HSOpts.ModulesSkipDiagnosticOptions) {
#define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name);
#define ENUM_DIAGOPT(Name, Type, Bits, Default) \
Record.push_back(static_cast<unsigned>(DiagOpts.get##Name()));
@@ -1235,33 +1235,31 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
}
// Header search paths.
- Record.clear();
- const HeaderSearchOptions &HSOpts =
- PP.getHeaderSearchInfo().getHeaderSearchOpts();
-
- // Include entries.
- Record.push_back(HSOpts.UserEntries.size());
- for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
- const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
- AddString(Entry.Path, Record);
- Record.push_back(static_cast<unsigned>(Entry.Group));
- Record.push_back(Entry.IsFramework);
- Record.push_back(Entry.IgnoreSysRoot);
- }
+ if (!HSOpts.ModulesSkipHeaderSearchPaths) {
+ // Include entries.
+ Record.push_back(HSOpts.UserEntries.size());
+ for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
+ const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
+ AddString(Entry.Path, Record);
+ Record.push_back(static_cast<unsigned>(Entry.Group));
+ Record.push_back(Entry.IsFramework);
+ Record.push_back(Entry.IgnoreSysRoot);
+ }
- // System header prefixes.
- Record.push_back(HSOpts.SystemHeaderPrefixes.size());
- for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
- AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
- Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
- }
+ // System header prefixes.
+ Record.push_back(HSOpts.SystemHeaderPrefixes.size());
+ for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
+ AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
+ Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
+ }
- // VFS overlay files.
- Record.push_back(HSOpts.VFSOverlayFiles.size());
- for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles)
- AddString(VFSOverlayFile, Record);
+ // VFS overlay files.
+ Record.push_back(HSOpts.VFSOverlayFiles.size());
+ for (StringRef VFSOverlayFile : HSOpts.VFSOverlayFiles)
+ AddString(VFSOverlayFile, Record);
- Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
+ Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
+ }
// Write out the diagnostic/pragma mappings.
WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule);
>From f602cd00b56e20861f5d8800bfcc6245a7d3d37e Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 10 Oct 2023 14:21:28 -0700
Subject: [PATCH 3/3] [clang][deps] Skip `DIAGNOSTIC_OPTIONS` and
`HEADER_SEARCH_PATHS`
---
.../lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 29df0c3a0afdb5c..dc7bb289cf8f905 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -252,6 +252,8 @@ class DependencyScanningAction : public tooling::ToolAction {
// TODO: Implement diagnostic bucketing to reduce the impact of strict
// context hashing.
ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
+ ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
+ ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
// Avoid some checks and module map parsing when loading PCM files.
ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;
More information about the cfe-commits
mailing list