[cfe-dev] Clang Preprocessor Speed Up

Wed Jun 15 11:27:41 PDT 2016

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