[llvm] acef1db - [APFloat] Remove some overly optimistic assertions

[David Majnemer via llvm-commits]

Author: David Majnemer
Date: 2025-08-12T18:32:58-07:00
New Revision: acef1db3b2dbc59e4c8f5fa9c167917a2dd207b0

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

LOG: [APFloat] Remove some overly optimistic assertions

An earlier draft of DoubleAPFloat::convertToSignExtendedInteger had
arranged for overflow to be handled in a different way.  However, these
assertions are now possible if Hi+Lo are out of range and Lo != 0.

A test has been added to defend against a regression.

Added: 
    

Modified: 
    llvm/lib/Support/APFloat.cpp
    llvm/unittests/ADT/APFloatTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 529d00201b9d4..d2a417ff87915 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -5536,7 +5536,7 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
   DoubleAPFloat Integral = *this;
   const opStatus RoundStatus = Integral.roundToIntegral(RM);
   if (RoundStatus == opInvalidOp)
-    return RoundStatus;
+    return opInvalidOp;
   const APFloat &IntegralHi = Integral.getFirst();
   const APFloat &IntegralLo = Integral.getSecond();
 
@@ -5548,7 +5548,7 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
         IntegralHi.convertToInteger(Input, Width, IsSigned, RM, &HiIsExact);
     // The conversion from an integer-valued float to an APInt may fail if the
     // result would be out of range.  Regardless, taking this path is only
-    // possible if rounding occured during the initial `roundToIntegral`.
+    // possible if rounding occurred during the initial `roundToIntegral`.
     return HiStatus == opOK ? opInexact : HiStatus;
   }
 
@@ -5575,9 +5575,10 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
     // If the signs 
diff er, the sum will fit. We can compute the result using
     // properties of two's complement arithmetic without a wide intermediate
     // integer. E.g., for uint128_t, (2^128, -1) should be 2^128 - 1.
-    [[maybe_unused]] opStatus LoStatus = IntegralLo.convertToInteger(
+    const opStatus LoStatus = IntegralLo.convertToInteger(
         Input, Width, /*IsSigned=*/true, RM, &LoIsExact);
-    assert(LoStatus == opOK && "Unexpected failure");
+    if (LoStatus == opInvalidOp)
+      return opInvalidOp;
 
     // Adjust the bit pattern of Lo to account for Hi's value:
     //  - For unsigned (Hi=2^Width): `2^Width + Lo` in `Width`-bit
@@ -5592,18 +5593,19 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
     return RoundStatus;
   }
 
-  // General case: Hi is not a power-of-two boundary, so we know it fits.
-  // Since we already rounded the full value, we now just need to convert the
-  // components to integers.  The rounding mode should not matter.
-  [[maybe_unused]] opStatus HiStatus = IntegralHi.convertToInteger(
+  // Convert Hi into an integer.  This may not fit but that is OK: we know that
+  // Hi + Lo would not fit either in this situation.
+  const opStatus HiStatus = IntegralHi.convertToInteger(
       Input, Width, IsSigned, rmTowardZero, &HiIsExact);
-  assert(HiStatus == opOK && "Unexpected failure");
+  if (HiStatus == opInvalidOp)
+    return HiStatus;
 
   // Convert Lo into a temporary integer of the same width.
   APSInt LoResult{Width, /*isUnsigned=*/!IsSigned};
-  [[maybe_unused]] opStatus LoStatus =
+  const opStatus LoStatus =
       IntegralLo.convertToInteger(LoResult, rmTowardZero, &LoIsExact);
-  assert(LoStatus == opOK && "Unexpected failure");
+  if (LoStatus == opInvalidOp)
+    return LoStatus;
 
   // Add Lo to Hi. This addition is guaranteed not to overflow because of the
   // double-double canonicalization rule (`|Lo| <= ulp(Hi)/2`). The only case

diff  --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index 29299cafcb014..b518ef854d9a1 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -6093,6 +6093,10 @@ auto testCases() {
       TestCase{{DMAX, 0.0}, 128, true}.withAll(
           "170141183460469231731687303715884105727", Invalid),
 
+      // Input: Largest semPPCDoubleDoubleLegacy
+      TestCase{{DMAX, 0x1.ffffffffffffep+969}, 128, true}.withAll(
+          "170141183460469231731687303715884105727", Invalid),
+
       // 7. Round to negative -0
       TestCase{{-PM100, 0.0}, 32, true}
           .with(RTZ, "0", Inexact)