[PATCH] D80860: Exact integer emptiness checks for FlatAffineConstraints

[Tobias Grosser via Phabricator via llvm-commits]

grosser added a comment.

Hi @arjunp, I am wondering if there are ways to prevent future coverity warnings on this code?



================
Comment at: mlir/lib/Analysis/Presburger/Simplex.cpp:131
+  for (unsigned col = 0; col < nCol; ++col)
+    tableau(row, col) /= gcd;
+}
----------------
arjunp wrote:
> mehdi_amini wrote:
> > What guarantees that `gcd` can't be 0 and produce an invalid division here?
> > Should we guard this loop?
> > 
> > (flagged by Coverity)
> One of the elements being GCD'd is a "denominator" value that is always non-zero. I will add a comment noting this.
Given that these are coverity errors, it might be useful to document this with an assert to ensure that coverity does not warn about this anymore.


================
Comment at: mlir/lib/Analysis/Presburger/Simplex.cpp:325
+
+    int64_t diff = retConst * elem - constTerm * retElem;
+    if ((diff == 0 && rowUnknown[row] < rowUnknown[*retRow]) ||
----------------
arjunp wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > If `retRow` is true, then `retConst` is used uninitialized right? Can you look into this?
> > > (flagged by Coverity)
> > (Same with  `retElem`)
> If `retRow` has a value then lines 319-322 were executed in the iteration that that values was set in, so the values of `retElem` and `retConst` were also set in that iteration already.
Again, as these are coverity errors the question is how we can avoid coverity to flag these issues. I am almost tempted to just suggest to set retElement and retConst to 0 and add a comment stating that this is done to avoid the coverity warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80860