[PATCH] D28582: [SCEV] Simplify SolveLinEquationWithOverflow a bit.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

Your change looks correct to me, but I don't know why we chose to express N/D as a BW+1 bit integer in the first place.



================
Comment at: lib/Analysis/ScalarEvolution.cpp:7035
   //
   // (N / D) may need BW+1 bits in its representation.  Hence, we'll use this
   // bit width during computations.
----------------
I don't understand why "(N / D) may need BW+1 bits in its representation" is true, btw.  It seems to me `N/D` should always fit in `BW` bits.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:7040
   Mod.setBit(BW - Mult2);  // Mod = N / D
-  APInt I = AD.multiplicativeInverse(Mod);
+  // I is guaranteed to fit into BW bits, so truncate it.
+  APInt I = AD.multiplicativeInverse(Mod).trunc(BW);
----------------
Add an assert here?


Repository:
  rL LLVM

https://reviews.llvm.org/D28582