Patch IEEEFloat::isSignificandAllZeros and IEEEFloat::isSignificandAllOnes (bug 34579)

[andrewbriand]

Summary

Patch IEEEFloat::isSignificandAllZeros and IEEEFloat::isSignificandAllOnes to behave correctly in the case that the size of the significand is a multiple of the width of the integerParts making up the significand.

The patch to IEEEFloat::isSignificandAllOnes fixes bug 34579, and the patch to IEEE:Float:isSignificandAllZeros fixes the unit test "APFloatTest.x87Next" I added here. I have included both in this diff since the changes are very similar.

[craig.topper]

llvm/lib/Support/APFloat.cpp

845

Does the original code just work if we change this line to "unsigned PartCount = partCountForBits(semantics->precision)"? That will calculate the part count without the guard bit.

[andrewbriand]

Use partCountForBits(semantics->precision) to calculate part count without guard bit.

[andrewbriand]

llvm/lib/Support/APFloat.cpp

845

Yes, that does work. I've updated the patch accordingly.

[craig.topper]

LGTM

[andrewbriand]

In D88340#2304257, @craig.topper wrote:

LGTM

Thank you, I don't have commit access so I can't commit this myself.

[craig.topper]

Commited in b23916504a1a9f29c7519ed83813774eecce1789

llvm/lib/Support/APFloat.cpp

bool IEEEFloat::isSmallest() const {
   return isFiniteNonZero() && exponent == semantics->minExponent &&
     significandMSB() == 0;
 }
 
 bool IEEEFloat::isSignificandAllOnes() const {
   // Test if the significand excluding the integral bit is all ones. This allows
   // us to test for binade boundaries.
   const integerPart *Parts = significandParts();
-  const unsigned PartCount = partCount();
+  const unsigned PartCount = partCountForBits(semantics->precision);
   for (unsigned i = 0; i < PartCount - 1; i++)
     if (~Parts[i])
       return false;
 
   // Set the unused high bits to all ones when we compare.
   const unsigned NumHighBits =
     PartCount*integerPartWidth - semantics->precision + 1;
   assert(NumHighBits <= integerPartWidth && "Can not have more high bits to "
          "fill than integerPartWidth");
   const integerPart HighBitFill =
     ~integerPart(0) << (integerPartWidth - NumHighBits);
   if (~(Parts[PartCount - 1] | HighBitFill))
     return false;
 
   return true;
 }
 
 bool IEEEFloat::isSignificandAllZeros() const {
   // Test if the significand excluding the integral bit is all zeros. This
   // allows us to test for binade boundaries.
   const integerPart *Parts = significandParts();
-  const unsigned PartCount = partCount();
+  const unsigned PartCount = partCountForBits(semantics->precision);
 
   for (unsigned i = 0; i < PartCount - 1; i++)
     if (Parts[i])
       return false;
 
   const unsigned NumHighBits =
     PartCount*integerPartWidth - semantics->precision + 1;
   assert(NumHighBits <= integerPartWidth && "Can not have more high bits to "

llvm/unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, PPCDoubleDoubleFrexp) {
   // 0.75 + 0.75 << 53
   APFloat Result =
       frexp(APFloat(APFloat::PPCDoubleDouble(), APInt(128, 2, Input)), Exp,
             APFloat::rmNearestTiesToEven);
   EXPECT_EQ(2, Exp);
   EXPECT_EQ(0x3fe8000000000000ull, Result.bitcastToAPInt().getRawData()[0]);
   EXPECT_EQ(0x3c98000000000000ull, Result.bitcastToAPInt().getRawData()[1]);
 }
+
+TEST(APFloatTest, x87Largest) {
+  APFloat MaxX87Val = APFloat::getLargest(APFloat::x87DoubleExtended());
+  EXPECT_TRUE(MaxX87Val.isLargest());
+}
+
+TEST(APFloatTest, x87Next) {
+  APFloat F(APFloat::x87DoubleExtended(), "-1.0");
+  F.next(false);
+  EXPECT_TRUE(ilogb(F) == -1);
+}
 }