[PATCH] D80860: Exact integer emptiness checks for FlatAffineConstraints

[Arjun P via Phabricator via llvm-commits]

arjunp marked 2 inline comments as done.
arjunp added inline comments.


================
Comment at: mlir/include/mlir/Analysis/Presburger/Fraction.h:65
+inline int64_t ceil(Fraction f) {
+  return f.num / f.den + (f.num > 0 && f.num % f.den != 0);
+}
----------------
bondhugula wrote:
> Are you assuming f.den is always positive? If both `num` and `den` are both negative, you'd want to add one.
Yes, `den` is always positive. There was an incomplete comment for the member declaration that I've fixed. (This is also documented in the top-level class documentation)


================
Comment at: mlir/unittests/Analysis/AffineStructuresTest.cpp:186
+                         {
+                             {3, -3, 1} // 3x - 3y + 1 = 0, i.e., y = x + 1/3.
+                         }));
----------------
bondhugula wrote:
> I'm sorry this would be caught by the GCD test itself. Instead, could you please replace the equality with something to the effect:
> 
> ```
> 3x - 3y >= -2
> 3x - 3y  <= -1
> ```
> This should be empty as well but would exercise your method given how `isIntegerEmpty()` is structured now. 
Since `checkSample` directly calls `findIntegerSample()` rather than `isIntegerEmpty()`, this shouldn't be an issue. Nonetheless, I've also added the test you requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80860