I wrote the following bit of code static APInt FloorOfQuotient(APInt a, APInt b) { unsigned bits = a.getBitWidth(); APInt q(bits, 1), r(bits, 1); APInt::sdivrem(a, b, q, r); * errs() << "sdivrem(" << a << ", " << b << ") = (" << q << ", " << r << ")\n"; * if (r == 0) return q; else { if ((a.sgt(0) && b.sgt(0)) || (a.slt(0) && b.slt(0))) return q; else return q - 1; } } but sometimes see surprising results. Here are examples: sdivrem(9, -2) = (-4, 1) sdivrem(9, -3) = (-3, 0) sdivrem(10, -3) = (-3, 1) sdivrem(38, 2) = (19, 0) sdivrem(29, 1) = (29, 0) *sdivrem(-8, -1) = (-8, 0)* *sdivrem(-9, -1) = (-9, 0)* What's up with the last two? Am I doing something wrong (again)? Thanks, Preston -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120520/426080c7/attachment.html>
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] > On Behalf Of Preston Briggs > Subject: [LLVMdev] APInt::sdivrem error?> APInt q(bits, 1), r(bits, 1);The APInt constructor has three arguments, the last one being whether or not the value is to be treated as signed. It defaults to false, as you appear to have just verified. - Chuck THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY MATERIAL and is thus for use only by the intended recipient. If you received this in error, please contact the sender and delete the e-mail and its attachments from all computers.
OK, the code for sdivrem in APInt.h is wrong. Here's what's written: static void sdivrem(const APInt &LHS, const APInt &RHS, APInt &Quotient, APInt &Remainder) { if (LHS.isNegative()) { if (RHS.isNegative()) APInt::udivrem(-LHS, -RHS, Quotient, Remainder); else APInt::udivrem(-LHS, RHS, Quotient, Remainder); Quotient = -Quotient; Remainder = -Remainder; } else if (RHS.isNegative()) { APInt::udivrem(LHS, -RHS, Quotient, Remainder); Quotient = -Quotient; } else { APInt::udivrem(LHS, RHS, Quotient, Remainder); } } Here's what it should be: static void sdivrem(const APInt &LHS, const APInt &RHS, APInt &Quotient, APInt &Remainder) { if (LHS.isNegative()) { if (RHS.isNegative()) APInt::udivrem(-LHS, -RHS, Quotient, Remainder); else *{ ** APInt::udivrem(-LHS, RHS, Quotient, Remainder); Quotient = -Quotient; ** Remainder = -Remainder; }* } else if (RHS.isNegative()) { APInt::udivrem(LHS, -RHS, Quotient, Remainder); Quotient = -Quotient; } else { APInt::udivrem(LHS, RHS, Quotient, Remainder); } } What should I do to submit the correction? Thanks, Preston On Sun, May 20, 2012 at 6:21 PM, Preston Briggs <preston.briggs at gmail.com> wrote:> I wrote the following bit of code > > static APInt FloorOfQuotient(APInt a, APInt b) { > unsigned bits = a.getBitWidth(); > APInt q(bits, 1), r(bits, 1); > APInt::sdivrem(a, b, q, r); > errs() << "sdivrem(" << a << ", " << b << ") = (" << q << ", " << r << > ")\n"; > if (r == 0) > return q; > else { > if ((a.sgt(0) && b.sgt(0)) || > (a.slt(0) && b.slt(0))) > return q; > else > return q - 1; > } > } > > > but sometimes see surprising results. Here are examples: > > sdivrem(9, -2) = (-4, 1) > sdivrem(9, -3) = (-3, 0) > sdivrem(10, -3) = (-3, 1) > sdivrem(38, 2) = (19, 0) > sdivrem(29, 1) = (29, 0) > sdivrem(-8, -1) = (-8, 0) > sdivrem(-9, -1) = (-9, 0) > > > What's up with the last two? > > Am I doing something wrong (again)? > > Thanks, > Preston >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120521/d1a37577/attachment.html>
On Mon, May 21, 2012 at 11:27 AM, Preston Briggs <preston.briggs at gmail.com> wrote:> OK, the code for sdivrem in APInt.h is wrong. > Here's what's written: > > static void sdivrem(const APInt &LHS, const APInt &RHS, > APInt &Quotient, APInt &Remainder) { > if (LHS.isNegative()) { > if (RHS.isNegative()) > APInt::udivrem(-LHS, -RHS, Quotient, Remainder); > else > APInt::udivrem(-LHS, RHS, Quotient, Remainder); > Quotient = -Quotient; > Remainder = -Remainder; > } else if (RHS.isNegative()) { > APInt::udivrem(LHS, -RHS, Quotient, Remainder); > Quotient = -Quotient; > } else { > APInt::udivrem(LHS, RHS, Quotient, Remainder); > } > } > > > Here's what it should be: > > static void sdivrem(const APInt &LHS, const APInt &RHS, > APInt &Quotient, APInt &Remainder) { > if (LHS.isNegative()) { > if (RHS.isNegative()) > APInt::udivrem(-LHS, -RHS, Quotient, Remainder); > else { > APInt::udivrem(-LHS, RHS, Quotient, Remainder); > Quotient = -Quotient; > Remainder = -Remainder; > } > } else if (RHS.isNegative()) { > APInt::udivrem(LHS, -RHS, Quotient, Remainder); > Quotient = -Quotient; > } else { > APInt::udivrem(LHS, RHS, Quotient, Remainder); > } > } > > > What should I do to submit the correction?Send a patch to llvm-commits, preferably including a testcase in unittests/ADT/APIntTest.cpp . See http://llvm.org/docs/DeveloperPolicy.html#patches . -Eli