[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/lib/Analysis/Presburger/Simplex.cpp:907
+  int64_t minRoundedUp;
+  if (auto opt = computeOptimum(Simplex::Direction::Down, coeffs))
+    minRoundedUp = ceil(*opt);
----------------
Please avoid `auto` here and similarly below.


================
Comment at: mlir/lib/Analysis/Presburger/Simplex.cpp:944
+Optional<SmallVector<int64_t, 8>>
+Simplex::findIntegerSampleRecursively(Matrix &basis, unsigned level) {
+  if (level == basis.getNumRows())
----------------
Please do not return `SmallVector`s this way - return value optimization here will not happen I believe leading to too many copies. Instead pass SmallVectorImpl<int64_t> by reference as an output argument. @mehdi_amini or @rriddle  can confirm on the style part and RVO.


================
Comment at: mlir/unittests/Analysis/AffineStructuresTest.cpp:34
+void checkSample(bool hasValue, const FlatAffineConstraints &fac) {
+  auto maybeSample = fac.findIntegerSample();
+  if (!hasValue) {
----------------
Avoid `auto` here for readability.


================
Comment at: mlir/unittests/Analysis/AffineStructuresTest.cpp:38
+    if (maybeSample.hasValue()) {
+      for (auto x : *maybeSample)
+        llvm::errs() << x << ' ';
----------------
Avoid `auto`.


================
Comment at: mlir/unittests/Analysis/AffineStructuresTest.cpp:114
+
+  // x <= 10 and y <= 10 and 11 <= z and x + 2y = 3z
+  // This implies x + 2y >= 33 and x + 2y <= 30, which has no solution.
----------------
Please terminate comments with a period when it ends.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80860