[APFloat] Fix subtraction of subnormal numbers

[ekatz]

Summary

Fix incorrect determination of the bigger number out of the two subtracted, while subnormal numbers are involved.

[lattner]

I'd recommend adding Steve Canon to APFloat related patches. Thanks!

[ekatz]

Ping

[ekatz]

Ping

[foad]

LGTM.

Would you mind adding a fused multiply-add test case like the one from bug 44010?

llvm/lib/Support/APFloat.cpp

1549

The diff would be smaller if you left these tests in the same order :-)

[foad]

LGTM.

[foad]

@ekatz are you able to commit this yourself? Or are planning to obtain commit access (https://llvm.org/docs/DeveloperPolicy.html#new-contributors)?

[ekatz]

In D69772#1753881, @foad wrote:

@ekatz are you able to commit this yourself? Or are planning to obtain commit access (https://llvm.org/docs/DeveloperPolicy.html#new-contributors)?

Thanks for asking. I've requested commit access - waiting for reply.

[ekatz]

Reordered the conditions, to make a smaller diff.

Also replaced the previous test case with the one from PR44010, as it is practically the same, but also tests the result.

[foad]

Looks even better to me now :-)

llvm/lib/Support/APFloat.cpp

lostFraction IEEEFloat::addOrSubtractSignificand(const IEEEFloat &rhs,
   subtract ^= static_cast<bool>(sign ^ rhs.sign);
 
   /* Are we bigger exponent-wise than the RHS?  */
   bits = exponent - rhs.exponent;
 
   /* Subtraction is more subtle than one might naively expect.  */
   if (subtract) {
     IEEEFloat temp_rhs(rhs);
-    bool reverse;
 
-    if (bits == 0) {
-      reverse = compareAbsoluteValue(temp_rhs) == cmpLessThan;
-      lost_fraction = lfExactlyZero;
-    } else if (bits > 0) {
+    if (bits > 0) {
       lost_fraction = temp_rhs.shiftSignificandRight(bits - 1);
       shiftSignificandLeft(1);
-      reverse = false;
-    } else {
+    } else if (bits < 0) {
       lost_fraction = shiftSignificandRight(-bits - 1);
       temp_rhs.shiftSignificandLeft(1);
-      reverse = true;
-    }
+    } else
+      lost_fraction = lfExactlyZero;
 
-    if (reverse) {
+    // Should we reverse the subtraction.
+    if (compareAbsoluteValue(temp_rhs) == cmpLessThan) {
       carry = temp_rhs.subtractSignificand
         (*this, lost_fraction != lfExactlyZero);
       copySignificand(temp_rhs);
       sign = !sign;
     } else {
       carry = subtractSignificand
         (temp_rhs, lost_fraction != lfExactlyZero);
     }

llvm/unittests/ADT/APFloatTest.cpp

// Test x87 extended precision case from http://llvm.org/PR20728.
     APFloat A(APFloat::x87DoubleExtended(), 3.0);
 
     bool losesInfo = false;
     M1.fusedMultiplyAdd(M1, A, APFloat::rmNearestTiesToEven);
     M1.convert(APFloat::IEEEsingle(), APFloat::rmNearestTiesToEven, &losesInfo);
     EXPECT_FALSE(losesInfo);
     EXPECT_EQ(4.0f, M1.convertToFloat());
   }
+
+  // Test addition of a subnormal number with a negative normal.
+  // This used to trigger an assert failure.
+  {
+    APFloat f1(123.456);
+    APFloat f2(APFloat::IEEEdouble(), "-0x1p-1074");
+    APFloat f3(APFloat::IEEEdouble(), "+0x1p-1074");
+
+    f1.fusedMultiplyAdd(f2, f3, APFloat::rmNearestTiesToEven);
+  }
 }
 
 TEST(APFloatTest, MinNum) {
   APFloat f1(1.0);
   APFloat f2(2.0);
   APFloat nan = APFloat::getNaN(APFloat::IEEEdouble());
 
   EXPECT_EQ(1.0, minnum(f1, f2).convertToDouble());