[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

Tue Mar 22 09:28:32 PDT 2022

mgehre-amd marked an inline comment as done.
mgehre-amd added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422

+def err_int_to_float_bit_int_max_size : Error<
+  "cannot convert '_BitInt' operands of more than %0 bits to floating point">;
----------------
erichkeane wrote:
> Can you explain the issue here?  This is supposed to be well-defined behavior.
I saw `Assertion LC != RTLIB::UNKNOWN_LIBCALL && "Don't know how to expand this SINT_TO_FP!"' failed.` in the backend. I think we would need to add library calls for floating to bitint (and vice versa) to the bitint library to enable that.

================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1232

+  if (llvm::isPowerOf2_32(radix)) {
+    unsigned BitsPerDigit = llvm::Log2(radix);
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234