[cfe-dev] Clang Preprocessor Speed Up
Reid Kleckner via cfe-dev
cfe-dev at lists.llvm.org
Wed Jun 15 15:34:16 PDT 2016
I didn't have a lot of time to dig into the patch, but an off by default
flag to speed things up with a reduced the diagnostic experience doesn't
seem that valuable.
I thought there was a macro expansion note backtrace limit. Is there a way
that we can leverage that to generate unique expansion source locations
when the expansion level is low but cut them off when the expansion level
is deep? I'm not a preprocessor expert, so can you elaborate on exactly
what you're caching and reusing here?
On Wed, Jun 15, 2016 at 1:17 PM, Андрей Серебро <cfe-dev at lists.llvm.org>
wrote:
> Yep, of course it passes, since without using the new flag it's pretty
> much the same release-38.
> I don't know, probably, either some new tests should be generated and
> clang with new flag should be run on them or we can just use old ones but
> somehow make clang run with new flag...
>
> 15.06.2016, 21:28, "Bruno Cardoso Lopes" <bruno.cardoso at gmail.com>:
>
> Hi, the speedups look great. Do this patch passes a clean `ninja
> check-clang`?
>
> On Wed, Jun 15, 2016 at 8:30 AM, Андрей Серебро <cfe-dev at lists.llvm.org>
> wrote:
>
> Ouch, forgot to attach stats. Here they are.
>
> 15.06.2016, 18:28, "Андрей Серебро" <andy.silv at yandex.ru>:
>
> So, taking your advice I got rid of caching for now and concentrated on
> turning off source locations.
>
> I have introduced new flag -no-macro-exploc-tracking. Passing it to
> compiler
> does exactly what is intended to do - we don't generate expansion
> locations
> for macro tokens. To evaluate correct __LINE__ expansion, we just memorize
> the top most macro that is currently being expanded (it is ok since all
> macro expansions turn to produce one-liners... but there are some cases
> with
> multiline arguments for function-like macros, I didn't really get right
> now
> should we do something with it).
>
> I have tested it tough on boost 1.61. Among first 11000 files at least the
> success of compilation is the same for patched and clean release 38.
>
> Average boosting of preprocessing is 1.2 times. Also cases in which clang
> ran out of source locations will be less frequent I hope. At least, here
> is
> a fine example where compiler doesn't fall with the flag and fall without:
>
> clang -c -ftemplate-depth-32 -O3 -finline-functions -Wno-inline -Wall
> -pedantic -Wno-variadic-macros -pedantic-errors -std=c++11 -I
> $BOOST_1_61_HOME -no-macro-exploc-tracking
> $BOOST_1_61_HOME/libs/vmd/test/test_doc_modifiers_return_type.cpp
>
> If I don't put the flag, the process hangs (at least, on my machine).
>
> I attach the patch, hope for some feedback.
>
> P.S. Also here are preprocessing benchmarks for boost.
>
> 09.06.2016, 16:19, "Андрей Серебро" <andy.silv at yandex.ru>:
>
> Interesting moment about Andy's comment. I found out that GCC already has
> (if I understood correctly) teh feature with expansion tracking depth.
>
> The option is -ftrack-macro-expansion[=level] and it's description can be
> found here:
> https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html
>
> As I understand, clang doesn't bother with this yet.
>
> 05.06.2016, 01:38, "Андрей Серебро" <andy.silv at yandex.ru>:
>
> Well, you know, this splitting sounds like a good idea. I guess I'll try
> to
> follow your advice, for it's really somehow strange to solve two tasks in
> a
> time.
>
> I weighed the advantages and disadvantages and I guess that saving source
> locations would just vanish any sense from the activity of boosting macro
> expansion, because we will need to do all the same works as we did without
> caching at all: to create proper locations we will need to go down from
> the
> very top macro downside for every token. So, the idea from my previous
> post
> was not that sane - saving proper source locations means just give up the
> idea at all, it will barely reduce time consumption at all.
>
> You are right about debugging purpose of SL, and I think the idea with
> limitation on depths, for which info will be present, is solid. Looks like
> more flexible then complete turning off.
>
> About scratch space - to be honest, I haven't thought about it yet. I
> didn't
> even look in this side, so that's good you have pointed it. First of all,
> I
> need to understand what is this scratch space all about. =)
>
> Such things like pragmas inside macro definitions which spoil caching can
> be
> overcome by just forbidding caching in certain situations, yet of course
> all
> such situations should be defined carefully.
>
> 04.06.2016, 10:43, "Andy Gibbs" <andyg1001 at hotmail.co.uk>:
>
> On Friday 03 Jun 2016 14:33:37 Андрей Серебро wrote:
>
> Ok, that would be a miracle.
>
> As I understand, unfortunately getting rid of proper source locations
> causes
> big problems, yet it's currently hard for me to find some small samples
> on
> which everything crashes. I mean, it would be great to reduce the crash
> code dramatically, for now the examples of misbehavior are huge.
> I think a good understanding of interactions between SourceManager, Lexer
> and Parser is needed now because it's somewhere in between their
> communication, so to say, which turns to be broken after the patch. I
> see a
> possibility in trying to save proper SourceLocations, yet I'm not sure it
> will be such an easy feature.
> I wonder if I can find some person who deeply understands the concept of
> SourceLocations so that it's possible to discuss the underlying problems?
> Guys, I need your piece of advice :|
>
>
> My suggestion, but others should jump in here too, is that you split the
> patch
> up into two separate aims.
>
> The first aim would be the reduction of SourceLocation information.
>
> The second aim would be caching of previous macro expansions (from what I
> understand of your patch, I think this is what you are attempting?).
>
> I think this would be a benefit since it may be easier to get the patches
> working if you work on the two parts independently. Plus you indicated
> that
> the first still has a sizeable benefit. And I think the second will give
> you
> problems (detailed below).
>
> I'm afraid I am not able to help you much on the interactions of the
> SourceManager, Lexer and Parser. There are others on this mailing list who
> would be able to help you much better in this regard.
>
> Regarding reduction of SourceLocation information, these are my thoughts.
> Please note that I haven't assessed the practicality of this, and I'm
> writing
> as I think so it may not even be coherent! Caveats aside, you talked
> previously of having this as a compiler switch. I think this is a good
> idea
> since the SourceLocation information is needed for debugging macro
> expansions.
> I would suggest implementing this switch as a depth counter so that
> SourceLocation information is only once a macro expansion reaches a
> certain
> depth the SourceLocation information will be elided. In a sense, the
> important information of a macro expansion is the SourceLocations of the
> original tokens going into the macro and the SourceLocation of the final
> scratch buffer. At this point, I'm wondering whether it is useful to have
> SourceLocation information for concatenated tokens... Maybe someone else
> can
> give there thoughts here.
>
> The other thing you can think about here is that of actually releasing a
> scratch buffer no longer in use. I think clang doesn't do this currently
> (probably because of keeping all the SourceLocation information), but if
> you
> looked at having a reference count on a scratch buffer then maybe it will
> be
> possible to release it once its last use is complete. This has the benefit
> of
> also freeing memory... and may also improve the speed of the compiler.
> Again,
> others should give their thoughts here. Personally, I would be very
> interested to see if this was a possible line of improvement since with
> extensive macro expansions, the memory consumption of clang is huge.
>
> In fact, thinking about this, this may be the first place to start -- it
> is
> sort of linked to eliding SourceLocation information, but it has the
> advantage
> that you are freeing the whole scratch buffer since it has been
> incorporated
> into another higher-level scratch buffer, and so is no longer referenced
> anywhere and its disappearance shouldn't (hopefully!) cause so many
> problems
> in clang diagnostics, for example, which even so would need to be adapted
> to
> skip these elided macro expansion steps.
>
> Finally, regarding the caching of previous macro expansions, you need to
> be
> very careful because macro definitions can change, yes even inside a macro
> expansion too (think pragma pop_macro). For that matter, you need to think
> about how to handle any _Pragma expansion since these can cause side
> effects.
> This is why I would leave this side of your patch until last. It will be
> the
> hardest to test and guarantee correctness.
>
> Keep up the good work! :o)
>
> Cheers,
> Andy
>
>
>
> --
> Regards,
> Andrei Serebro
> tel. +79111758381
>
>
>
>
> --
> Regards,
> Andrei Serebro
> tel. +79111758381
>
>
>
>
> --
> Regards,
> Andrei Serebro
> tel. +79111758381
>
>
>
>
> --
> Regards,
> Andrei Serebro
> tel. +79111758381
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
>
>
>
> --
> Regards,
> Andrei Serebro
> tel. +79111758381
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160615/39d96f55/attachment.html>
More information about the cfe-dev
mailing list