[PATCH] D134942: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D134942#3828449 <https://reviews.llvm.org/D134942#3828449>, @nickdesaulniers wrote:

> In D134942#3827216 <https://reviews.llvm.org/D134942#3827216>, @hokein wrote:
>
>> Some more perf data on building linux kernel (x86_64)
>>
>> before: getFileID 2.4% (1.10% on `getFileIDSlow`)
>> after: getFileID 2.35% (1.05% on `getFileIDSlow`)
>
> What compiler was used as the bootstrap?
>
> For me bootstrapping w/ clang, I observed:
>
>   Before:
>   +    2.11%  clang-15                           [.] clang::SourceManager::getFileIDLocal
>   After:
>   +    2.01%  clang-15                           [.] clang::SourceManager::getFileIDLocal

That's interesting. I also used clang (v14) for bootstrapping.

What's your base revision? I'm using d32b8fdbdb4b99a5cc21604db6211fc506eb1f9b <https://reviews.llvm.org/rGd32b8fdbdb4b99a5cc21604db6211fc506eb1f9b>, looking at your profile (clang-15), I think your base revision is older than mine (my profile shows clang-16).



================
Comment at: clang/lib/Lex/TokenLexer.cpp:1008-1013
+    assert(!Partition.empty() &&
+           llvm::all_of(Partition.drop_front(),
+                        [&SM](const Token &T) {
+                          return SM.getFileID((&T - 1)->getLocation()) ==
+                                 SM.getFileID(T.getLocation());
+                        }) &&
----------------
nickdesaulniers wrote:
> Could you just check that all of the tokens in the partition have the same fileID as the first token?
> 
> ```
> FileID FirstFID = SM.getFileID(Partition[0]->getLocation());
> llvm::all_of(Partition, [&SM, &FirstID](const Token &T) { return SM.getFileID(T.getLocation() == FID; });
> ```
> or move the assertion into the take_while above so we iterate less?
The optimization for this case is that we don't call any `getFileID`, the getFileID is only needed in the assert sanity check, so moving the assertion to `take_while` doesn't really work.

I adjust the code to save some unnecessary `getFileID` call in assert.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134942/new/

https://reviews.llvm.org/D134942