[PATCH] D80860: Exact integer emptiness checks for FlatAffineConstraints

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula requested changes to this revision.
bondhugula added inline comments.
This revision now requires changes to proceed.


================
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);
+}
----------------
arjunp wrote:
> 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)
If `den` is positive, this can be simplified to avoid an extra check. 
```
f.num % f.den > 0 ? f.num / f.den + 1 : f.num / f.den;
```
(one comparison instead of two comparisons)

Also, if there is a doc comment here, please mention that `den` is positive. Actually, you don't even need this logic; mlir::ceilDiv does exactly this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80860