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

[Erich Keane via Phabricator via cfe-commits]

erichkeane 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">;
----------------
Can you explain the issue here?  This is supposed to be well-defined behavior.


================
Comment at: clang/include/clang/Basic/TargetInfo.h:599
   virtual size_t getMaxBitIntWidth() const {
-    // FIXME: this value should be llvm::IntegerType::MAX_INT_BITS, which is
-    // maximum bit width that LLVM claims its IR can support. However, most
-    // backends currently have a bug where they only support division
-    // operations on types that are <= 128 bits and crash otherwise. We're
-    // setting the max supported value to 128 to be conservative.
-    return 128;
+    // TODO: Return 128 for targets that don't link libbitint?
+    return llvm::IntegerType::MAX_INT_BITS;
----------------
This is definitely a TODO we need to do before accepting this.  The purpose of this function is that each individual target can 'override' this function.  This is an 'opt in'.


================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1232
 
+  if (llvm::isPowerOf2_32(radix)) {
+    unsigned BitsPerDigit = llvm::Log2(radix);
----------------
Can you explain what is going on here?  This isn't at all obvious to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234