From ae8f78d4de403965603ed2b61898d820db2449f9 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Fri, 21 Aug 2009 03:15:28 +0000 Subject: [PATCH] Fix bug with APInt::getBitsNeeded with for base 10 numbers 0-9. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@79593 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Support/APInt.cpp | 72 ++++++++++++++++++-------------- unittests/ADT/APIntTest.cpp | 82 +++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 30 deletions(-) diff --git a/lib/Support/APInt.cpp b/lib/Support/APInt.cpp index 6b637ba20cb..b4e94c91849 100644 --- a/lib/Support/APInt.cpp +++ b/lib/Support/APInt.cpp @@ -44,6 +44,35 @@ inline static uint64_t* getMemory(unsigned numWords) { return result; } +/// A utility function that converts a character to a digit. +inline static unsigned getDigit(char cdigit, uint8_t radix) { + // Get a digit + unsigned digit = 0; + if (radix == 16) { + if (!isxdigit(cdigit)) + llvm_unreachable("Invalid hex digit in string"); + if (isdigit(cdigit)) + digit = cdigit - '0'; + else if (cdigit >= 'a') + digit = cdigit - 'a' + 10; + else if (cdigit >= 'A') + digit = cdigit - 'A' + 10; + else + llvm_unreachable("huh? we shouldn't get here"); + } else if (isdigit(cdigit)) { + digit = cdigit - '0'; + assert((radix == 10 || + (radix == 8 && digit != 8 && digit != 9) || + (radix == 2 && (digit == 0 || digit == 1))) && + "Invalid digit in string for given radix"); + } else { + llvm_unreachable("Invalid character in digit string"); + } + + return digit; +} + + void APInt::initSlowCase(unsigned numBits, uint64_t val, bool isSigned) { pVal = getClearedMemory(getNumWords()); pVal[0] = val; @@ -611,22 +640,27 @@ unsigned APInt::getBitsNeeded(const StringRef& str, uint8_t radix) { if (radix == 16) return slen * 4 + isNegative; - // Otherwise it must be radix == 10, the hard case - assert(radix == 10 && "Invalid radix"); - // This is grossly inefficient but accurate. We could probably do something // with a computation of roughly slen*64/20 and then adjust by the value of // the first few digits. But, I'm not sure how accurate that could be. // Compute a sufficient number of bits that is always large enough but might - // be too large. This avoids the assertion in the constructor. - unsigned sufficient = slen*64/18; + // be too large. This avoids the assertion in the constructor. This + // calculation doesn't work appropriately for the numbers 0-9, so just use 4 + // bits in that case. + unsigned sufficient = slen == 1 ? 4 : slen * 64/18; // Convert to the actual binary value. APInt tmp(sufficient, StringRef(p, slen), radix); - // Compute how many bits are required. - return isNegative + tmp.logBase2() + 1; + // Compute how many bits are required. If the log is infinite, assume we need + // just bit. + unsigned log = tmp.logBase2(); + if (log == (unsigned)-1) { + return isNegative + 1; + } else { + return isNegative + log + 1; + } } // From http://www.burtleburtle.net, byBob Jenkins. @@ -2039,29 +2073,7 @@ void APInt::fromString(unsigned numbits, const StringRef& str, uint8_t radix) { // Enter digit traversal loop for (StringRef::iterator e = str.end(); p != e; ++p) { - // Get a digit - unsigned digit = 0; - char cdigit = *p; - if (radix == 16) { - if (!isxdigit(cdigit)) - llvm_unreachable("Invalid hex digit in string"); - if (isdigit(cdigit)) - digit = cdigit - '0'; - else if (cdigit >= 'a') - digit = cdigit - 'a' + 10; - else if (cdigit >= 'A') - digit = cdigit - 'A' + 10; - else - llvm_unreachable("huh? we shouldn't get here"); - } else if (isdigit(cdigit)) { - digit = cdigit - '0'; - assert((radix == 10 || - (radix == 8 && digit != 8 && digit != 9) || - (radix == 2 && (digit == 0 || digit == 1))) && - "Invalid digit in string for given radix"); - } else { - llvm_unreachable("Invalid character in digit string"); - } + unsigned digit = getDigit(*p, radix); // Shift or multiply the value by the radix if (slen > 1) { diff --git a/unittests/ADT/APIntTest.cpp b/unittests/ADT/APIntTest.cpp index 8208af30cd0..0e094ffaefd 100644 --- a/unittests/ADT/APIntTest.cpp +++ b/unittests/ADT/APIntTest.cpp @@ -235,6 +235,88 @@ TEST(APIntTest, fromString) { EXPECT_EQ(APInt(32, -32), APInt(32, "-20", 16)); } +TEST(APIntTest, StringBitsNeeded2) { + EXPECT_EQ(1U, APInt::getBitsNeeded( "0", 2)); + EXPECT_EQ(1U, APInt::getBitsNeeded( "1", 2)); + EXPECT_EQ(2U, APInt::getBitsNeeded( "10", 2)); + EXPECT_EQ(2U, APInt::getBitsNeeded( "11", 2)); + EXPECT_EQ(3U, APInt::getBitsNeeded("100", 2)); + + EXPECT_EQ(1U, APInt::getBitsNeeded( "+0", 2)); + EXPECT_EQ(1U, APInt::getBitsNeeded( "+1", 2)); + EXPECT_EQ(2U, APInt::getBitsNeeded( "+10", 2)); + EXPECT_EQ(2U, APInt::getBitsNeeded( "+11", 2)); + EXPECT_EQ(3U, APInt::getBitsNeeded("+100", 2)); + + EXPECT_EQ(2U, APInt::getBitsNeeded( "-0", 2)); + EXPECT_EQ(2U, APInt::getBitsNeeded( "-1", 2)); + EXPECT_EQ(3U, APInt::getBitsNeeded( "-10", 2)); + EXPECT_EQ(3U, APInt::getBitsNeeded( "-11", 2)); + EXPECT_EQ(4U, APInt::getBitsNeeded("-100", 2)); +} + +TEST(APIntTest, StringBitsNeeded8) { + EXPECT_EQ(3U, APInt::getBitsNeeded( "0", 8)); + EXPECT_EQ(3U, APInt::getBitsNeeded( "7", 8)); + EXPECT_EQ(6U, APInt::getBitsNeeded("10", 8)); + EXPECT_EQ(6U, APInt::getBitsNeeded("17", 8)); + EXPECT_EQ(6U, APInt::getBitsNeeded("20", 8)); + + EXPECT_EQ(3U, APInt::getBitsNeeded( "+0", 8)); + EXPECT_EQ(3U, APInt::getBitsNeeded( "+7", 8)); + EXPECT_EQ(6U, APInt::getBitsNeeded("+10", 8)); + EXPECT_EQ(6U, APInt::getBitsNeeded("+17", 8)); + EXPECT_EQ(6U, APInt::getBitsNeeded("+20", 8)); + + EXPECT_EQ(4U, APInt::getBitsNeeded( "-0", 8)); + EXPECT_EQ(4U, APInt::getBitsNeeded( "-7", 8)); + EXPECT_EQ(7U, APInt::getBitsNeeded("-10", 8)); + EXPECT_EQ(7U, APInt::getBitsNeeded("-17", 8)); + EXPECT_EQ(7U, APInt::getBitsNeeded("-20", 8)); +} + +TEST(APIntTest, StringBitsNeeded10) { + EXPECT_EQ(1U, APInt::getBitsNeeded( "0", 10)); + EXPECT_EQ(2U, APInt::getBitsNeeded( "3", 10)); + EXPECT_EQ(4U, APInt::getBitsNeeded( "9", 10)); + EXPECT_EQ(4U, APInt::getBitsNeeded("10", 10)); + EXPECT_EQ(5U, APInt::getBitsNeeded("19", 10)); + EXPECT_EQ(5U, APInt::getBitsNeeded("20", 10)); + + EXPECT_EQ(1U, APInt::getBitsNeeded( "+0", 10)); + EXPECT_EQ(4U, APInt::getBitsNeeded( "+9", 10)); + EXPECT_EQ(4U, APInt::getBitsNeeded("+10", 10)); + EXPECT_EQ(5U, APInt::getBitsNeeded("+19", 10)); + EXPECT_EQ(5U, APInt::getBitsNeeded("+20", 10)); + + EXPECT_EQ(2U, APInt::getBitsNeeded( "-0", 10)); + EXPECT_EQ(5U, APInt::getBitsNeeded( "-9", 10)); + EXPECT_EQ(5U, APInt::getBitsNeeded("-10", 10)); + EXPECT_EQ(6U, APInt::getBitsNeeded("-19", 10)); + EXPECT_EQ(6U, APInt::getBitsNeeded("-20", 10)); +} + +TEST(APIntTest, StringBitsNeeded16) { + EXPECT_EQ(4U, APInt::getBitsNeeded( "0", 16)); + EXPECT_EQ(4U, APInt::getBitsNeeded( "F", 16)); + EXPECT_EQ(8U, APInt::getBitsNeeded("10", 16)); + EXPECT_EQ(8U, APInt::getBitsNeeded("1F", 16)); + EXPECT_EQ(8U, APInt::getBitsNeeded("20", 16)); + + EXPECT_EQ(4U, APInt::getBitsNeeded( "+0", 16)); + EXPECT_EQ(4U, APInt::getBitsNeeded( "+F", 16)); + EXPECT_EQ(8U, APInt::getBitsNeeded("+10", 16)); + EXPECT_EQ(8U, APInt::getBitsNeeded("+1F", 16)); + EXPECT_EQ(8U, APInt::getBitsNeeded("+20", 16)); + + EXPECT_EQ(5U, APInt::getBitsNeeded( "-0", 16)); + EXPECT_EQ(5U, APInt::getBitsNeeded( "-F", 16)); + EXPECT_EQ(9U, APInt::getBitsNeeded("-10", 16)); + EXPECT_EQ(9U, APInt::getBitsNeeded("-1F", 16)); + EXPECT_EQ(9U, APInt::getBitsNeeded("-20", 16)); +} + + #ifdef GTEST_HAS_DEATH_TEST TEST(APIntTest, StringDeath) { EXPECT_DEATH(APInt(0, "", 0), "Bitwidth too small"); -- 2.34.1