[llvm] [Support][APInt] Fix sign extension, exponent and mantissa in APInt::roundToDouble (PR #192451)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-llvm-adt

Author: Andi Drebes (andidr)

<details>
<summary>Changes</summary>

Conversion of an `APInt` to double via `APInt::roundToDouble` misses several edge cases that result in crashes due to an assertion or in erroneous values due to incorrect values for the exponent and mantissa of the generated double.

The assertion is triggered when attempting to convert a multi-word `APInt` without active bits or with active bits only in the first word, e.g.,

  `APInt(65, 0, true).roundToDouble(true)` or
  `APInt(65, 1, true).roundToDouble(true)`

The issue is caused by passing the bit width as the argument to `SignExtend64`, exceeding the maximum expected bit width of 64.

Incorrect values for the mantissa or exponent are calculated for any multi-word, unsigned value, e.g.,

  `APInt(65, "18446744073709551616" /* 2^64 */, 10).roundToDouble(false)`

This is due to `APInt::roundToDouble` expecting the exponent to correspond to the highest of the fractional part of the double rather than to the highest active bit and due to the missing code clearing the highest bit after generation of the mantissa.

This patch solves the issues by simplifying the treatment of single-word integers and by adjusting the exponent and mantissa for multi-word integers. Tests are added for the edge cases and some regular cases.

---
Full diff: https://github.com/llvm/llvm-project/pull/192451.diff


2 Files Affected:

- (modified) llvm/lib/Support/APInt.cpp (+13-10) 
- (modified) llvm/unittests/ADT/APIntTest.cpp (+30) 


``````````diff
diff --git a/llvm/lib/Support/APInt.cpp b/llvm/lib/Support/APInt.cpp
index 6aa5fc615a302..1d1765a9e178b 100644
--- a/llvm/lib/Support/APInt.cpp
+++ b/llvm/lib/Support/APInt.cpp
@@ -900,12 +900,9 @@ APInt llvm::APIntOps::RoundDoubleToAPInt(double Double, unsigned width) {
 double APInt::roundToDouble(bool isSigned) const {
   // Handle the simple case where the value is contained in one uint64_t.
   // It is wrong to optimize getWord(0) to VAL; there might be more than one word.
-  if (isSingleWord() || getActiveBits() <= APINT_BITS_PER_WORD) {
-    if (isSigned) {
-      int64_t sext = SignExtend64(getWord(0), BitWidth);
-      return double(sext);
-    }
-    return double(getWord(0));
+  if (isSingleWord()) {
+    return isSigned ? double(bit_cast<int64_t>(getWord(0)))
+                    : double(getWord(0));
   }
 
   // Determine if the value is negative.
@@ -917,10 +914,15 @@ double APInt::roundToDouble(bool isSigned) const {
   // Figure out how many bits we're using.
   unsigned n = Tmp.getActiveBits();
 
-  // The exponent (without bias normalization) is just the number of bits
-  // we are using. Note that the sign bit is gone since we constructed the
-  // absolute value.
-  uint64_t exp = n;
+  // Early exit for 0 to avoid negative indexes
+  if (n == 0)
+    return 0.0;
+
+  // The exponent (without bias normalization) is just the number of
+  // bits we are using (minus 1 to account for the fact that the
+  // exponent is on 2). Note that the sign bit is gone since we
+  // constructed the absolute value.
+  uint64_t exp = n-1;
 
   // Return infinity for exponent overflow
   if (exp > 1023) {
@@ -947,6 +949,7 @@ double APInt::roundToDouble(bool isSigned) const {
   }
 
   // The leading bit of mantissa is implicit, so get rid of it.
+  mantissa &= ~(1ULL << std::min(n-1, 51U));
   uint64_t sign = isNeg ? (1ULL << (APINT_BITS_PER_WORD - 1)) : 0;
   uint64_t I = sign | (exp << 52) | mantissa;
   return bit_cast<double>(I);
diff --git a/llvm/unittests/ADT/APIntTest.cpp b/llvm/unittests/ADT/APIntTest.cpp
index ee4c59de34fc4..d5fe5b30b2fd7 100644
--- a/llvm/unittests/ADT/APIntTest.cpp
+++ b/llvm/unittests/ADT/APIntTest.cpp
@@ -3980,4 +3980,34 @@ TEST(APIntTest, clmulh) {
                 .getSExtValue(),
             21845);
 }
+
+TEST(APIntTest, roundToDouble) {
+  // Single-word, positive
+  EXPECT_EQ(APInt(64, 0, false).roundToDouble(false), 0.0);
+  EXPECT_EQ(APInt(64, 1, false).roundToDouble(false), 1.0);
+  EXPECT_EQ(APInt(64, 2, false).roundToDouble(false), 2.0);
+  EXPECT_EQ(APInt(64, 1ULL << 63, false).roundToDouble(false), 9223372036854775808.0);
+
+  // Single-word, negative
+  EXPECT_EQ(APInt(64, 0, true).roundToDouble(true), 0.0);
+  EXPECT_EQ(APInt(64, -1, true).roundToDouble(true), -1.0);
+  EXPECT_EQ(APInt(64, -2, true).roundToDouble(true), -2.0);
+  EXPECT_EQ(APInt(64, 1ULL << 63, true).roundToDouble(true), -9223372036854775808.0);
+
+  // Multi-word, positive, active bits in first word
+  EXPECT_EQ(APInt(65, 0, false).roundToDouble(false), 0.0);
+  EXPECT_EQ(APInt(65, 1, false).roundToDouble(false), 1.0);
+  EXPECT_EQ(APInt(65, 2, false).roundToDouble(false), 2.0);
+  EXPECT_EQ(APInt(65, 1ULL << 63, false).roundToDouble(true), 9223372036854775808.0);
+
+  // Multi-word, positive, active bits outside first word
+  EXPECT_EQ(APInt(65, "18446744073709551616" /* 2^64 */, 10).roundToDouble(false), 18446744073709551616.0);
+
+  // Multi-word, negative
+  EXPECT_EQ(APInt(65, 0, true).roundToDouble(true), 0.0);
+
+  EXPECT_EQ(APInt(65, -1, true).roundToDouble(true), -1.0);
+  EXPECT_EQ(APInt(65, -2, true).roundToDouble(true), -2.0);
+  EXPECT_EQ(APInt(65, "18446744073709551616" /* 2^64 */, 10).roundToDouble(true), -18446744073709551616.0);
+}
 } // end anonymous namespace

``````````

</details>


https://github.com/llvm/llvm-project/pull/192451