[PATCH] D120334: [NFC][Lexer] Use more appropriate LangOptionsBase type for Lexer::LangOpts
Dawid Jurczak via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 22 23:59:51 PST 2022
yurai007 added a comment.
In D120334#3338367 <https://reviews.llvm.org/D120334#3338367>, @jansvoboda11 wrote:
> In D120334#3338314 <https://reviews.llvm.org/D120334#3338314>, @yurai007 wrote:
>
>> In D120334#3337961 <https://reviews.llvm.org/D120334#3337961>, @jansvoboda11 wrote:
>>
>>> The performance implications are pretty interesting! Have you tried avoiding the copy altogether by just storing the reference?
>>
>> Yes, actually that was my first attempt but I failed miserably. LangOpts::LineComment is mutated in Lexer so LangOpts cannot be simply const&. Making it non-const reference is technically doable with some extra adjustment for Lexer constructors.
>> However I'm worried a bit how it would impact external LangOpts users after mutating LineComment member by Lexer. As long as LangOpts acts as simple cache it's easy to reason about but when we start to share it with Lexer callers I'm not longer sure. That's why eventually base class approach was chosen.
>
> Got it. It seems like getting rid of the `LineComment` mutation would be pretty straightforward though. I don't expect any big perf win by replacing copy of a handful of bitfields with indirection, but it would be interesting to see the experiment nonetheless.
Ok. I can explore more this direction and come back with results.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120334/new/
https://reviews.llvm.org/D120334
More information about the cfe-commits
mailing list