[PATCH] Fix APInt division algorithm
Yaron Keren
yaron.keren at gmail.com
Thu Mar 26 01:07:53 PDT 2015
Minor nitpicks, LGTM.
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;
----------------
Could b be LLVM_CONSTEXPR ?
================
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]);
----------------
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?
================
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 =
----------------
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.
http://reviews.llvm.org/D8448
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list