[PATCH] Fix APInt long division algorithm

[Paweł Bylica]

Hi chandlerc,

This patch fixes step D4 of Knuth's division algorithm implementation. Negative sign of the step result was not always detected due to incorrect "borrow" handling.

REPOSITORY
  rL LLVM

http://reviews.llvm.org/D9196

Files:
  lib/Support/APInt.cpp
  unittests/ADT/APIntTest.cpp

Index: lib/Support/APInt.cpp
===================================================================
--- lib/Support/APInt.cpp
+++ lib/Support/APInt.cpp
@@ -1586,28 +1586,18 @@
     // this step is actually negative, (u[j+n]...u[j]) should be left as the
     // true value plus b**(n+1), namely as the b's complement of
     // the true value, and a "borrow" to the left should be remembered.
-    bool isNeg = false;
+    int64_t borrow = 0;
     for (unsigned i = 0; i < n; ++i) {
-      uint64_t u_tmp = (uint64_t(u[j+i+1]) << 32) | uint64_t(u[j+i]);
-      uint64_t subtrahend = uint64_t(qp) * uint64_t(v[i]);
-      bool borrow = subtrahend > u_tmp;
-      DEBUG(dbgs() << "KnuthDiv: u_tmp = " << u_tmp
-                   << ", subtrahend = " << subtrahend
+      uint64_t p = uint64_t(qp) * uint64_t(v[i]);
+      int64_t subres = int64_t(u[j+i]) - borrow - (unsigned)p;
+      u[j+i] = (unsigned)subres;
+      borrow = (p >> 32) - (subres >> 32);
+      DEBUG(dbgs() << "KnuthDiv: u[j+i] = " << u[j+i]
                    << ", borrow = " << borrow << '\n');
-
-      uint64_t result = u_tmp - subtrahend;
-      unsigned k = j + i;
-      u[k++] = (unsigned)result;         // subtraction low word
-      u[k++] = (unsigned)(result >> 32); // subtraction high word
-      while (borrow && k <= m+n) {       // deal with borrow to the left
-        borrow = u[k] == 0;
-        u[k]--;
-        k++;
-      }
-      isNeg |= borrow;
-      DEBUG(dbgs() << "KnuthDiv: u[j+i] = " << u[j+i] 
-                   << ", u[j+i+1] = " << u[j+i+1] << '\n');
     }
+    bool isNeg = u[j+n] < borrow;
+    u[j+n] -= (unsigned)borrow;
+
     DEBUG(dbgs() << "KnuthDiv: after subtraction:");
     DEBUG(for (int i = m+n; i >=0; i--) dbgs() << " " << u[i]);
     DEBUG(dbgs() << '\n');
Index: unittests/ADT/APIntTest.cpp
===================================================================
--- unittests/ADT/APIntTest.cpp
+++ unittests/ADT/APIntTest.cpp
@@ -409,6 +409,42 @@
   EXPECT_EQ(r, c);
 }
 
+TEST(APIntTest, divrem_big6) {
+  auto a = APInt{512, "10000000000000001000000000000001", 16};
+  auto b = APInt{512, "ffffffffffffffff00000000000000000000000001", 16};
+  auto c = APInt{512, "10000000000000000000000000000000", 16};
+
+  auto p = a * b + c;
+  auto q = p.udiv(a);
+  auto r = p.urem(a);
+  EXPECT_EQ(b, q);
+  EXPECT_EQ(c, r);
+  APInt::udivrem(p, a, q, r);
+  EXPECT_EQ(b, q);
+  EXPECT_EQ(c, r);
+  q = p.udiv(b);
+  r = p.urem(b);
+  EXPECT_EQ(a, q);
+  EXPECT_EQ(c, r);
+  APInt::udivrem(p, b, q, r);
+  EXPECT_EQ(a, q);
+  EXPECT_EQ(c, r);
+  q = p.sdiv(a);
+  r = p.srem(a);
+  EXPECT_EQ(b, q);
+  EXPECT_EQ(c, r);
+  APInt::sdivrem(p, a, q, r);
+  EXPECT_EQ(b, q);
+  EXPECT_EQ(c, r);
+  q = p.sdiv(b);
+  r = p.srem(b);
+  EXPECT_EQ(a, q);
+  EXPECT_EQ(c, r);
+  APInt::sdivrem(p, b, q, r);
+  EXPECT_EQ(a, q);
+  EXPECT_EQ(c, r);
+}
+
 TEST(APIntTest, fromString) {
   EXPECT_EQ(APInt(32, 0), APInt(32,   "0", 2));
   EXPECT_EQ(APInt(32, 1), APInt(32,   "1", 2));

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D9196.24230.patch
Type: text/x-patch
Size: 2984 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150422/6d12ab70/attachment.bin>