Skip to content

Commit acef1db

Browse files
committed
[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.
1 parent 331a5db commit acef1db

File tree

2 files changed

+17
-11
lines changed

2 files changed

+17
-11
lines changed

llvm/lib/Support/APFloat.cpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5536,7 +5536,7 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
55365536
DoubleAPFloat Integral = *this;
55375537
const opStatus RoundStatus = Integral.roundToIntegral(RM);
55385538
if (RoundStatus == opInvalidOp)
5539-
return RoundStatus;
5539+
return opInvalidOp;
55405540
const APFloat &IntegralHi = Integral.getFirst();
55415541
const APFloat &IntegralLo = Integral.getSecond();
55425542

@@ -5548,7 +5548,7 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
55485548
IntegralHi.convertToInteger(Input, Width, IsSigned, RM, &HiIsExact);
55495549
// The conversion from an integer-valued float to an APInt may fail if the
55505550
// result would be out of range. Regardless, taking this path is only
5551-
// possible if rounding occured during the initial `roundToIntegral`.
5551+
// possible if rounding occurred during the initial `roundToIntegral`.
55525552
return HiStatus == opOK ? opInexact : HiStatus;
55535553
}
55545554

@@ -5575,9 +5575,10 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
55755575
// If the signs differ, the sum will fit. We can compute the result using
55765576
// properties of two's complement arithmetic without a wide intermediate
55775577
// integer. E.g., for uint128_t, (2^128, -1) should be 2^128 - 1.
5578-
[[maybe_unused]] opStatus LoStatus = IntegralLo.convertToInteger(
5578+
const opStatus LoStatus = IntegralLo.convertToInteger(
55795579
Input, Width, /*IsSigned=*/true, RM, &LoIsExact);
5580-
assert(LoStatus == opOK && "Unexpected failure");
5580+
if (LoStatus == opInvalidOp)
5581+
return opInvalidOp;
55815582

55825583
// Adjust the bit pattern of Lo to account for Hi's value:
55835584
// - For unsigned (Hi=2^Width): `2^Width + Lo` in `Width`-bit
@@ -5592,18 +5593,19 @@ APFloat::opStatus DoubleAPFloat::convertToSignExtendedInteger(
55925593
return RoundStatus;
55935594
}
55945595

5595-
// General case: Hi is not a power-of-two boundary, so we know it fits.
5596-
// Since we already rounded the full value, we now just need to convert the
5597-
// components to integers. The rounding mode should not matter.
5598-
[[maybe_unused]] opStatus HiStatus = IntegralHi.convertToInteger(
5596+
// Convert Hi into an integer. This may not fit but that is OK: we know that
5597+
// Hi + Lo would not fit either in this situation.
5598+
const opStatus HiStatus = IntegralHi.convertToInteger(
55995599
Input, Width, IsSigned, rmTowardZero, &HiIsExact);
5600-
assert(HiStatus == opOK && "Unexpected failure");
5600+
if (HiStatus == opInvalidOp)
5601+
return HiStatus;
56015602

56025603
// Convert Lo into a temporary integer of the same width.
56035604
APSInt LoResult{Width, /*isUnsigned=*/!IsSigned};
5604-
[[maybe_unused]] opStatus LoStatus =
5605+
const opStatus LoStatus =
56055606
IntegralLo.convertToInteger(LoResult, rmTowardZero, &LoIsExact);
5606-
assert(LoStatus == opOK && "Unexpected failure");
5607+
if (LoStatus == opInvalidOp)
5608+
return LoStatus;
56075609

56085610
// Add Lo to Hi. This addition is guaranteed not to overflow because of the
56095611
// double-double canonicalization rule (`|Lo| <= ulp(Hi)/2`). The only case

llvm/unittests/ADT/APFloatTest.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6093,6 +6093,10 @@ auto testCases() {
60936093
TestCase{{DMAX, 0.0}, 128, true}.withAll(
60946094
"170141183460469231731687303715884105727", Invalid),
60956095

6096+
// Input: Largest semPPCDoubleDoubleLegacy
6097+
TestCase{{DMAX, 0x1.ffffffffffffep+969}, 128, true}.withAll(
6098+
"170141183460469231731687303715884105727", Invalid),
6099+
60966100
// 7. Round to negative -0
60976101
TestCase{{-PM100, 0.0}, 32, true}
60986102
.with(RTZ, "0", Inexact)

0 commit comments

Comments
 (0)