[llvm] 4db3dce - [llvm] produce a more consistent estimates of bit width needed when parsing an integer (#205947)

[via llvm-commits]

Author: Jeremy Kun
Date: 2026-09-25T14:05:28-07:00
New Revision: 4db3dceb8d7bffa4b69042fe043652d616396237

URL: https://github.com/llvm/llvm-project/commit/4db3dceb8d7bffa4b69042fe043652d616396237
DIFF: https://github.com/llvm/llvm-project/commit/4db3dceb8d7bffa4b69042fe043652d616396237.diff

LOG: [llvm] produce a more consistent estimates of bit width needed when parsing an integer (#205947)

A colleague of mine noticed that `"12535824225335233"` parses in MLIR's
integer attribute parser as a 68-bit integer, even though it is a 53-bit
constant. I traced this back to `StringRef::consumeInteger`'s heuristic
estimate of the bit size. This change replaces that heuristic with a
default 64-bit storage, doubling the storage as more digits are parsed.
This produces a potentially larger over-estimate of the total storage
required, but does so in a less arbitrary manner and reduces the
over-approximation for numbers close to the 64-bit boundary.

Nb., A first iteration of this change tightened that estimate to at most
a 1-bit overapproximation with a lookup-table.

Assisted by Gemini

Added: 
    

Modified: 
    llvm/lib/Support/StringRef.cpp
    llvm/unittests/ADT/StringRefTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/StringRef.cpp b/llvm/lib/Support/StringRef.cpp
index 02270a6e223af..1741bccd752ff 100644
--- a/llvm/lib/Support/StringRef.cpp
+++ b/llvm/lib/Support/StringRef.cpp
@@ -522,16 +522,17 @@ bool StringRef::consumeInteger(unsigned Radix, APInt &Result) {
     return false;
   }
 
-  // (Over-)estimate the required number of bits.
   unsigned Log2Radix = 0;
   while ((1U << Log2Radix) < Radix) Log2Radix++;
   bool IsPowerOf2Radix = ((1U << Log2Radix) == Radix);
 
-  unsigned BitWidth = Log2Radix * Str.size();
-  if (BitWidth < Result.getBitWidth())
-    BitWidth = Result.getBitWidth(); // don't shrink the result
-  else if (BitWidth > Result.getBitWidth())
+  // Initialize Result to a reasonable starting width (at least 64 bits),
+  // but do not shrink it if it was already larger.
+  unsigned BitWidth = std::max(64U, Result.getBitWidth());
+
+  if (Result.getBitWidth() < BitWidth) {
     Result = Result.zext(BitWidth);
+  }
 
   APInt RadixAP, CharAP; // unused unless !IsPowerOf2Radix
   if (!IsPowerOf2Radix) {
@@ -558,6 +559,21 @@ bool StringRef::consumeInteger(unsigned Radix, APInt &Result) {
     if (CharVal >= Radix)
       break;
 
+    // Check if one more digit will overflow, and grow if so.
+    if (Result.getActiveBits() + Log2Radix > Result.getBitWidth()) {
+      unsigned NewWidth = Result.getBitWidth() * 2;
+
+      // Guard against overflow of the NewWidth itself
+      if (NewWidth < Result.getBitWidth())
+        return true;
+
+      Result = Result.zext(NewWidth);
+      if (!IsPowerOf2Radix) {
+        RadixAP = RadixAP.zext(NewWidth);
+        CharAP = CharAP.zext(NewWidth);
+      }
+    }
+
     // Add in this character.
     if (IsPowerOf2Radix) {
       Result <<= Log2Radix;

diff  --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index bb5fdd0a98aae..956c7295c3cd9 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -968,6 +968,65 @@ TEST(StringRefTest, consumeIntegerSigned) {
   }
 }
 
+TEST(StringRefTest, consumeIntegerAPIntBitWidth) {
+  // Decimal large number (12535824225335233)
+  // Fits in 64 bits, so should not grow beyond initial 64 bits.
+  {
+    APInt U;
+    StringRef Str = "12535824225335233";
+    bool Success = Str.consumeInteger(10, U);
+    ASSERT_FALSE(Success);
+    EXPECT_EQ(U.getZExtValue(), 12535824225335233ULL);
+    EXPECT_EQ(U.getBitWidth(), 64U);
+  }
+
+  // Hex version of same number (2c894405eaf7c1)
+  // Fits in 64 bits, so should not grow beyond initial 64 bits.
+  {
+    APInt U;
+    StringRef Str = "2c894405eaf7c1";
+    bool Success = Str.consumeInteger(16, U);
+    ASSERT_FALSE(Success);
+    EXPECT_EQ(U.getZExtValue(), 12535824225335233ULL);
+    EXPECT_EQ(U.getBitWidth(), 64U);
+  }
+
+  // A very large decimal number (100 digits)
+  // Needs 333 bits. Started at 64, doubled to 128, 256, 512.
+  {
+    APInt U;
+    std::string LargeDec(100, '9');
+    StringRef Str = LargeDec;
+    bool Success = Str.consumeInteger(10, U);
+    ASSERT_FALSE(Success);
+    EXPECT_EQ(U.getBitWidth(), 512U);
+  }
+
+  // Trailing garbage should not trigger growth or failure.
+  {
+    APInt U;
+    StringRef Str = "123g";
+    bool Success = Str.consumeInteger(10, U);
+    ASSERT_FALSE(Success);
+    EXPECT_EQ(U.getZExtValue(), 123ULL);
+    EXPECT_EQ(U.getBitWidth(), 64U);
+    EXPECT_EQ(Str, "g");
+  }
+
+  // Very long trailing garbage should also not trigger growth or failure.
+  {
+    APInt U;
+    std::string LongGarbage = "1";
+    LongGarbage.append(10000, 'g');
+    StringRef Str = LongGarbage;
+    bool Success = Str.consumeInteger(10, U);
+    ASSERT_FALSE(Success);
+    EXPECT_EQ(U.getZExtValue(), 1ULL);
+    EXPECT_EQ(U.getBitWidth(), 64U);
+    EXPECT_EQ(Str.size(), 10000U);
+  }
+}
+
 struct GetDoubleStrings {
   const char *Str;
   bool AllowInexact;