[clang] [clang] Cache the analysis-based warning policy in effect (PR #212213)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Anonmiraj (AnonMiraj)

<details>
<summary>Changes</summary>

While benchmarking with warnings enabled, I found that `AnalysisBasedWarnings::getPolicyInEffectAt` runs at the end of every function body, performing six location-sensitive `isIgnored()` queries. This overhead comes from #<!-- -->136323 ([compile-time impact](https://llvm-compile-time-tracker.com/compare.php?from=2a9f77f6bd48d757b2d45aadcb6cf76ef4b4ef32&to=71ce9e26aec00e4af27a69ccfab8ca1773ed7018&stat=instructions:u)). 

Since these six diagnostics only depend on the diagnostic state at the query location and whether it is in a system header or macro, we can cache the computed policy rather than recomputing it for every function.

The cache flushes when a `#pragma clang diagnostic` changes severities, and it bypasses active diagnostic suppression mappings.

Compile-time results for this pr:
https://llvm-compile-time-tracker.com/compare.php?from=49de424f45389cb757c3cc8c50daf38d024e2314&to=a61503b54e9568254885777cf89f5ca1586ec99f&stat=instructions%3Au

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


4 Files Affected:

- (modified) clang/include/clang/Basic/Diagnostic.h (+11) 
- (modified) clang/include/clang/Sema/AnalysisBasedWarnings.h (+7) 
- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+41-5) 
- (modified) clang/lib/Sema/Sema.cpp (+4) 


``````````diff
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 826b747f2c751..18cbd34c29ce5 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -583,6 +583,17 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
       DiagSuppressionMapping;
 
 public:
+  /// Returns a cache key representing the diagnostic state at \p Loc.
+  const void *getDiagStateKeyForLoc(SourceLocation Loc) const {
+    return GetDiagStateForLoc(Loc);
+  }
+
+  /// True if an active diagnostic suppression mapping makes severity dependent
+  /// on the file path.
+  bool hasDiagSuppressionMapping() const {
+    return static_cast<bool>(DiagSuppressionMapping);
+  }
+
   explicit DiagnosticsEngine(IntrusiveRefCntPtr<DiagnosticIDs> Diags,
                              DiagnosticOptions &DiagOpts,
                              DiagnosticConsumer *client = nullptr,
diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h
index 0ed61e56825be..c79e72bf3037f 100644
--- a/clang/include/clang/Sema/AnalysisBasedWarnings.h
+++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h
@@ -66,6 +66,10 @@ class AnalysisBasedWarnings {
   Policy PolicyOverrides;
   void clearOverrides();
 
+  /// Caches results for getPolicyInEffectAt().
+  /// Flushed whenever a diagnostic pragma changes severities.
+  llvm::DenseMap<const void *, Policy> PolicyCache[4];
+
   /// \name Statistics
   /// @{
 
@@ -129,6 +133,9 @@ class AnalysisBasedWarnings {
   // will override the policy used to issue warnings.
   Policy &getPolicyOverrides() { return PolicyOverrides; }
 
+  /// Drop cached getPolicyInEffectAt() results (diagnostic state changed).
+  void clearPolicyCache();
+
   void PrintStats() const;
 };
 
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index e070d9f1a9b85..f0674f80a825c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2759,23 +2759,59 @@ sema::AnalysisBasedWarnings::Policy
 sema::AnalysisBasedWarnings::getPolicyInEffectAt(SourceLocation Loc) {
   using namespace diag;
   DiagnosticsEngine &D = S.getDiagnostics();
+
+  // This runs at the end of every function definition, and the checks below
+  // resolve Loc against the pragma diagnostic state (and system header/macro
+  // classification) once per queried diagnostic. Those inputs fully determine
+  // the result, so cache the policy on them instead (PolicyOverrides are
+  // transient per-function state and are applied after the cache lookup).
+  const bool Cacheable = !D.hasDiagSuppressionMapping();
+  const void *StateKey = nullptr;
+  unsigned SysIdx = 0;
+  if (Cacheable) {
+    StateKey = D.getDiagStateKeyForLoc(Loc);
+    if (Loc.isValid() && D.hasSourceManager()) {
+      const SourceManager &SM = D.getSourceManager();
+      SysIdx = (SM.isInSystemHeader(SM.getExpansionLoc(Loc)) ? 2u : 0u) |
+               (SM.isInSystemMacro(Loc) ? 1u : 0u);
+    }
+    auto It = PolicyCache[SysIdx].find(StateKey);
+    if (It != PolicyCache[SysIdx].end()) {
+      Policy P = It->second;
+      P.enableCheckUnreachable |= PolicyOverrides.enableCheckUnreachable;
+      P.enableThreadSafetyAnalysis |=
+          PolicyOverrides.enableThreadSafetyAnalysis;
+      P.enableConsumedAnalysis |= PolicyOverrides.enableConsumedAnalysis;
+      return P;
+    }
+  }
+
   Policy P;
 
   // Note: The enabled checks should be kept in sync with the switch in
   // SemaPPCallbacks::PragmaDiagnostic().
   P.enableCheckUnreachable =
-      PolicyOverrides.enableCheckUnreachable ||
       areAnyEnabled(D, Loc, warn_unreachable, warn_unreachable_break,
                     warn_unreachable_return, warn_unreachable_loop_increment);
 
-  P.enableThreadSafetyAnalysis = PolicyOverrides.enableThreadSafetyAnalysis ||
-                                 areAnyEnabled(D, Loc, warn_double_lock);
+  P.enableThreadSafetyAnalysis = areAnyEnabled(D, Loc, warn_double_lock);
 
-  P.enableConsumedAnalysis = PolicyOverrides.enableConsumedAnalysis ||
-                             areAnyEnabled(D, Loc, warn_use_in_invalid_state);
+  P.enableConsumedAnalysis = areAnyEnabled(D, Loc, warn_use_in_invalid_state);
+
+  if (Cacheable)
+    PolicyCache[SysIdx][StateKey] = P;
+
+  P.enableCheckUnreachable |= PolicyOverrides.enableCheckUnreachable;
+  P.enableThreadSafetyAnalysis |= PolicyOverrides.enableThreadSafetyAnalysis;
+  P.enableConsumedAnalysis |= PolicyOverrides.enableConsumedAnalysis;
   return P;
 }
 
+void sema::AnalysisBasedWarnings::clearPolicyCache() {
+  for (auto &M : PolicyCache)
+    M.clear();
+}
+
 void sema::AnalysisBasedWarnings::clearOverrides() {
   PolicyOverrides.enableCheckUnreachable = false;
   PolicyOverrides.enableConsumedAnalysis = false;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 78fbc9e31842d..322ae410ed6cd 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -227,6 +227,10 @@ class SemaPPCallbacks : public PPCallbacks {
   }
   void PragmaDiagnostic(SourceLocation Loc, StringRef Namespace,
                         diag::Severity Mapping, StringRef Str) override {
+    // The pragma changed diagnostic severities; drop any cached analysis
+    // warning policies derived from the previous state.
+    S->AnalysisWarnings.clearPolicyCache();
+
     // If one of the analysis-based diagnostics was enabled while processing
     // a function, we want to note it in the analysis-based warnings so they
     // can be run at the end of the function body even if the analysis warnings

``````````

</details>


https://github.com/llvm/llvm-project/pull/212213