From 08de551f4f1087c4dd3220fee8ec328fa168168c Mon Sep 17 00:00:00 2001 From: Ehud Katz Date: Tue, 7 Jan 2020 08:45:18 +0200 Subject: [PATCH] [APFloat] Fix fusedMultiplyAdd when `this` equals to `Addend` Up until now, the arguments to `fusedMultiplyAdd` are passed by reference. We must save the `Addend` value on the beginning of the function, before we modify `this`, as they may be the same reference. To fix this, we now pass the `addend` parameter of `multiplySignificand` by value (instead of by-ref), and have a default value of zero. Fix PR44051. Differential Revision: https://reviews.llvm.org/D70422 --- llvm/include/llvm/ADT/APFloat.h | 3 ++- llvm/lib/Support/APFloat.cpp | 21 ++++++++++++--------- llvm/unittests/ADT/APFloatTest.cpp | 8 ++++++++ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h index e415054..ed25b2c 100644 --- a/llvm/include/llvm/ADT/APFloat.h +++ b/llvm/include/llvm/ADT/APFloat.h @@ -487,7 +487,8 @@ private: integerPart addSignificand(const IEEEFloat &); integerPart subtractSignificand(const IEEEFloat &, integerPart); lostFraction addOrSubtractSignificand(const IEEEFloat &, bool subtract); - lostFraction multiplySignificand(const IEEEFloat &, const IEEEFloat *); + lostFraction multiplySignificand(const IEEEFloat &, IEEEFloat); + lostFraction multiplySignificand(const IEEEFloat&); lostFraction divideSignificand(const IEEEFloat &); void incrementSignificand(); void initialize(const fltSemantics *); diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp index d26c5e6..723bbbc 100644 --- a/llvm/lib/Support/APFloat.cpp +++ b/llvm/lib/Support/APFloat.cpp @@ -992,7 +992,7 @@ IEEEFloat::integerPart IEEEFloat::subtractSignificand(const IEEEFloat &rhs, on to the full-precision result of the multiplication. Returns the lost fraction. */ lostFraction IEEEFloat::multiplySignificand(const IEEEFloat &rhs, - const IEEEFloat *addend) { + IEEEFloat addend) { unsigned int omsb; // One, not zero, based MSB. unsigned int partsCount, newPartsCount, precision; integerPart *lhsSignificand; @@ -1036,7 +1036,7 @@ lostFraction IEEEFloat::multiplySignificand(const IEEEFloat &rhs, // toward left by two bits, and adjust exponent accordingly. exponent += 2; - if (addend && addend->isNonZero()) { + if (addend.isNonZero()) { // The intermediate result of the multiplication has "2 * precision" // signicant bit; adjust the addend to be consistent with mul result. // @@ -1065,19 +1065,18 @@ lostFraction IEEEFloat::multiplySignificand(const IEEEFloat &rhs, significand.parts = fullSignificand; semantics = &extendedSemantics; - IEEEFloat extendedAddend(*addend); - status = extendedAddend.convert(extendedSemantics, rmTowardZero, &ignored); + status = addend.convert(extendedSemantics, rmTowardZero, &ignored); assert(status == opOK); (void)status; // Shift the significand of the addend right by one bit. This guarantees // that the high bit of the significand is zero (same as fullSignificand), // so the addition will overflow (if it does overflow at all) into the top bit. - lost_fraction = extendedAddend.shiftSignificandRight(1); + lost_fraction = addend.shiftSignificandRight(1); assert(lost_fraction == lfExactlyZero && "Lost precision while shifting addend for fused-multiply-add."); - lost_fraction = addOrSubtractSignificand(extendedAddend, false); + lost_fraction = addOrSubtractSignificand(addend, false); /* Restore our state. */ if (newPartsCount == 1) @@ -1120,6 +1119,10 @@ lostFraction IEEEFloat::multiplySignificand(const IEEEFloat &rhs, return lost_fraction; } +lostFraction IEEEFloat::multiplySignificand(const IEEEFloat &rhs) { + return multiplySignificand(rhs, IEEEFloat(*semantics)); +} + /* Multiply the significands of LHS and RHS to DST. */ lostFraction IEEEFloat::divideSignificand(const IEEEFloat &rhs) { unsigned int bit, i, partsCount; @@ -1725,7 +1728,7 @@ IEEEFloat::opStatus IEEEFloat::multiply(const IEEEFloat &rhs, fs = multiplySpecials(rhs); if (isFiniteNonZero()) { - lostFraction lost_fraction = multiplySignificand(rhs, nullptr); + lostFraction lost_fraction = multiplySignificand(rhs); fs = normalize(rounding_mode, lost_fraction); if (lost_fraction != lfExactlyZero) fs = (opStatus) (fs | opInexact); @@ -1826,7 +1829,7 @@ IEEEFloat::opStatus IEEEFloat::fusedMultiplyAdd(const IEEEFloat &multiplicand, addend.isFinite()) { lostFraction lost_fraction; - lost_fraction = multiplySignificand(multiplicand, &addend); + lost_fraction = multiplySignificand(multiplicand, addend); fs = normalize(rounding_mode, lost_fraction); if (lost_fraction != lfExactlyZero) fs = (opStatus) (fs | opInexact); @@ -2449,7 +2452,7 @@ IEEEFloat::roundSignificandWithExponent(const integerPart *decSigParts, if (exp >= 0) { /* multiplySignificand leaves the precision-th bit set to 1. */ - calcLostFraction = decSig.multiplySignificand(pow5, nullptr); + calcLostFraction = decSig.multiplySignificand(pow5); powHUerr = powStatus != opOK; } else { calcLostFraction = decSig.divideSignificand(pow5); diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp index db529a0..adbf1b3 100644 --- a/llvm/unittests/ADT/APFloatTest.cpp +++ b/llvm/unittests/ADT/APFloatTest.cpp @@ -547,6 +547,14 @@ TEST(APFloatTest, FMA) { f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven); EXPECT_EQ(-8.85242279E-41f, f1.convertToFloat()); } + + // Test using only a single instance of APFloat. + { + APFloat F(1.5); + + F.fusedMultiplyAdd(F, F, APFloat::rmNearestTiesToEven); + EXPECT_EQ(3.75, F.convertToDouble()); + } } TEST(APFloatTest, MinNum) { -- 2.7.4