Skip to content

Commit 0dec30d

Browse files
beetreesbeetrees
committed
[APFloat] Fix IEEEFloat::addOrSubtractSignificand and IEEEFloat::normalize 
1 parent 7e3187e commit 0dec30d

File tree

3 files changed

+216
-15
lines changed

3 files changed

+216
-15
lines changed

‎llvm/include/llvm/ADT/APFloat.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,8 @@ class IEEEFloat final {
750750

751751
/// Sign bit of the number.
752752
unsigned int sign : 1;
753+
754+
friend class IEEEFloatUnitTestHelper;
753755
};
754756

755757
hash_code hash_value(const IEEEFloat &Arg);

‎llvm/lib/Support/APFloat.cpp

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,7 +1682,8 @@ APFloat::opStatus IEEEFloat::normalize(roundingMode rounding_mode,
16821682
/* Before rounding normalize the exponent of fcNormal numbers. */
16831683
omsb = significandMSB() + 1;
16841684

1685-
if (omsb) {
1685+
// Only skip this `if` if the value is exactly zero.
1686+
if (omsb || lost_fraction != lfExactlyZero) {
16861687
/* OMSB is numbered from 1. We want to place it in the integer
16871688
bit numbered PRECISION if possible, with a compensating change in
16881689
the exponent. */
@@ -1864,7 +1865,7 @@ APFloat::opStatus IEEEFloat::addOrSubtractSpecials(const IEEEFloat &rhs,
18641865
/* Add or subtract two normal numbers. */
18651866
lostFraction IEEEFloat::addOrSubtractSignificand(const IEEEFloat &rhs,
18661867
bool subtract) {
1867-
integerPart carry;
1868+
integerPart carry = 0;
18681869
lostFraction lost_fraction;
18691870
int bits;
18701871

@@ -1882,35 +1883,54 @@ lostFraction IEEEFloat::addOrSubtractSignificand(const IEEEFloat &rhs,
18821883
"This floating point format does not support signed values");
18831884

18841885
IEEEFloat temp_rhs(rhs);
1886+
bool lost_fraction_is_from_rhs = false;
18851887

18861888
if (bits == 0)
18871889
lost_fraction = lfExactlyZero;
18881890
else if (bits > 0) {
18891891
lost_fraction = temp_rhs.shiftSignificandRight(bits - 1);
1892+
lost_fraction_is_from_rhs = true;
18901893
shiftSignificandLeft(1);
18911894
} else {
18921895
lost_fraction = shiftSignificandRight(-bits - 1);
18931896
temp_rhs.shiftSignificandLeft(1);
18941897
}
18951898

18961899
// Should we reverse the subtraction.
1897-
if (compareAbsoluteValue(temp_rhs) == cmpLessThan) {
1898-
carry = temp_rhs.subtractSignificand
1899-
(*this, lost_fraction != lfExactlyZero);
1900+
cmpResult cmp_result = compareAbsoluteValue(temp_rhs);
1901+
if (cmp_result == cmpLessThan) {
1902+
bool borrow =
1903+
lost_fraction != lfExactlyZero && !lost_fraction_is_from_rhs;
1904+
if (borrow) {
1905+
// The lost fraction is being subtracted, borrow from the significand
1906+
// and invert `lost_fraction`.
1907+
if (lost_fraction == lfLessThanHalf)
1908+
lost_fraction = lfMoreThanHalf;
1909+
else if (lost_fraction == lfMoreThanHalf)
1910+
lost_fraction = lfLessThanHalf;
1911+
}
1912+
carry = temp_rhs.subtractSignificand(*this, borrow);
19001913
copySignificand(temp_rhs);
19011914
sign = !sign;
1902-
} else {
1903-
carry = subtractSignificand
1904-
(temp_rhs, lost_fraction != lfExactlyZero);
1915+
} else if (cmp_result == cmpGreaterThan) {
1916+
bool borrow = lost_fraction != lfExactlyZero && lost_fraction_is_from_rhs;
1917+
if (borrow) {
1918+
// The lost fraction is being subtracted, borrow from the significand
1919+
// and invert `lost_fraction`.
1920+
if (lost_fraction == lfLessThanHalf)
1921+
lost_fraction = lfMoreThanHalf;
1922+
else if (lost_fraction == lfMoreThanHalf)
1923+
lost_fraction = lfLessThanHalf;
1924+
}
1925+
carry = subtractSignificand(temp_rhs, borrow);
1926+
} else { // cmpEqual
1927+
zeroSignificand();
1928+
if (lost_fraction != lfExactlyZero && lost_fraction_is_from_rhs) {
1929+
// rhs is slightly larger due to the lost fraction, flip the sign.
1930+
sign = !sign;
1931+
}
19051932
}
19061933

1907-
/* Invert the lost fraction - it was on the RHS and
1908-
subtracted. */
1909-
if (lost_fraction == lfLessThanHalf)
1910-
lost_fraction = lfMoreThanHalf;
1911-
else if (lost_fraction == lfMoreThanHalf)
1912-
lost_fraction = lfLessThanHalf;
1913-
19141934
/* The code above is intended to ensure that no borrow is
19151935
necessary. */
19161936
assert(!carry);

‎llvm/unittests/ADT/APFloatTest.cpp

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,37 @@ static std::string convertToString(double d, unsigned Prec, unsigned Pad,
4747
return std::string(Buffer.data(), Buffer.size());
4848
}
4949

50+
namespace llvm {
51+
namespace detail {
52+
class IEEEFloatUnitTestHelper {
53+
public:
54+
static void runTest(bool subtract, bool lhsSign,
55+
APFloat::ExponentType lhsExponent,
56+
APFloat::integerPart lhsSignificand, bool rhsSign,
57+
APFloat::ExponentType rhsExponent,
58+
APFloat::integerPart rhsSignificand, bool expectedSign,
59+
APFloat::ExponentType expectedExponent,
60+
APFloat::integerPart expectedSignificand,
61+
lostFraction expectedLoss) {
62+
// `addOrSubtractSignificand` only uses the sign, exponent and significand
63+
IEEEFloat lhs(1.0);
64+
lhs.sign = lhsSign;
65+
lhs.exponent = lhsExponent;
66+
lhs.significand.part = lhsSignificand;
67+
IEEEFloat rhs(1.0);
68+
rhs.sign = rhsSign;
69+
rhs.exponent = rhsExponent;
70+
rhs.significand.part = rhsSignificand;
71+
lostFraction resultLoss = lhs.addOrSubtractSignificand(rhs, subtract);
72+
EXPECT_EQ(resultLoss, expectedLoss);
73+
EXPECT_EQ(lhs.sign, expectedSign);
74+
EXPECT_EQ(lhs.exponent, expectedExponent);
75+
EXPECT_EQ(lhs.significand.part, expectedSignificand);
76+
}
77+
};
78+
} // namespace detail
79+
} // namespace llvm
80+
5081
namespace {
5182

5283
TEST(APFloatTest, isSignaling) {
@@ -560,6 +591,104 @@ TEST(APFloatTest, FMA) {
560591
EXPECT_EQ(-8.85242279E-41f, f1.convertToFloat());
561592
}
562593

594+
// The `addOrSubtractSignificand` can be considered to have 9 possible cases
595+
// when subtracting: all combinations of {cmpLessThan, cmpGreaterThan,
596+
// cmpEqual} and {no loss, loss from lhs, loss from rhs}. Test each reachable
597+
// case here.
598+
599+
// Regression test for failing the `assert(!carry)` in
600+
// `addOrSubtractSignificand` and normalizing the exponent even when the
601+
// significand is zero if there is a lost fraction.
602+
// This tests cmpEqual, loss from lhs
603+
{
604+
APFloat f1(-1.4728589E-38f);
605+
APFloat f2(3.7105144E-6f);
606+
APFloat f3(5.5E-44f);
607+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
608+
EXPECT_EQ(-0.0f, f1.convertToFloat());
609+
}
610+
611+
// Test cmpGreaterThan, no loss
612+
{
613+
APFloat f1(2.0f);
614+
APFloat f2(2.0f);
615+
APFloat f3(-3.5f);
616+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
617+
EXPECT_EQ(0.5f, f1.convertToFloat());
618+
}
619+
620+
// Test cmpLessThan, no loss
621+
{
622+
APFloat f1(2.0f);
623+
APFloat f2(2.0f);
624+
APFloat f3(-4.5f);
625+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
626+
EXPECT_EQ(-0.5f, f1.convertToFloat());
627+
}
628+
629+
// Test cmpEqual, no loss
630+
{
631+
APFloat f1(2.0f);
632+
APFloat f2(2.0f);
633+
APFloat f3(-4.0f);
634+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
635+
EXPECT_EQ(0.0f, f1.convertToFloat());
636+
}
637+
638+
// Test cmpLessThan, loss from lhs
639+
{
640+
APFloat f1(2.0000002f);
641+
APFloat f2(2.0000002f);
642+
APFloat f3(-32.0f);
643+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
644+
EXPECT_EQ(-27.999998f, f1.convertToFloat());
645+
}
646+
647+
// Test cmpGreaterThan, loss from rhs
648+
{
649+
APFloat f1(1e10f);
650+
APFloat f2(1e10f);
651+
APFloat f3(-2.0000002f);
652+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
653+
EXPECT_EQ(1e20f, f1.convertToFloat());
654+
}
655+
656+
// Test cmpGreaterThan, loss from lhs
657+
{
658+
APFloat f1(1e-36f);
659+
APFloat f2(0.0019531252f);
660+
APFloat f3(-1e-45f);
661+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
662+
EXPECT_EQ(1.953124e-39f, f1.convertToFloat());
663+
}
664+
665+
// {cmpEqual, cmpLessThan} with loss from rhs can't occur for the usage in
666+
// `fusedMultiplyAdd` as `multiplySignificand` normalises the MSB of lhs to
667+
// one bit below the top.
668+
669+
// Test cases from #104984
670+
{
671+
APFloat f1(0.24999998f);
672+
APFloat f2(2.3509885e-38f);
673+
APFloat f3(-1e-45f);
674+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
675+
EXPECT_EQ(5.87747e-39f, f1.convertToFloat());
676+
}
677+
{
678+
APFloat f1(4.4501477170144023e-308);
679+
APFloat f2(0.24999999999999997);
680+
APFloat f3(-8.475904604373977e-309);
681+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
682+
EXPECT_EQ(2.64946468816203e-309, f1.convertToDouble());
683+
}
684+
{
685+
APFloat f1(APFloat::IEEEhalf(), APInt(16, 0x8fffu));
686+
APFloat f2(APFloat::IEEEhalf(), APInt(16, 0x2bffu));
687+
APFloat f3(APFloat::IEEEhalf(), APInt(16, 0x0172u));
688+
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
689+
EXPECT_EQ(0x808eu, f1.bitcastToAPInt().getZExtValue());
690+
}
691+
563692
// Test using only a single instance of APFloat.
564693
{
565694
APFloat F(1.5);
@@ -8089,4 +8218,54 @@ TEST(APFloatTest, Float4E2M1FNToFloat) {
80898218
EXPECT_TRUE(SmallestDenorm.isDenormal());
80908219
EXPECT_EQ(0x0.8p0, SmallestDenorm.convertToFloat());
80918220
}
8221+
8222+
TEST(APFloatTest, AddOrSubtractSignificand) {
8223+
typedef detail::IEEEFloatUnitTestHelper Helper;
8224+
// Test cases are all combinations of:
8225+
// {equal exponents, LHS larger exponent, RHS larger exponent}
8226+
// {equal significands, LHS larger significand, RHS larger significand}
8227+
// {no loss, loss}
8228+
8229+
// Equal exponents (loss cannot occur as their is no shifting)
8230+
Helper::runTest(true, false, 1, 0x10, false, 1, 0x5, false, 1, 0xb,
8231+
lfExactlyZero);
8232+
Helper::runTest(false, false, -2, 0x20, true, -2, 0x20, false, -2, 0,
8233+
lfExactlyZero);
8234+
Helper::runTest(false, true, 3, 0x20, false, 3, 0x30, false, 3, 0x10,
8235+
lfExactlyZero);
8236+
8237+
// LHS larger exponent
8238+
// LHS significand greater after shitfing
8239+
Helper::runTest(true, false, 7, 0x100, false, 3, 0x100, false, 6, 0x1e0,
8240+
lfExactlyZero);
8241+
Helper::runTest(true, false, 7, 0x100, false, 3, 0x101, false, 6, 0x1df,
8242+
lfMoreThanHalf);
8243+
// Significands equal after shitfing
8244+
Helper::runTest(true, false, 7, 0x100, false, 3, 0x1000, false, 6, 0,
8245+
lfExactlyZero);
8246+
Helper::runTest(true, false, 7, 0x100, false, 3, 0x1001, true, 6, 0,
8247+
lfLessThanHalf);
8248+
// RHS significand greater after shitfing
8249+
Helper::runTest(true, false, 7, 0x100, false, 3, 0x10000, true, 6, 0x1e00,
8250+
lfExactlyZero);
8251+
Helper::runTest(true, false, 7, 0x100, false, 3, 0x10001, true, 6, 0x1e00,
8252+
lfLessThanHalf);
8253+
8254+
// RHS larger exponent
8255+
// RHS significand greater after shitfing
8256+
Helper::runTest(true, false, 3, 0x100, false, 7, 0x100, true, 6, 0x1e0,
8257+
lfExactlyZero);
8258+
Helper::runTest(true, false, 3, 0x101, false, 7, 0x100, true, 6, 0x1df,
8259+
lfMoreThanHalf);
8260+
// Significands equal after shitfing
8261+
Helper::runTest(true, false, 3, 0x1000, false, 7, 0x100, false, 6, 0,
8262+
lfExactlyZero);
8263+
Helper::runTest(true, false, 3, 0x1001, false, 7, 0x100, false, 6, 0,
8264+
lfLessThanHalf);
8265+
// LHS significand greater after shitfing
8266+
Helper::runTest(true, false, 3, 0x10000, false, 7, 0x100, false, 6, 0x1e00,
8267+
lfExactlyZero);
8268+
Helper::runTest(true, false, 3, 0x10001, false, 7, 0x100, false, 6, 0x1e00,
8269+
lfLessThanHalf);
8270+
}
80928271
} // namespace

0 commit comments

Comments
 (0)