[PATCH] D28207: Add second fast path for DiagnosticsEngine::GetDiagStatePointForLoc

Richard Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 2 14:57:11 PST 2017


rsmith added inline comments.


================
Comment at: lib/Basic/Diagnostic.cpp:179
+
+  // 2nd most frequent case: L is before the first diag state change.
+  FullSourceLoc FirstStateChangePos = DiagStatePoints[1].Loc;
----------------
It's surprising to me that this would be particularly frequent.

I suspect what you're actually seeing is a consequence of a bug in how we manage `DiagStatePoint`s with modules. It looks like `ASTReader::InitializeContext` is called once per top-level PCM file that we load, and its call to `ReadPragmaDiagnosticMappings` adds entries to the `DiagStatePoints` list regardless of whether they've already been added. So, we'll end up with duplicates in the `DiagStatePoints` list, and it won't even be in translation unit order.

Can you take a look at the `DiagStatePoints` list for a translation unit where you see a performance problem and check whether it seems reasonable?


https://reviews.llvm.org/D28207





More information about the cfe-commits mailing list