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;
----------------
djasper wrote:
> rsmith wrote:
> > 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?
> I looked at the DiagStatePoints and they do look somewhat sane but suspicious.
>
> The translation unit I am looking at has (AFAICT) only includes that get
> translated to modules. DiagStatePoints only has entries for the translation
> unit itself, not a single one coming from any of the modules. So
> DiagStatePoints looks like:
>
> [0]: <<invalid>> (for the command line)
> [1]: myfile.cc:100
> [2]: myfile.cc:100
> ...
> [2500]: myfile.cc:2800
>
> And because of that, the new fast path is hit every single time when a source
> location coming from a module is queried. There are always two entries for a
> line of myfile.cc which always seem to denote entering and exiting a macro.
>
> So, specific questions:
> - Should there by DiagStatePoints from modules?
> - Should there really be a DiagStatePoint entry (or actually two) for every
> macro invocation in myfile.cc?
Yes, there should be `DiagStatePoints` from modules, but support for that seems
to basically be unimplemented at this point.
I believe the `DiagStatePoints` for macros are coming from a `_Pragma("clang
diagnostic ...")` within the macro expansion. Doesn't look like there's any
other way we'd create them.
https://reviews.llvm.org/D28207
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
