Fix APFloat mod sign

[skatkov]

Summary

IEEE specification requires the sign of the remainder is the same as numerator in case remainder is zero.

[skatkov]

What can I do to make a progress with this fix?

[skatkov]

ping...

[scanon]

mod is not bound to the IEEE 754 remainder operation. It binds the C fmod operation. You're looking for the remainder operation.

However, fmod has the same requirement: "The fmod functions return the value x − ny, for some integer n such that, if y is nonzero, the result has the same sign as x and magnitude less than the magnitude of y."

lib/Support/APFloat.cpp

IEEEFloat::opStatus IEEEFloat::remainder(const IEEEFloat &rhs) {
   delete[] x;
   return fs;
 }
 
 /* Normalized llvm frem (C fmod). */
 IEEEFloat::opStatus IEEEFloat::mod(const IEEEFloat &rhs) {
   opStatus fs;
   fs = modSpecials(rhs);
+  unsigned int origSign = sign;
 
   while (isFiniteNonZero() && rhs.isFiniteNonZero() &&
          compareAbsoluteValue(rhs) != cmpLessThan) {
     IEEEFloat V = scalbn(rhs, ilogb(*this) - ilogb(rhs), rmNearestTiesToEven);
     if (compareAbsoluteValue(V) == cmpLessThan)
       V = scalbn(V, -1, rmNearestTiesToEven);
     V.sign = sign;
   
     fs = subtract(V, rmNearestTiesToEven);
     assert(fs==opOK);
   }
+  if (isZero())
+    sign = origSign; // IEEE754 requires this
   return fs;
 }
 
 /* Normalized fused-multiply-add.  */
 IEEEFloat::opStatus IEEEFloat::fusedMultiplyAdd(const IEEEFloat &multiplicand,
                                                 const IEEEFloat &addend,
                                                 roundingMode rounding_mode) {
   opStatus fs;

unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, mod) {
     EXPECT_TRUE(f1.isNaN());
   }
   {
     APFloat f1 = APFloat::getInf(APFloat::IEEEdouble(), false);
     APFloat f2(APFloat::IEEEdouble(), "1.0");
     EXPECT_EQ(f1.mod(f2), APFloat::opInvalidOp);
     EXPECT_TRUE(f1.isNaN());
   }
+  {
+    APFloat f1(APFloat::IEEEdouble(), "-4.0");
+    APFloat f2(APFloat::IEEEdouble(), "-2.0");
+    APFloat expected(APFloat::IEEEdouble(), "-0.0");
+    EXPECT_EQ(f1.mod(f2), APFloat::opOK);
+    EXPECT_TRUE(f1.bitwiseIsEqual(expected));
+  }
+  {
+    APFloat f1(APFloat::IEEEdouble(), "-4.0");
+    APFloat f2(APFloat::IEEEdouble(), "2.0");
+    APFloat expected(APFloat::IEEEdouble(), "-0.0");
+    EXPECT_EQ(f1.mod(f2), APFloat::opOK);
+    EXPECT_TRUE(f1.bitwiseIsEqual(expected));
+  }
 }
 
 TEST(APFloatTest, PPCDoubleDoubleAddSpecial) {
   using DataType = std::tuple<uint64_t, uint64_t, uint64_t, uint64_t,
                               APFloat::fltCategory, APFloat::roundingMode>;
   DataType Data[] = {
       // (1 + 0) + (-1 + 0) = fcZero
       std::make_tuple(0x3ff0000000000000ull, 0, 0xbff0000000000000ull, 0,