[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 25 15:32:25 PDT 2023
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/69975
>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/4] [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/4] [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/4] [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;
>From f33ab359c2345c7571f62ccccac4a7af0d86b6ed Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 25 Oct 2023 15:30:42 -0700
Subject: [PATCH 4/4] Add -fmodules-skip-header-search-paths test, run
clang-format
---
clang/include/clang/Driver/Options.td | 4 +++
clang/lib/Frontend/FrontendActions.cpp | 15 ++++++++
clang/lib/Serialization/ASTWriter.cpp | 20 +++++------
.../Modules/header-search-paths-mismatch.c | 36 +++++++++++++++++++
4 files changed, 65 insertions(+), 10 deletions(-)
create mode 100644 clang/test/Modules/header-search-paths-mismatch.c
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 68ec32bdf56a555..2a37c089fa44f40 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2948,6 +2948,10 @@ defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-opti
HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse,
PosFlag<SetTrue, [], [], "Disable writing diagnostic options">,
NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>;
+defm modules_skip_header_search_paths : BoolFOption<"modules-skip-header-search-paths",
+ HeaderSearchOpts<"ModulesSkipHeaderSearchPaths">, DefaultFalse,
+ PosFlag<SetTrue, [], [], "Disable writing header search paths">,
+ NegFlag<SetFalse>, BothFlags<[], [CC1Option]>>;
def fincremental_extensions :
Flag<["-"], "fincremental-extensions">,
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index ce6e0b7126b635a..2afcf1cf9f68c81 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -661,6 +661,21 @@ namespace {
return false;
}
+ bool ReadHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
+ bool Complain) override {
+ Out.indent(2) << "Header search paths:\n";
+ Out.indent(4) << "User entries:\n";
+ for (const auto &Entry : HSOpts.UserEntries)
+ Out.indent(6) << Entry.Path << "\n";
+ Out.indent(4) << "System header prefixes:\n";
+ for (const auto &Prefix : HSOpts.SystemHeaderPrefixes)
+ Out.indent(6) << Prefix.Prefix << "\n";
+ Out.indent(4) << "VFS overlay files:\n";
+ for (const auto &Overlay : HSOpts.VFSOverlayFiles)
+ Out.indent(6) << Overlay << "\n";
+ return false;
+ }
+
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d5689cb3c97cb39..b5a9f745b5da86f 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1222,16 +1222,16 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
#define ENUM_DIAGOPT(Name, Type, Bits, Default) \
Record.push_back(static_cast<unsigned>(DiagOpts.get##Name()));
#include "clang/Basic/DiagnosticOptions.def"
- Record.push_back(DiagOpts.Warnings.size());
- for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
- AddString(DiagOpts.Warnings[I], Record);
- Record.push_back(DiagOpts.Remarks.size());
- for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
- AddString(DiagOpts.Remarks[I], Record);
- // Note: we don't serialize the log or serialization file names, because they
- // are generally transient files and will almost always be overridden.
- Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
- Record.clear();
+ Record.push_back(DiagOpts.Warnings.size());
+ for (unsigned I = 0, N = DiagOpts.Warnings.size(); I != N; ++I)
+ AddString(DiagOpts.Warnings[I], Record);
+ Record.push_back(DiagOpts.Remarks.size());
+ for (unsigned I = 0, N = DiagOpts.Remarks.size(); I != N; ++I)
+ AddString(DiagOpts.Remarks[I], Record);
+ // Note: we don't serialize the log or serialization file names, because
+ // they are generally transient files and will almost always be overridden.
+ Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
+ Record.clear();
}
// Header search paths.
diff --git a/clang/test/Modules/header-search-paths-mismatch.c b/clang/test/Modules/header-search-paths-mismatch.c
new file mode 100644
index 000000000000000..d733b39bb2367fa
--- /dev/null
+++ b/clang/test/Modules/header-search-paths-mismatch.c
@@ -0,0 +1,36 @@
+// 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"
+
+//--- one/foo.h
+//--- two/foo.h
+
+// By default, mismatched header search paths are ignored, old PCM file gets reused.
+//
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -I %t/one
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \
+// RUN: -fsyntax-only %t/tu.c -I %t/two -Rmodule-build 2>&1 \
+// RUN: | FileCheck %s --allow-empty --check-prefix=DID-REUSE
+// DID-REUSE-NOT: remark: building module 'Mod'
+//
+// RUN: %clang_cc1 -module-file-info %t/cache1/Mod.pcm | FileCheck %s --check-prefix=HS-PATHS -DPREFIX=%/t
+// HS-PATHS: Header search paths:
+// HS-PATHS-NEXT: User entries:
+// HS-PATHS-NEXT: [[PREFIX]]/one
+
+// When skipping serialization of header search paths, 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-header-search-paths -I %t/one
+// 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-header-search-paths -I %t/two -Rmodule-build 2>&1 \
+// RUN: | FileCheck %s --check-prefix=DID-REUSE --allow-empty
+//
+// RUN: %clang_cc1 -module-file-info %t/cache2/Mod.pcm | FileCheck %s --check-prefix=NO-HS-PATHS
+// NO-HS-PATHS-NOT: Header search paths:
More information about the cfe-commits
mailing list