erichkeane added inline comments.
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1232
+ if (llvm::isPowerOf2_32(radix)) {
+ unsigned BitsPerDigit = llvm::Log2(radix);
----------------
mgehre-amd wrote:
> erichkeane wrote:
> > Can you explain what is going on here? This isn't at all obvious to me.
> When I adapted the test case clang/test/Lexer/bitint-constants.c
> https://reviews.llvm.org/D122234#change-GjEFAAe2jzfe to test literals with
> 2097152 hex digits, clang spend minutes in the code below to try to turn that
> literal into a ApInt of 8388608 bits.
>
> That is because the code below does a generic `Val *= Radix` and `Val +=
> Digit`. The `Val *= Radix` is very slow on a ApInt with 8388608 bits, and
> it's also not needed when
> we know that `Radix` is a power of two. Then one can just copy the bits from
> each Digit into the right position in Val.
>
> If the integer literals can just be 128 bits or something, it doesn't matter,
> but now we need to support really big integer literals with having increased
> the limit on _BitInt width.
Hmm, so this is only really an issue with base-10 integer values? Is the same
'clang spent minutes in...' still a problem for base 10 literals? It would be
unfortunate if we cannot just fix that.
Additionally, I'd like some comment to explain this, it isn't particularly
clear what it is doing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122234/new/
https://reviews.llvm.org/D122234
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
