From 4455142a95bb3d0f6e6cbb336d6558919cb59bb8 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sat, 26 Nov 2011 03:38:02 +0000 Subject: [PATCH] Fix APFloat::convert so that it handles narrowing conversions correctly; it was returning incorrect values in rare cases, and incorrectly marking exact conversions as inexact in some more common cases. Fixes PR11406, and a missed optimization in test/CodeGen/X86/fp-stack-O0.ll. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@145141 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Support/APFloat.cpp | 85 ++++++++++++++------------------- test/CodeGen/X86/fp-stack-O0.ll | 2 +- unittests/ADT/APFloatTest.cpp | 24 ++++++++++ 3 files changed, 61 insertions(+), 50 deletions(-) diff --git a/lib/Support/APFloat.cpp b/lib/Support/APFloat.cpp index f2388944929..0ae6f1c39e3 100644 --- a/lib/Support/APFloat.cpp +++ b/lib/Support/APFloat.cpp @@ -1854,20 +1854,33 @@ APFloat::convert(const fltSemantics &toSemantics, lostFraction lostFraction; unsigned int newPartCount, oldPartCount; opStatus fs; + int shift; + const fltSemantics &fromSemantics = *semantics; - assertArithmeticOK(*semantics); + assertArithmeticOK(fromSemantics); assertArithmeticOK(toSemantics); lostFraction = lfExactlyZero; newPartCount = partCountForBits(toSemantics.precision + 1); oldPartCount = partCount(); + shift = toSemantics.precision - fromSemantics.precision; - /* Handle storage complications. If our new form is wider, - re-allocate our bit pattern into wider storage. If it is - narrower, we ignore the excess parts, but if narrowing to a - single part we need to free the old storage. - Be careful not to reference significandParts for zeroes - and infinities, since it aborts. */ + bool X86SpecialNan = false; + if (&fromSemantics == &APFloat::x87DoubleExtended && + &toSemantics != &APFloat::x87DoubleExtended && category == fcNaN && + (!(*significandParts() & 0x8000000000000000ULL) || + !(*significandParts() & 0x4000000000000000ULL))) { + // x86 has some unusual NaNs which cannot be represented in any other + // format; note them here. + X86SpecialNan = true; + } + + // If this is a truncation, perform the shift before we narrow the storage. + if (shift < 0 && (category==fcNormal || category==fcNaN)) + lostFraction = shiftRight(significandParts(), oldPartCount, -shift); + + // Fix the storage so it can hold to new value. if (newPartCount > oldPartCount) { + // The new type requires more storage; make it available. integerPart *newParts; newParts = new integerPart[newPartCount]; APInt::tcSet(newParts, 0, newPartCount); @@ -1875,60 +1888,34 @@ APFloat::convert(const fltSemantics &toSemantics, APInt::tcAssign(newParts, significandParts(), oldPartCount); freeSignificand(); significand.parts = newParts; - } else if (newPartCount < oldPartCount) { - /* Capture any lost fraction through truncation of parts so we get - correct rounding whilst normalizing. */ - if (category==fcNormal) - lostFraction = lostFractionThroughTruncation - (significandParts(), oldPartCount, toSemantics.precision); - if (newPartCount == 1) { - integerPart newPart = 0; - if (category==fcNormal || category==fcNaN) - newPart = significandParts()[0]; - freeSignificand(); - significand.part = newPart; - } + } else if (newPartCount == 1 && oldPartCount != 1) { + // Switch to built-in storage for a single part. + integerPart newPart = 0; + if (category==fcNormal || category==fcNaN) + newPart = significandParts()[0]; + freeSignificand(); + significand.part = newPart; } + // Now that we have the right storage, switch the semantics. + semantics = &toSemantics; + + // If this is an extension, perform the shift now that the storage is + // available. + if (shift > 0 && (category==fcNormal || category==fcNaN)) + APInt::tcShiftLeft(significandParts(), newPartCount, shift); + if (category == fcNormal) { - /* Re-interpret our bit-pattern. */ - exponent += toSemantics.precision - semantics->precision; - semantics = &toSemantics; fs = normalize(rounding_mode, lostFraction); *losesInfo = (fs != opOK); } else if (category == fcNaN) { - int shift = toSemantics.precision - semantics->precision; - // Do this now so significandParts gets the right answer - const fltSemantics *oldSemantics = semantics; - semantics = &toSemantics; - *losesInfo = false; - // No normalization here, just truncate - if (shift>0) - APInt::tcShiftLeft(significandParts(), newPartCount, shift); - else if (shift < 0) { - unsigned ushift = -shift; - // Figure out if we are losing information. This happens - // if are shifting out something other than 0s, or if the x87 long - // double input did not have its integer bit set (pseudo-NaN), or if the - // x87 long double input did not have its QNan bit set (because the x87 - // hardware sets this bit when converting a lower-precision NaN to - // x87 long double). - if (APInt::tcLSB(significandParts(), newPartCount) < ushift) - *losesInfo = true; - if (oldSemantics == &APFloat::x87DoubleExtended && - (!(*significandParts() & 0x8000000000000000ULL) || - !(*significandParts() & 0x4000000000000000ULL))) - *losesInfo = true; - APInt::tcShiftRight(significandParts(), newPartCount, ushift); - } + *losesInfo = lostFraction != lfExactlyZero || X86SpecialNan; // gcc forces the Quiet bit on, which means (float)(double)(float_sNan) // does not give you back the same bits. This is dubious, and we // don't currently do it. You're really supposed to get // an invalid operation signal at runtime, but nobody does that. fs = opOK; } else { - semantics = &toSemantics; - fs = opOK; *losesInfo = false; } diff --git a/test/CodeGen/X86/fp-stack-O0.ll b/test/CodeGen/X86/fp-stack-O0.ll index b9cb5d7894c..df90254dbd2 100644 --- a/test/CodeGen/X86/fp-stack-O0.ll +++ b/test/CodeGen/X86/fp-stack-O0.ll @@ -10,7 +10,7 @@ declare i32 @x2(x86_fp80, x86_fp80) nounwind ; Pass arguments on the stack. ; CHECK-NEXT: movq %rsp, [[RCX:%r..]] ; Copy constant-pool value. -; CHECK-NEXT: fldt LCPI +; CHECK-NEXT: fldl LCPI ; CHECK-NEXT: fstpt 16([[RCX]]) ; Copy x1 return value. ; CHECK-NEXT: fstpt ([[RCX]]) diff --git a/unittests/ADT/APFloatTest.cpp b/unittests/ADT/APFloatTest.cpp index b6e02e3a9a3..cc207f764da 100644 --- a/unittests/ADT/APFloatTest.cpp +++ b/unittests/ADT/APFloatTest.cpp @@ -653,4 +653,28 @@ TEST(APFloatTest, getLargest) { EXPECT_EQ(1.7976931348623158e+308, APFloat::getLargest(APFloat::IEEEdouble).convertToDouble()); } +TEST(APFloatTest, convert) { + bool losesInfo; + APFloat test(APFloat::IEEEdouble, "1.0"); + test.convert(APFloat::IEEEsingle, APFloat::rmNearestTiesToEven, &losesInfo); + EXPECT_EQ(1.0f, test.convertToFloat()); + EXPECT_FALSE(losesInfo); + + test = APFloat(APFloat::x87DoubleExtended, "0x1p-53"); + test.add(APFloat(APFloat::x87DoubleExtended, "1.0"), APFloat::rmNearestTiesToEven); + test.convert(APFloat::IEEEdouble, APFloat::rmNearestTiesToEven, &losesInfo); + EXPECT_EQ(1.0, test.convertToDouble()); + EXPECT_TRUE(losesInfo); + + test = APFloat(APFloat::IEEEquad, "0x1p-53"); + test.add(APFloat(APFloat::IEEEquad, "1.0"), APFloat::rmNearestTiesToEven); + test.convert(APFloat::IEEEdouble, APFloat::rmNearestTiesToEven, &losesInfo); + EXPECT_EQ(1.0, test.convertToDouble()); + EXPECT_TRUE(losesInfo); + + test = APFloat(APFloat::x87DoubleExtended, "0xf.fffffffp+28"); + test.convert(APFloat::IEEEdouble, APFloat::rmNearestTiesToEven, &losesInfo); + EXPECT_EQ(4294967295.0, test.convertToDouble()); + EXPECT_FALSE(losesInfo); +} }