Skip to content

[clang][deps] Skip writing DIAG_PRAGMA_MAPPINGS record #70874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

jansvoboda11
Copy link
Contributor

@jansvoboda11 jansvoboda11 commented Oct 31, 2023

Following up on #69975, this patch skips writing DIAG_PRAGMA_MAPPINGS as well. Deserialization of this PCM record is still showing up in profiles, since it needs to be VBR-decoded for every transitively loaded PCM file.

The scanner doesn't make any guarantees about diagnostic accuracy (and it even disables all warnings), so skipping this record should be safe.

@jansvoboda11 jansvoboda11 force-pushed the skip-pragma-diag-mappings branch from c3602bc to 450c5ad Compare November 9, 2023 23:57
@jansvoboda11 jansvoboda11 marked this pull request as ready for review November 9, 2023 23:57
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Nov 9, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 9, 2023

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/70874.diff

3 Files Affected:

  • (modified) clang/include/clang/Lex/HeaderSearchOptions.h (+7-1)
  • (modified) clang/lib/Serialization/ASTWriter.cpp (+2-2)
  • (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp (+2)
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 114af14dec7f5a8..fa2d0b502d72c19 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -247,6 +247,11 @@ class HeaderSearchOptions {
   LLVM_PREFERRED_TYPE(bool)
   unsigned ModulesSkipHeaderSearchPaths : 1;
 
+  /// Whether to entirely skip writing pragma diagnostic mappings.
+  /// Primarily used to speed up deserialization during dependency scanning.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned ModulesSkipPragmaDiagnosticMappings : 1;
+
   LLVM_PREFERRED_TYPE(bool)
   unsigned ModulesHashContent : 1;
 
@@ -270,7 +275,8 @@ class HeaderSearchOptions {
         ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false),
         ModulesValidateDiagnosticOptions(true),
         ModulesSkipDiagnosticOptions(false),
-        ModulesSkipHeaderSearchPaths(false), ModulesHashContent(false),
+        ModulesSkipHeaderSearchPaths(false),
+        ModulesSkipPragmaDiagnosticMappings(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 1e86566d81fbc02..0161ad10f3f2381 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1261,8 +1261,8 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
     Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
   }
 
-  // Write out the diagnostic/pragma mappings.
-  WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule);
+  if (!HSOpts.ModulesSkipPragmaDiagnosticMappings)
+    WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule);
 
   // Header search entry usage.
   auto HSEntryUsage = PP.getHeaderSearchInfo().computeUserEntryUsage();
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index d1d3cc50cb25b83..c54e6d523800b6b 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -255,6 +255,8 @@ class DependencyScanningAction : public tooling::ToolAction {
     ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;
     ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
     ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
+    ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings =
+        true;
 
     // Avoid some checks and module map parsing when loading PCM files.
     ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false;

@jansvoboda11 jansvoboda11 merged commit 22c6851 into llvm:main Nov 10, 2023
@jansvoboda11 jansvoboda11 deleted the skip-pragma-diag-mappings branch November 10, 2023 15:04
jansvoboda11 added a commit to swiftlang/llvm-project that referenced this pull request Nov 10, 2023
Following up on llvm#69975, this patch skips writing `DIAG_PRAGMA_MAPPINGS`
as well. Deserialization of this PCM record is still showing up in
profiles, since it needs to be VBR-decoded for every transitively loaded
PCM file.

The scanner doesn't make any guarantees about diagnostic accuracy (and
it even disables all warnings), so skipping this record should be safe.

(cherry picked from commit 22c6851)
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
Following up on llvm#69975, this patch skips writing `DIAG_PRAGMA_MAPPINGS`
as well. Deserialization of this PCM record is still showing up in
profiles, since it needs to be VBR-decoded for every transitively loaded
PCM file.

The scanner doesn't make any guarantees about diagnostic accuracy (and
it even disables all warnings), so skipping this record should be safe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants