Skip to content

Commit

Permalink
[APFloat] Fix IEEEFloat::addOrSubtractSignificand and `IEEEFloat::n…
Browse files Browse the repository at this point in the history 
…ormalize`
  • Loading branch information
@beetrees
beetrees committed Nov 25, 2024
1 parent 7e3187e commit eabd142
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 16 deletions.
9 changes: 8 additions & 1 deletion llvm/include/llvm/ADT/APFloat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,8 @@ static constexpr fltCategory fcNaN = APFloatBase::fcNaN;
static constexpr fltCategory fcNormal = APFloatBase::fcNormal;
static constexpr fltCategory fcZero = APFloatBase::fcZero;

class IEEEFloat final {
// Not final for testing purposes only.
class IEEEFloat {
public:
/// \name Constructors
/// @{
Expand Down Expand Up @@ -623,7 +624,11 @@ class IEEEFloat final {

integerPart addSignificand(const IEEEFloat &);
integerPart subtractSignificand(const IEEEFloat &, integerPart);
// Not private for testing purposes only.
protected:
lostFraction addOrSubtractSignificand(const IEEEFloat &, bool subtract);

private:
lostFraction multiplySignificand(const IEEEFloat &, IEEEFloat,
bool ignoreAddend = false);
lostFraction multiplySignificand(const IEEEFloat&);
Expand Down Expand Up @@ -727,6 +732,8 @@ class IEEEFloat final {
void copySignificand(const IEEEFloat &);
void freeSignificand();

// Not private for testing purposes only.
protected:
/// Note: this must be the first data member.
/// The semantics that this value obeys.
const fltSemantics *semantics;
Expand Down
50 changes: 35 additions & 15 deletions llvm/lib/Support/APFloat.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1682,7 +1682,8 @@ APFloat::opStatus IEEEFloat::normalize(roundingMode rounding_mode,
/* Before rounding normalize the exponent of fcNormal numbers. */
omsb = significandMSB() + 1;

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

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

IEEEFloat temp_rhs(rhs);
bool lost_fraction_is_from_rhs = false;

if (bits == 0)
lost_fraction = lfExactlyZero;
else if (bits > 0) {
lost_fraction = temp_rhs.shiftSignificandRight(bits - 1);
lost_fraction_is_from_rhs = true;
shiftSignificandLeft(1);
} else {
lost_fraction = shiftSignificandRight(-bits - 1);
temp_rhs.shiftSignificandLeft(1);
}

// Should we reverse the subtraction.
if (compareAbsoluteValue(temp_rhs) == cmpLessThan) {
carry = temp_rhs.subtractSignificand
(*this, lost_fraction != lfExactlyZero);
cmpResult cmp_result = compareAbsoluteValue(temp_rhs);
if (cmp_result == cmpLessThan) {
bool borrow =
lost_fraction != lfExactlyZero && !lost_fraction_is_from_rhs;
if (borrow) {
// The lost fraction is being subtracted, borrow from the significand
// and invert `lost_fraction`.
if (lost_fraction == lfLessThanHalf)
lost_fraction = lfMoreThanHalf;
else if (lost_fraction == lfMoreThanHalf)
lost_fraction = lfLessThanHalf;
}
carry = temp_rhs.subtractSignificand(*this, borrow);
copySignificand(temp_rhs);
sign = !sign;
} else {
carry = subtractSignificand
(temp_rhs, lost_fraction != lfExactlyZero);
} else if (cmp_result == cmpGreaterThan) {
bool borrow = lost_fraction != lfExactlyZero && lost_fraction_is_from_rhs;
if (borrow) {
// The lost fraction is being subtracted, borrow from the significand
// and invert `lost_fraction`.
if (lost_fraction == lfLessThanHalf)
lost_fraction = lfMoreThanHalf;
else if (lost_fraction == lfMoreThanHalf)
lost_fraction = lfLessThanHalf;
}
carry = subtractSignificand(temp_rhs, borrow);
} else { // cmpEqual
zeroSignificand();
if (lost_fraction != lfExactlyZero && lost_fraction_is_from_rhs) {
// rhs is slightly larger due to the lost fraction, flip the sign.
sign = !sign;
}
}

/* Invert the lost fraction - it was on the RHS and
subtracted. */
if (lost_fraction == lfLessThanHalf)
lost_fraction = lfMoreThanHalf;
else if (lost_fraction == lfMoreThanHalf)
lost_fraction = lfLessThanHalf;

/* The code above is intended to ensure that no borrow is
necessary. */
assert(!carry);
Expand Down
176 changes: 176 additions & 0 deletions llvm/unittests/ADT/APFloatTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,104 @@ TEST(APFloatTest, FMA) {
EXPECT_EQ(-8.85242279E-41f, f1.convertToFloat());
}

// The `addOrSubtractSignificand` can be considered to have 9 possible cases
// when subtracting: all combinations of {cmpLessThan, cmpGreaterThan,
// cmpEqual} and {no loss, loss from lhs, loss from rhs}. Test each reachable
// case here.

// Regression test for failing the `assert(!carry)` in
// `addOrSubtractSignificand` and normalizing the exponent even when the
// significand is zero if there is a lost fraction.
// This tests cmpEqual, loss from lhs
{
APFloat f1(-1.4728589E-38f);
APFloat f2(3.7105144E-6f);
APFloat f3(5.5E-44f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(-0.0f, f1.convertToFloat());
}

// Test cmpGreaterThan, no loss
{
APFloat f1(2.0f);
APFloat f2(2.0f);
APFloat f3(-3.5f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(0.5f, f1.convertToFloat());
}

// Test cmpLessThan, no loss
{
APFloat f1(2.0f);
APFloat f2(2.0f);
APFloat f3(-4.5f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(-0.5f, f1.convertToFloat());
}

// Test cmpEqual, no loss
{
APFloat f1(2.0f);
APFloat f2(2.0f);
APFloat f3(-4.0f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(0.0f, f1.convertToFloat());
}

// Test cmpLessThan, loss from lhs
{
APFloat f1(2.0000002f);
APFloat f2(2.0000002f);
APFloat f3(-32.0f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(-27.999998f, f1.convertToFloat());
}

// Test cmpGreaterThan, loss from rhs
{
APFloat f1(1e10f);
APFloat f2(1e10f);
APFloat f3(-2.0000002f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(1e20f, f1.convertToFloat());
}

// Test cmpGreaterThan, loss from lhs
{
APFloat f1(1e-36f);
APFloat f2(0.0019531252f);
APFloat f3(-1e-45f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(1.953124e-39f, f1.convertToFloat());
}

// {cmpEqual, cmpLessThan} with loss from rhs can't occur for the usage in
// `fusedMultiplyAdd` as `multiplySignificand` normalises the MSB of lhs to
// one bit below the top.

// Test cases from #104984
{
APFloat f1(0.24999998f);
APFloat f2(2.3509885e-38f);
APFloat f3(-1e-45f);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(5.87747e-39f, f1.convertToFloat());
}
{
APFloat f1(4.4501477170144023e-308);
APFloat f2(0.24999999999999997);
APFloat f3(-8.475904604373977e-309);
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(2.64946468816203e-309, f1.convertToDouble());
}
{
APFloat f1(APFloat::IEEEhalf(), APInt(16, 0x8fffu));
APFloat f2(APFloat::IEEEhalf(), APInt(16, 0x2bffu));
APFloat f3(APFloat::IEEEhalf(), APInt(16, 0x0172u));
f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
EXPECT_EQ(0x808eu, f1.bitcastToAPInt().getZExtValue());
}

// Test using only a single instance of APFloat.
{
APFloat F(1.5);
Expand Down Expand Up @@ -8089,4 +8187,82 @@ TEST(APFloatTest, Float4E2M1FNToFloat) {
EXPECT_TRUE(SmallestDenorm.isDenormal());
EXPECT_EQ(0x0.8p0, SmallestDenorm.convertToFloat());
}

TEST(APFloatTest, AddOrSubtractSignificand) {
class TestIEEEFloat : detail::IEEEFloat {
TestIEEEFloat(bool sign, APFloat::ExponentType exponent,
APFloat::integerPart significand)
: detail::IEEEFloat(1.0) {
// `addOrSubtractSignificand` only uses the sign, exponent and significand
this->sign = sign;
this->exponent = exponent;
this->significand.part = significand;
}

public:
static void runTest(bool subtract, bool lhsSign,
APFloat::ExponentType lhsExponent,
APFloat::integerPart lhsSignificand, bool rhsSign,
APFloat::ExponentType rhsExponent,
APFloat::integerPart rhsSignificand, bool expectedSign,
APFloat::ExponentType expectedExponent,
APFloat::integerPart expectedSignificand,
lostFraction expectedLoss) {
TestIEEEFloat lhs(lhsSign, lhsExponent, lhsSignificand);
TestIEEEFloat rhs(rhsSign, rhsExponent, rhsSignificand);
lostFraction resultLoss = lhs.addOrSubtractSignificand(rhs, subtract);
EXPECT_EQ(resultLoss, expectedLoss);
EXPECT_EQ(lhs.sign, expectedSign);
EXPECT_EQ(lhs.exponent, expectedExponent);
EXPECT_EQ(lhs.significand.part, expectedSignificand);
}
};

// Test cases are all combinations of:
// {equal exponents, LHS larger exponent, RHS larger exponent}
// {equal significands, LHS larger significand, RHS larger significand}
// {no loss, loss}

// Equal exponents (loss cannot occur as their is no shifting)
TestIEEEFloat::runTest(true, false, 1, 0x10, false, 1, 0x5, false, 1, 0xb,
lfExactlyZero);
TestIEEEFloat::runTest(false, false, -2, 0x20, true, -2, 0x20, false, -2, 0,
lfExactlyZero);
TestIEEEFloat::runTest(false, true, 3, 0x20, false, 3, 0x30, false, 3, 0x10,
lfExactlyZero);

// LHS larger exponent
// LHS significand greater after shitfing
TestIEEEFloat::runTest(true, false, 7, 0x100, false, 3, 0x100, false, 6,
0x1e0, lfExactlyZero);
TestIEEEFloat::runTest(true, false, 7, 0x100, false, 3, 0x101, false, 6,
0x1df, lfMoreThanHalf);
// Significands equal after shitfing
TestIEEEFloat::runTest(true, false, 7, 0x100, false, 3, 0x1000, false, 6, 0,
lfExactlyZero);
TestIEEEFloat::runTest(true, false, 7, 0x100, false, 3, 0x1001, true, 6, 0,
lfLessThanHalf);
// RHS significand greater after shitfing
TestIEEEFloat::runTest(true, false, 7, 0x100, false, 3, 0x10000, true, 6,
0x1e00, lfExactlyZero);
TestIEEEFloat::runTest(true, false, 7, 0x100, false, 3, 0x10001, true, 6,
0x1e00, lfLessThanHalf);

// RHS larger exponent
// RHS significand greater after shitfing
TestIEEEFloat::runTest(true, false, 3, 0x100, false, 7, 0x100, true, 6, 0x1e0,
lfExactlyZero);
TestIEEEFloat::runTest(true, false, 3, 0x101, false, 7, 0x100, true, 6, 0x1df,
lfMoreThanHalf);
// Significands equal after shitfing
TestIEEEFloat::runTest(true, false, 3, 0x1000, false, 7, 0x100, false, 6, 0,
lfExactlyZero);
TestIEEEFloat::runTest(true, false, 3, 0x1001, false, 7, 0x100, false, 6, 0,
lfLessThanHalf);
// LHS significand greater after shitfing
TestIEEEFloat::runTest(true, false, 3, 0x10000, false, 7, 0x100, false, 6,
0x1e00, lfExactlyZero);
TestIEEEFloat::runTest(true, false, 3, 0x10001, false, 7, 0x100, false, 6,
0x1e00, lfLessThanHalf);
}
} // namespace

0 comments on commit eabd142

Please sign in to comment.