[PATCH] D135541: [TargetLowering][RISCV][X86] Support even divisors in expandDIVREMByConstant.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7327
   APInt Mod = APInt::getSignedMinValue(BitWidth + 1);
   APInt MulFactor = Divisor.zext(BitWidth + 1);
   MulFactor = MulFactor.multiplicativeInverse(Mod);
----------------
craig.topper wrote:
> efriedma wrote:
> > craig.topper wrote:
> > > craig.topper wrote:
> > > > jrtc27 wrote:
> > > > > Should this not be using the original divisor (whether by shifting back by TrailingZeros or making a copy you use elsewhere instead of doing an lshrInPlace)?
> > > > Yes! Thank you, that's probably the bug.
> > > Well I think that just exposed worse problems in the algorithm. The multiplicative inverse doesn't exist if Divisor is even. It needs to be coprime with 1<<BitWidth. But they will both have 2 as a common factor.
> > After the subtraction, you have something like "udiv exact X, Y".  So you can use the same algorithm as BuildExactSDIV (i.e. shift right X and Y, then multiply X by the inverse of Y).
> I already shifted the dividend and the divisor to get the remainder. I think I should subtract the uncorrected remainder from the shifted dividend and do the multiplicative inverse to calculate the quotient on that. Then correct the remainder with the part shifted off earlier.
Yes, that's equivalent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135541/new/

https://reviews.llvm.org/D135541