[PATCH] D29346: Fix APFloat mod

[Simon Byrne via Phabricator via llvm-commits]

simonbyrne created this revision.

The existing code for APFloat mod was incorrect in two ways:

- intermediate rounding gave incorrect results in some cases (e.g. frem double 0.3 0.01)
- the method of integer truncation could cause intermediate overflow

This caused subsequent errors in the constant propagation pass, see https://llvm.org/bugs/show_bug.cgi?id=3316


Repository:
  rL LLVM

https://reviews.llvm.org/D29346

Files:
  lib/Support/APFloat.cpp


Index: lib/Support/APFloat.cpp
===================================================================
--- lib/Support/APFloat.cpp
+++ lib/Support/APFloat.cpp
@@ -1739,43 +1739,30 @@
   return fs;
 }
 
-/* Normalized llvm frem (C fmod).
-   This is not currently correct in all cases.  */
+/* Normalized llvm frem (C fmod).  */
 IEEEFloat::opStatus IEEEFloat::mod(const IEEEFloat &rhs) {
   opStatus fs;
   fs = modSpecials(rhs);
 
   if (isFiniteNonZero() && rhs.isFiniteNonZero()) {
-    IEEEFloat V = *this;
-    unsigned int origSign = sign;
-
-    fs = V.divide(rhs, rmNearestTiesToEven);
-    if (fs == opDivByZero)
-      return fs;
-
-    int parts = partCount();
-    integerPart *x = new integerPart[parts];
-    bool ignored;
-    fs = V.convertToInteger(x, parts * integerPartWidth, true,
-                            rmTowardZero, &ignored);
-    if (fs==opInvalidOp) {
-      delete[] x;
-      return fs;
-    }
-
-    fs = V.convertFromZeroExtendedInteger(x, parts * integerPartWidth, true,
-                                          rmNearestTiesToEven);
-    assert(fs==opOK);   // should always work
-
-    fs = V.multiply(rhs, rmNearestTiesToEven);
-    assert(fs==opOK || fs==opInexact);   // should not overflow or underflow
+    while (true) {
+      IEEEFloat V = *this;
 
-    fs = subtract(V, rmNearestTiesToEven);
-    assert(fs==opOK || fs==opInexact);   // likewise
+      fs = V.divide(rhs, rmTowardZero);
+      if (fs == opDivByZero)
+        return fs;
+      V.roundToIntegral(rmTowardZero);
+      if (V.isZero())
+        break;
 
-    if (isZero())
-      sign = origSign;    // IEEE754 requires this
-    delete[] x;
+      /* *this = *this - V*rhs     */
+      V.sign = !V.sign;
+      lostFraction lost_fraction;
+      lost_fraction = V.multiplySignificand(rhs, this);
+      fs = normalize(rmTowardZero, lost_fraction);
+      assert(fs==opOK);   // should always be exact
+      *this = V;
+    }
   }
   return fs;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29346.86483.patch
Type: text/x-patch
Size: 1959 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170131/3d7e7fbb/attachment.bin>