[PATCH] Fix APInt division algorithm

[Paweł Bylica]

REPOSITORY
  rL LLVM

================
Comment at: lib/Support/APInt.cpp:1510
@@ -1512,1 +1509,3 @@
+  // b denotes the base of the number system. In our case b is 2^32.
+  static const uint64_t b = uint64_t(1) << 32;
 
----------------
yaron.keren wrote:
> Could b be LLVM_CONSTEXPR ?
> 
Probably. I'm not used to constexpr because Visual Studio still does not support it.

================
Comment at: lib/Support/APInt.cpp:1583
@@ -1582,3 +1582,3 @@
     for (unsigned i = 0; i < n; ++i) {
-      uint64_t u_tmp = uint64_t(u[j+i]) | (uint64_t(u[j+i+1]) << 32);
+      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]);
----------------
yaron.keren wrote:
> The parenthesis around the shift make the expression more readable, even if not required.
> 
> You flipped the | arguments so they would consistent with the usual u[j+n], u[j+n-1]...u[j] representation?
> 
I flipped arguments because I like high word being on the left. You shift left to make space for a low word. 

================
Comment at: lib/Support/APInt.cpp:1784
@@ -1800,3 +1783,3 @@
     // order words.
-    if (lhsWords == 1) {
+    if (lhsWords == 1) { // FIXME: Probably a dead case, lshWords is never 1
       uint64_t tmp =
----------------
yaron.keren wrote:
> I'd remove the FIXME and explain that *currently* this is a dead case since all callers in APInt calls either test for divide() with lhswords == rhswords == 1 or proivde the a small radix before calling divide() but it is worth keeping this code alive for correctness in case some new code does call divide() with lhswords == rhswords == 1.
> 
Ok, I will change that. I'm just a bit worried that this code cannot be reached with unittests.

http://reviews.llvm.org/D8448

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/