[llvm] [APFloat] Fix literals with long significands. (PR #102051)

via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 17 12:36:23 PDT 2024


https://github.com/keinflue updated https://github.com/llvm/llvm-project/pull/102051

>From 4074828b5a6fd12ecf9a30f6183dceaff79caac2 Mon Sep 17 00:00:00 2001
From: keinflue <80230456+keinflue at users.noreply.github.com>
Date: Mon, 5 Aug 2024 21:22:30 +0200
Subject: [PATCH 1/3] [APFloat] Fix literals with long significands.

Replace fixed-size arrays in powerOf5 with SmallVector to support
significands longer than about 10000 digits.

The new limit should be around UINT_MAX/815, practically limited by
processing time.

Fixes #28406
---
 llvm/lib/Support/APFloat.cpp | 92 ++++++++++++++++++++----------------
 1 file changed, 51 insertions(+), 41 deletions(-)

diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 7f68c5ab9b7cf7..0f91953cca3012 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/llvm-config.h"
@@ -27,6 +28,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include <cstring>
 #include <limits.h>
+#include <limits>
 
 #define APFLOAT_DISPATCH_ON_SEMANTICS(METHOD_CALL)                             \
   do {                                                                         \
@@ -308,24 +310,6 @@ constexpr RoundingMode APFloatBase::rmTowardNegative;
 constexpr RoundingMode APFloatBase::rmTowardZero;
 constexpr RoundingMode APFloatBase::rmNearestTiesToAway;
 
-/* A tight upper bound on number of parts required to hold the value
-   pow(5, power) is
-
-     power * 815 / (351 * integerPartWidth) + 1
-
-   However, whilst the result may require only this many parts,
-   because we are multiplying two values to get it, the
-   multiplication may require an extra part with the excess part
-   being zero (consider the trivial case of 1 * 1, tcFullMultiply
-   requires two parts to hold the single-part result).  So we add an
-   extra one to guarantee enough space whilst multiplying.  */
-const unsigned int maxExponent = 16383;
-const unsigned int maxPrecision = 113;
-const unsigned int maxPowerOfFiveExponent = maxExponent + maxPrecision - 1;
-const unsigned int maxPowerOfFiveParts =
-    2 +
-    ((maxPowerOfFiveExponent * 815) / (351 * APFloatBase::integerPartWidth));
-
 unsigned int APFloatBase::semanticsPrecision(const fltSemantics &semantics) {
   return semantics.precision;
 }
@@ -763,25 +747,44 @@ ulpsFromBoundary(const APFloatBase::integerPart *parts, unsigned int bits,
 
 /* Place pow(5, power) in DST, and return the number of parts used.
    DST must be at least one part larger than size of the answer.  */
-static unsigned int
-powerOf5(APFloatBase::integerPart *dst, unsigned int power) {
+static SmallVector<APFloatBase::integerPart, 0> powerOf5(unsigned int power) {
   static const APFloatBase::integerPart firstEightPowers[] = { 1, 5, 25, 125, 625, 3125, 15625, 78125 };
-  APFloatBase::integerPart pow5s[maxPowerOfFiveParts * 2 + 5];
+
+  // Make sure that the following expression does not overflow.
+  assert(power <= UINT_MAX / 815);
+
+  // A tight upper bound on number of parts required to hold the value
+  // pow(5, power) is
+  //
+  //   power * 815 / (351 * integerPartWidth) + 1
+  //
+  // However, whilst the result may require only this many parts,
+  // because we are multiplying two values to get it, the
+  // multiplication may require an extra part with the excess part
+  // being zero (consider the trivial case of 1 * 1, tcFullMultiply
+  // requires two parts to hold the single-part result).  So we add an
+  // extra one to guarantee enough space whilst multiplying.
+  const unsigned int maxPowerOfFiveParts =
+      2 + ((power * 815) / (351 * APFloatBase::integerPartWidth));
+
+  SmallVector<APFloatBase::integerPart, 0> pow5s(maxPowerOfFiveParts * 2 + 5);
   pow5s[0] = 78125 * 5;
 
   unsigned int partsCount = 1;
-  APFloatBase::integerPart scratch[maxPowerOfFiveParts], *p1, *p2, *pow5;
-  unsigned int result;
-  assert(power <= maxExponent);
+  SmallVector<APFloatBase::integerPart, 0> scratch(maxPowerOfFiveParts);
+  APFloatBase::integerPart *p1, *p2, *pow5;
+  unsigned int resultSize;
+
+  SmallVector<APFloatBase::integerPart, 0> dst(maxPowerOfFiveParts);
 
-  p1 = dst;
-  p2 = scratch;
+  p1 = dst.data();
+  p2 = scratch.data();
 
   *p1 = firstEightPowers[power & 7];
   power >>= 3;
 
-  result = 1;
-  pow5 = pow5s;
+  resultSize = 1;
+  pow5 = pow5s.data();
 
   for (unsigned int n = 0; power; power >>= 1, n++) {
     /* Calculate pow(5,pow(2,n+3)) if we haven't yet.  */
@@ -796,10 +799,10 @@ powerOf5(APFloatBase::integerPart *dst, unsigned int power) {
     if (power & 1) {
       APFloatBase::integerPart *tmp;
 
-      APInt::tcFullMultiply(p2, p1, pow5, result, partsCount);
-      result += partsCount;
-      if (p2[result - 1] == 0)
-        result--;
+      APInt::tcFullMultiply(p2, p1, pow5, resultSize, partsCount);
+      resultSize += partsCount;
+      if (p2[resultSize - 1] == 0)
+        resultSize--;
 
       /* Now result is in p1 with partsCount parts and p2 is scratch
          space.  */
@@ -811,10 +814,14 @@ powerOf5(APFloatBase::integerPart *dst, unsigned int power) {
     pow5 += partsCount;
   }
 
-  if (p1 != dst)
-    APInt::tcAssign(dst, p1, result);
+  if (p1 != dst.data())
+    APInt::tcAssign(dst.data(), p1, resultSize);
 
-  return result;
+  // Truncate to the actually used elements from the initial worst-case sizing
+  // during initialization.
+  dst.truncate(resultSize);
+
+  return dst;
 }
 
 /* Zero at the end to avoid modular arithmetic when adding one; used
@@ -2950,9 +2957,9 @@ IEEEFloat::opStatus
 IEEEFloat::roundSignificandWithExponent(const integerPart *decSigParts,
                                         unsigned sigPartCount, int exp,
                                         roundingMode rounding_mode) {
-  unsigned int parts, pow5PartCount;
-  fltSemantics calcSemantics = { 32767, -32767, 0, 0 };
-  integerPart pow5Parts[maxPowerOfFiveParts];
+  unsigned int parts;
+  fltSemantics calcSemantics = {std::numeric_limits<ExponentType>::max(),
+                                std::numeric_limits<ExponentType>::min(), 0, 0};
   bool isNearest;
 
   isNearest = (rounding_mode == rmNearestTiesToEven ||
@@ -2960,8 +2967,11 @@ IEEEFloat::roundSignificandWithExponent(const integerPart *decSigParts,
 
   parts = partCountForBits(semantics->precision + 11);
 
+  // Make sure that abs(exp) is representable.
+  assert(exp > INT_MIN);
+
   /* Calculate pow(5, abs(exp)).  */
-  pow5PartCount = powerOf5(pow5Parts, exp >= 0 ? exp: -exp);
+  SmallVector<integerPart, 0> pow5Parts = powerOf5(exp >= 0 ? exp : -exp);
 
   for (;; parts *= 2) {
     opStatus sigStatus, powStatus;
@@ -2977,8 +2987,8 @@ IEEEFloat::roundSignificandWithExponent(const integerPart *decSigParts,
 
     sigStatus = decSig.convertFromUnsignedParts(decSigParts, sigPartCount,
                                                 rmNearestTiesToEven);
-    powStatus = pow5.convertFromUnsignedParts(pow5Parts, pow5PartCount,
-                                              rmNearestTiesToEven);
+    powStatus = pow5.convertFromUnsignedParts(
+        pow5Parts.data(), pow5Parts.size(), rmNearestTiesToEven);
     /* Add exp, as 10^n = 5^n * 2^n.  */
     decSig.exponent += exp;
 

>From 52d1b9ada6a32102c19b7f6fc30fe25fa658aa31 Mon Sep 17 00:00:00 2001
From: keinflue <80230456+keinflue at users.noreply.github.com>
Date: Sat, 17 Aug 2024 21:08:09 +0200
Subject: [PATCH 2/3] fixup! [APFloat] Fix literals with long significands.

---
 llvm/lib/Support/APFloat.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 0f91953cca3012..6ee7cc34893ada 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -745,8 +745,7 @@ ulpsFromBoundary(const APFloatBase::integerPart *parts, unsigned int bits,
   return ~(APFloatBase::integerPart) 0; /* A lot.  */
 }
 
-/* Place pow(5, power) in DST, and return the number of parts used.
-   DST must be at least one part larger than size of the answer.  */
+/* Calculate and return pow(5, power). */
 static SmallVector<APFloatBase::integerPart, 0> powerOf5(unsigned int power) {
   static const APFloatBase::integerPart firstEightPowers[] = { 1, 5, 25, 125, 625, 3125, 15625, 78125 };
 

>From e5cb5e7e0003cb89e9db244d19aab3eabfdd4598 Mon Sep 17 00:00:00 2001
From: keinflue <80230456+keinflue at users.noreply.github.com>
Date: Sat, 17 Aug 2024 21:12:22 +0200
Subject: [PATCH 3/3] [APFloat] Add unit test case for #28406.

---
 llvm/unittests/ADT/APFloatTest.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index be675bb7fe5a53..ee5d51235882de 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -1122,6 +1122,14 @@ TEST(APFloatTest, fromDecimalString) {
   EXPECT_EQ(2.71828, convertToDoubleFromString("2.71828"));
 }
 
+TEST(APFloatTest, fromDecimalStringLongSignificand) {
+  APFloat f(0.0);
+  auto statusOrError = f.convertFromString("0." + std::string(17000, '9'),
+                                           APFloat::rmNearestTiesToEven);
+  EXPECT_FALSE(!statusOrError);
+  EXPECT_EQ(1.0, f.convertToDouble());
+}
+
 TEST(APFloatTest, fromStringSpecials) {
   const fltSemantics &Sem = APFloat::IEEEdouble();
   const unsigned Precision = 53;



More information about the llvm-commits mailing list