[PATCH] D80860: Exact integer emptiness checks for FlatAffineConstraints

Tue Jun 2 01:05:01 PDT 2020

arjunp marked an inline comment as done.
arjunp added inline comments.

================
Comment at: mlir/include/mlir/Analysis/Presburger/Matrix.h:66
+  unsigned nColumns;
+  std::vector<std::vector<INT>> data;
+};
----------------
arjunp wrote:
> Kayjukh wrote:
> > arjunp wrote:
> > > Kayjukh wrote:
> > > > Why not store the matrix data in a one-dimensional array?
> > > This makes the code simpler. In that case, resizing would be more expensive, and resizing is not uncommon, so it's unclear to me if performance considerations weigh in favour of having it be one-dimensional.
> > If resizing is a common operation then I agree that it can make sense to keep nested `SmallVector`s. If there is a noticeable performance impact in can always be fixed in a follow-up.
> In fact, resizing wouldn't be a problem because we only resize vertically, i.e. we only ever change the number of columns. But another operation that's used frequently is `swapRows` which should also be faster under this representation, so I think we can still keep it this way for now.
Sorry, I meant "we only ever change the number of rows".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80860