[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

vsk created this revision. vsk added reviewers: akyrtzi, doug.gregor. vsk added a subscriber: cfe-commits. The lexer sets the end location of macro arguments incorrectly *if*, while merging consecutive args to fit into a single SLocEntry, it finds args which come from different macro files. Fix t

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

nickdesaulniers added a comment. I know this was sped up slightly in 3339c568c43e4644f02289e5edfc78c860f19c9f, but this change makes `updateConsecutiveMacroArgTokens` the hottest function in clang in a bottom up profile of an entire build of the Linux kernel. It thrashes the one entry LastFile

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

vsk added a comment. In D20401#2770059 , @nickdesaulniers wrote: > I know this was sped up slightly in 3339c568c43e4644f02289e5edfc78c860f19c9f, > but this change makes `updateConsecutiveMacroArgTokens` the hottest function > in clang in a bottom up pro

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

sammccall added subscribers: hokein, sammccall. sammccall added a comment. Herald added a project: All. In D20401#2770059 , @nickdesaulniers wrote: > I know this was sped up slightly in 3339c568c43e4644f02289e5edfc78c860f19c9f, > but this change makes `u

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

nickdesaulniers added subscribers: justinstitt, nikic, tstellar, serge-sans-paille. nickdesaulniers added a comment. In D20401#3823476 , @sammccall wrote: > In D20401#2770059 , @nickdesaulniers > wrote: > >> I kno

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

sammccall added a comment. Thanks Nick for the info! No kernel experience here, so if you have any particular suggestions about how to measure the workload you care about it'd be much appreciated (e.g. are particular files that are slow enough to measure in isolation, or is it better to do a fu

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

nikic added a comment. In D20401#3824569 , @nickdesaulniers wrote: > But I wouldn't be surprised if other Linux distro's like RHEL bootstrap their > clang distribution via GCC. @tstellar or @serge-sans-paille or @nikic might > know. We did get a curio

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

hokein added a comment. >>> @hokein or I will try to find time to take a stab at this. >> >> Awesome, please keep me in the loop. > > Will do! https://reviews.llvm.org/D134942 is my attempt. In D20401#2770059 , @nickdesaulniers wrote: >> I discussed thi

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

nickdesaulniers added a comment. In D20401#3824713 , @sammccall wrote: > Thanks Nick for the info! No kernel experience here, so if you have any > particular suggestions about how to measure the workload you care about it'd > be much appreciated (e.g. ar

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

hokein added a comment. >> Meanwhile, I think besides evaluating the high level logic in in TokenLexer >> and how it might be improved, I think there's potentially an opportunity for >> a "AOS vs. SOA" speedup in SourceManager. >> SourceManager::LoadedSLocEntryTable is a >> llvm::SmallVector.

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

nickdesaulniers added a comment. In D20401#3833201 , @hokein wrote: >>> Meanwhile, I think besides evaluating the high level logic in in TokenLexer >>> and how it might be improved, I think there's potentially an opportunity >>> for a "AOS vs. SOA" speed

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

sammccall added a comment. Started a thread: https://discourse.llvm.org/t/macro-performance-lexer-and-sourcemanager/65713 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20401/new/ https://reviews.llvm.org/D20401 ___ cfe

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

vsk updated this revision to Diff 57711. vsk added a comment. - Add some comments to the unit test. http://reviews.llvm.org/D20401 Files: lib/Lex/TokenLexer.cpp test/CoverageMapping/Inputs/macros.h test/CoverageMapping/include-macros.c unittests/Lex/LexerTest.cpp Index: unittests/Lex/L

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

vsk updated this revision to Diff 57810. vsk added a comment. - Fix explanation of the test case in test/CoverageMapping. http://reviews.llvm.org/D20401 Files: lib/Lex/TokenLexer.cpp test/CoverageMapping/Inputs/macros.h test/CoverageMapping/include-macros.c unittests/Lex/LexerTest.cpp

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

vsk added a comment. I discussed this bug with Argyrios off-list, who lgtm'd on the condition that it doesn't introduce a performance regression. He suggested preprocessing Cocoa.h to stress the patch. After running a stabilization script, I used this command to stress RelNoAsserts builds of cl

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

Could you also check performance of creating a PCH file out of Cocoa.h ? > On May 19, 2016, at 1:47 PM, Vedant Kumar wrote: > > vsk added a comment. > > I discussed this bug with Argyrios off-list, who lgtm'd on the condition that > it doesn't introduce a performance regression. He suggested p

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

Yes, here are the results from the unpatched compiler: Avg. wall time (s): 0.73241 Std. deviation: 0.05850 And here are the results from the patched compiler: Avg. wall time (s): 0.75554 Std. deviation: 0.07492 The testing methodology was the same (100 trials), except I used `clang -x objectiv

Re: [PATCH] D20401: [Lexer] Don't merge macro args from different macro files

This revision was automatically updated to reflect the committed changes. Closed by commit rL270160: [Lexer] Don't merge macro args from different macro files (authored by vedantk). Changed prior to commit: http://reviews.llvm.org/D20401?vs=57810&id=57880#toc Repository: rL LLVM http://revi