[llvm] r233312 - Fix rare case where APInt divide algorithm applied un-needed transformation.
Yaron Keren
yaron.keren at gmail.com
Thu Mar 26 12:45:19 PDT 2015
Author: yrnkrn
Date: Thu Mar 26 14:45:19 2015
New Revision: 233312
URL: http://llvm.org/viewvc/llvm-project?rev=233312&view=rev
Log:
Fix rare case where APInt divide algorithm applied un-needed transformation.
APInt uses Knuth's D algorithm for long division. In rare cases the
implementation applied a transformation that was not needed.
Added unit tests for long division. KnuthDiv() procedure is fully covered.
There is a case in APInt::divide() that I believe is never used (marked with
a comment) as all users of divide() handle trivial cases earlier.
Patch by Pawel Bylica!
http://reviews.llvm.org/D8448
Modified:
llvm/trunk/lib/Support/APInt.cpp
llvm/trunk/unittests/ADT/APIntTest.cpp
Modified: llvm/trunk/lib/Support/APInt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=233312&r1=233311&r2=233312&view=diff
==============================================================================
--- llvm/trunk/lib/Support/APInt.cpp (original)
+++ llvm/trunk/lib/Support/APInt.cpp Thu Mar 26 14:45:19 2015
@@ -1511,21 +1511,18 @@ static void KnuthDiv(unsigned *u, unsign
assert(u && "Must provide dividend");
assert(v && "Must provide divisor");
assert(q && "Must provide quotient");
- assert(u != v && u != q && v != q && "Must us different memory");
+ assert(u != v && u != q && v != q && "Must use different memory");
assert(n>1 && "n must be > 1");
- // Knuth uses the value b as the base of the number system. In our case b
- // is 2^31 so we just set it to -1u.
- uint64_t b = uint64_t(1) << 32;
+ // b denotes the base of the number system. In our case b is 2^32.
+ LLVM_CONSTEXPR uint64_t b = uint64_t(1) << 32;
-#if 0
DEBUG(dbgs() << "KnuthDiv: m=" << m << " n=" << n << '\n');
DEBUG(dbgs() << "KnuthDiv: original:");
DEBUG(for (int i = m+n; i >=0; i--) dbgs() << " " << u[i]);
DEBUG(dbgs() << " by");
DEBUG(for (int i = n; i >0; i--) dbgs() << " " << v[i-1]);
DEBUG(dbgs() << '\n');
-#endif
// D1. [Normalize.] Set d = b / (v[n-1] + 1) and multiply all the digits of
// u and v by d. Note that we have taken Knuth's advice here to use a power
// of 2 value for d such that d * v[n-1] >= b/2 (b is the base). A power of
@@ -1550,13 +1547,12 @@ static void KnuthDiv(unsigned *u, unsign
}
}
u[m+n] = u_carry;
-#if 0
+
DEBUG(dbgs() << "KnuthDiv: normal:");
DEBUG(for (int i = m+n; i >=0; i--) dbgs() << " " << u[i]);
DEBUG(dbgs() << " by");
DEBUG(for (int i = n; i >0; i--) dbgs() << " " << v[i-1]);
DEBUG(dbgs() << '\n');
-#endif
// D2. [Initialize j.] Set j to m. This is the loop counter over the places.
int j = m;
@@ -1586,46 +1582,35 @@ static void KnuthDiv(unsigned *u, unsign
// (u[j+n]u[j+n-1]..u[j]) - qp * (v[n-1]...v[1]v[0]). This computation
// consists of a simple multiplication by a one-place number, combined with
// a subtraction.
+ // The digits (u[j+n]...u[j]) should be kept positive; if the result of
+ // 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;
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]);
bool borrow = subtrahend > u_tmp;
- DEBUG(dbgs() << "KnuthDiv: u_tmp == " << u_tmp
- << ", subtrahend == " << subtrahend
+ DEBUG(dbgs() << "KnuthDiv: u_tmp = " << u_tmp
+ << ", subtrahend = " << subtrahend
<< ", borrow = " << borrow << '\n');
uint64_t result = u_tmp - subtrahend;
unsigned k = j + i;
- u[k++] = (unsigned)(result & (b-1)); // subtract low word
- u[k++] = (unsigned)(result >> 32); // subtract high word
- while (borrow && k <= m+n) { // deal with borrow to the left
+ 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');
+ DEBUG(dbgs() << "KnuthDiv: u[j+i] = " << u[j+i]
+ << ", u[j+i+1] = " << u[j+i+1] << '\n');
}
DEBUG(dbgs() << "KnuthDiv: after subtraction:");
DEBUG(for (int i = m+n; i >=0; i--) dbgs() << " " << u[i]);
DEBUG(dbgs() << '\n');
- // The digits (u[j+n]...u[j]) should be kept positive; if the result of
- // 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.
- //
- if (isNeg) {
- bool carry = true; // true because b's complement is "complement + 1"
- for (unsigned i = 0; i <= m+n; ++i) {
- u[i] = ~u[i] + carry; // b's complement
- carry = carry && u[i] == 0;
- }
- }
- DEBUG(dbgs() << "KnuthDiv: after complement:");
- DEBUG(for (int i = m+n; i >=0; i--) dbgs() << " " << u[i]);
- DEBUG(dbgs() << '\n');
// D5. [Test remainder.] Set q[j] = qp. If the result of step D4 was
// negative, go to step D6; otherwise go on to step D7.
@@ -1647,7 +1632,7 @@ static void KnuthDiv(unsigned *u, unsign
u[j+n] += carry;
}
DEBUG(dbgs() << "KnuthDiv: after correction:");
- DEBUG(for (int i = m+n; i >=0; i--) dbgs() <<" " << u[i]);
+ DEBUG(for (int i = m+n; i >=0; i--) dbgs() << " " << u[i]);
DEBUG(dbgs() << "\nKnuthDiv: digit result = " << q[j] << '\n');
// D7. [Loop on j.] Decrease j by one. Now if j >= 0, go back to D3.
@@ -1680,9 +1665,7 @@ static void KnuthDiv(unsigned *u, unsign
}
DEBUG(dbgs() << '\n');
}
-#if 0
DEBUG(dbgs() << '\n');
-#endif
}
void APInt::divide(const APInt LHS, unsigned lhsWords,
@@ -1806,6 +1789,8 @@ void APInt::divide(const APInt LHS, unsi
// The quotient is in Q. Reconstitute the quotient into Quotient's low
// order words.
+ // This case is currently dead as all users of divide() handle trivial cases
+ // earlier.
if (lhsWords == 1) {
uint64_t tmp =
uint64_t(Q[0]) | (uint64_t(Q[1]) << (APINT_BITS_PER_WORD / 2));
Modified: llvm/trunk/unittests/ADT/APIntTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APIntTest.cpp?rev=233312&r1=233311&r2=233312&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/APIntTest.cpp (original)
+++ llvm/trunk/unittests/ADT/APIntTest.cpp Thu Mar 26 14:45:19 2015
@@ -209,6 +209,206 @@ TEST(APIntTest, i1) {
}
}
+TEST(APIntTest, divrem_big1) {
+ // Tests KnuthDiv rare step D6
+ APInt a{256, "1ffffffffffffffff", 16};
+ APInt b{256, "1ffffffffffffffff", 16};
+ APInt c{256, 0};
+
+ auto p = a * b + c;
+ auto q = p.udiv(a);
+ auto r = p.urem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.udiv(b);
+ r = p.urem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(a);
+ r = p.srem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(b);
+ r = p.srem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+}
+
+TEST(APIntTest, divrem_big2) {
+ // Tests KnuthDiv rare step D6
+ APInt a{1024, "111111ffffffffffffffff"
+ "ffffffffffffffffffffffffffffffff"
+ "fffffffffffffffffffffffffffffccf"
+ "ffffffffffffffffffffffffffffff00", 16};
+ APInt b{1024, "112233ceff"
+ "cecece000000ffffffffffffffffffff"
+ "ffffffffffffffffffffffffffffffff"
+ "ffffffffffffffffffffffffffffffff"
+ "ffffffffffffffffffffffffffffff33", 16};
+ APInt c{1024, 7919};
+
+ auto p = a * b + c;
+ auto q = p.udiv(a);
+ auto r = p.urem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.udiv(b);
+ r = p.urem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(a);
+ r = p.srem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(b);
+ r = p.srem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+}
+
+TEST(APIntTest, divrem_big3) {
+ // Tests KnuthDiv case without shift
+ APInt a{256, "ffffffffffffff0000000", 16};
+ APInt b{256, "80000001ffffffffffffffff", 16};
+ APInt c{256, 4219};
+
+ auto p = a * b + c;
+ auto q = p.udiv(a);
+ auto r = p.urem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.udiv(b);
+ r = p.urem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(a);
+ r = p.srem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(b);
+ r = p.srem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+}
+
+TEST(APIntTest, divrem_big4) {
+ // Tests heap allocation in divide() enfoced by huge numbers
+ auto a = APInt{4096, 1}.shl(2000);
+ auto b = APInt{4096, 5}.shl(2001);
+ auto c = APInt{4096, 4219*13};
+
+ auto p = a * b + c;
+ auto q = p.udiv(a);
+ auto r = p.urem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = APInt{1024, 0}; // test non-single word APInt conversion in divide()
+ r = APInt{1024, 0};
+ APInt::udivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.udiv(b);
+ r = p.urem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = APInt{1024, 0};
+ r = APInt{1024, 0};
+ APInt::udivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(a);
+ r = p.srem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = APInt{1024, 0};
+ r = APInt{1024, 0};
+ APInt::sdivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(b);
+ r = p.srem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = APInt{1024, 0};
+ r = APInt{1024, 0};
+ APInt::sdivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+}
+
+TEST(APIntTest, divrem_big5) {
+ // Tests one word divisor case of divide()
+ auto a = APInt{1024, 19}.shl(811);
+ auto b = APInt{1024, 4356013}; // one word
+ auto c = APInt{1024, 1};
+
+ auto p = a * b + c;
+ auto q = p.udiv(a);
+ auto r = p.urem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.udiv(b);
+ r = p.urem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::udivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(a);
+ r = p.srem(a);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, a, q, r);
+ EXPECT_EQ(q, b);
+ EXPECT_EQ(r, c);
+ q = p.sdiv(b);
+ r = p.srem(b);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+ APInt::sdivrem(p, b, q, r);
+ EXPECT_EQ(q, a);
+ EXPECT_EQ(r, c);
+}
+
TEST(APIntTest, fromString) {
EXPECT_EQ(APInt(32, 0), APInt(32, "0", 2));
EXPECT_EQ(APInt(32, 1), APInt(32, "1", 2));
More information about the llvm-commits
mailing list