Skip to content

mlir/Presburger: reinstate use of LogicalResult #97415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

artagnon
Copy link
Contributor

@artagnon artagnon commented Jul 2, 2024

Follow up on a desire post-landing d0fee98 (mlir/Presburger: strip dependency on MLIRSupport) to reinstate the use of LogicalResult in Presburger. Since db791b2 (mlir/LogicalResult: move into llvm), LogicalResult is in LLVM, and fulfilling this desire is possible while still maintaining the goal of stripping the Presburger library of mlir dependencies.

Follow up on a desire post-landing d0fee98 (mlir/Presburger: strip
dependency on MLIRSupport) to reinstate the use of LogicalResult in
Presburger. Since db791b2 (mlir/LogicalResult: move into llvm),
LogicalResult is in LLVM, and fulfilling this desire is possible while
still maintaining the goal of stripping the Presburger library of mlir
dependencies.
@llvmbot
Copy link
Member

llvmbot commented Jul 2, 2024

@llvm/pr-subscribers-mlir-presburger

@llvm/pr-subscribers-mlir

Author: Ramkumar Ramachandra (artagnon)

Changes

Follow up on a desire post-landing d0fee98 (mlir/Presburger: strip dependency on MLIRSupport) to reinstate the use of LogicalResult in Presburger. Since db791b2 (mlir/LogicalResult: move into llvm), LogicalResult is in LLVM, and fulfilling this desire is possible while still maintaining the goal of stripping the Presburger library of mlir dependencies.


Patch is 29.85 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97415.diff

7 Files Affected:

  • (modified) mlir/include/mlir/Analysis/Presburger/IntegerRelation.h (+8-4)
  • (modified) mlir/include/mlir/Analysis/Presburger/Simplex.h (+6-6)
  • (modified) mlir/lib/Analysis/FlatLinearValueConstraints.cpp (+2-2)
  • (modified) mlir/lib/Analysis/Presburger/IntegerRelation.cpp (+14-12)
  • (modified) mlir/lib/Analysis/Presburger/PresburgerRelation.cpp (+31-28)
  • (modified) mlir/lib/Analysis/Presburger/Simplex.cpp (+29-28)
  • (modified) mlir/lib/Analysis/Presburger/Utils.cpp (+16-14)
diff --git a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
index 5e5cd898b7518..a27fc8c37eeda 100644
--- a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
+++ b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
@@ -21,13 +21,17 @@
 #include "mlir/Analysis/Presburger/Utils.h"
 #include "llvm/ADT/DynamicAPInt.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/LogicalResult.h"
 #include <optional>
 
 namespace mlir {
 namespace presburger {
 using llvm::DynamicAPInt;
+using llvm::failure;
 using llvm::int64fromDynamicAPInt;
+using llvm::LogicalResult;
 using llvm::SmallVectorImpl;
+using llvm::success;
 
 class IntegerRelation;
 class IntegerPolyhedron;
@@ -478,7 +482,7 @@ class IntegerRelation {
   /// equality detection; if successful, the constant is substituted for the
   /// variable everywhere in the constraint system and then removed from the
   /// system.
-  bool constantFoldVar(unsigned pos);
+  LogicalResult constantFoldVar(unsigned pos);
 
   /// This method calls `constantFoldVar` for the specified range of variables,
   /// `num` variables starting at position `pos`.
@@ -501,7 +505,7 @@ class IntegerRelation {
   /// 3) this   = {0 <= d0 <= 5, 1 <= d1 <= 9}
   ///    other  = {2 <= d0 <= 6, 5 <= d1 <= 15},
   ///    output = {0 <= d0 <= 6, 1 <= d1 <= 15}
-  bool unionBoundingBox(const IntegerRelation &other);
+  LogicalResult unionBoundingBox(const IntegerRelation &other);
 
   /// Returns the smallest known constant bound for the extent of the specified
   /// variable (pos^th), i.e., the smallest known constant that is greater
@@ -774,8 +778,8 @@ class IntegerRelation {
   /// Eliminates a single variable at `position` from equality and inequality
   /// constraints. Returns `success` if the variable was eliminated, and
   /// `failure` otherwise.
-  inline bool gaussianEliminateVar(unsigned position) {
-    return gaussianEliminateVars(position, position + 1) == 1;
+  inline LogicalResult gaussianEliminateVar(unsigned position) {
+    return success(gaussianEliminateVars(position, position + 1) == 1);
   }
 
   /// Removes local variables using equalities. Each equality is checked if it
diff --git a/mlir/include/mlir/Analysis/Presburger/Simplex.h b/mlir/include/mlir/Analysis/Presburger/Simplex.h
index f413636e06910..4c40c4cdcb655 100644
--- a/mlir/include/mlir/Analysis/Presburger/Simplex.h
+++ b/mlir/include/mlir/Analysis/Presburger/Simplex.h
@@ -445,7 +445,7 @@ class LexSimplexBase : public SimplexBase {
   /// lexicopositivity of the basis transform. The row must have a non-positive
   /// sample value. If this is not possible, return failure. This occurs when
   /// the constraints have no solution or the sample value is zero.
-  bool moveRowUnknownToColumn(unsigned row);
+  LogicalResult moveRowUnknownToColumn(unsigned row);
 
   /// Given a row that has a non-integer sample value, add an inequality to cut
   /// away this fractional sample value from the polytope without removing any
@@ -459,7 +459,7 @@ class LexSimplexBase : public SimplexBase {
   ///
   /// Return failure if the tableau became empty, and success if it didn't.
   /// Failure status indicates that the polytope was integer empty.
-  bool addCut(unsigned row);
+  LogicalResult addCut(unsigned row);
 
   /// Undo the addition of the last constraint. This is only called while
   /// rolling back.
@@ -511,7 +511,7 @@ class LexSimplex : public LexSimplexBase {
   MaybeOptimum<SmallVector<Fraction, 8>> getRationalSample() const;
 
   /// Make the tableau configuration consistent.
-  bool restoreRationalConsistency();
+  LogicalResult restoreRationalConsistency();
 
   /// Return whether the specified row is violated;
   bool rowIsViolated(unsigned row) const;
@@ -626,7 +626,7 @@ class SymbolicLexSimplex : public LexSimplexBase {
   /// Return failure if the tableau became empty, indicating that the polytope
   /// is always integer empty in the current symbol domain.
   /// Return success otherwise.
-  bool doNonBranchingPivots();
+  LogicalResult doNonBranchingPivots();
 
   /// Get a row that is always violated in the current domain, if one exists.
   std::optional<unsigned> maybeGetAlwaysViolatedRow();
@@ -647,7 +647,7 @@ class SymbolicLexSimplex : public LexSimplexBase {
   /// at the time of the call. (This function may modify the symbol domain, but
   /// failure statu indicates that the polytope was empty for all symbol values
   /// in the initial domain.)
-  bool addSymbolicCut(unsigned row);
+  LogicalResult addSymbolicCut(unsigned row);
 
   /// Get the numerator of the symbolic sample of the specific row.
   /// This is an affine expression in the symbols with integer coefficients.
@@ -820,7 +820,7 @@ class Simplex : public SimplexBase {
   ///
   /// Returns success if the unknown was successfully restored to a non-negative
   /// sample value, failure otherwise.
-  bool restoreRow(Unknown &u);
+  LogicalResult restoreRow(Unknown &u);
 
   /// Find a pivot to change the sample value of row in the specified
   /// direction while preserving tableau consistency, except that if the
diff --git a/mlir/lib/Analysis/FlatLinearValueConstraints.cpp b/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
index 746cff525beb2..e628fb152b52f 100644
--- a/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
+++ b/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
@@ -1247,10 +1247,10 @@ LogicalResult FlatLinearValueConstraints::unionBoundingBox(
   if (!areVarsAligned(*this, otherCst)) {
     FlatLinearValueConstraints otherCopy(otherCst);
     mergeAndAlignVars(/*offset=*/getNumDimVars(), this, &otherCopy);
-    return success(IntegerPolyhedron::unionBoundingBox(otherCopy));
+    return IntegerPolyhedron::unionBoundingBox(otherCopy);
   }
 
-  return success(IntegerPolyhedron::unionBoundingBox(otherCst));
+  return IntegerPolyhedron::unionBoundingBox(otherCst);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index 6b438692ff6f9..d7a3a933b75dd 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/LogicalResult.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
@@ -1552,22 +1553,22 @@ static int findEqualityToConstant(const IntegerRelation &cst, unsigned pos,
   return -1;
 }
 
-bool IntegerRelation::constantFoldVar(unsigned pos) {
+LogicalResult IntegerRelation::constantFoldVar(unsigned pos) {
   assert(pos < getNumVars() && "invalid position");
   int rowIdx;
   if ((rowIdx = findEqualityToConstant(*this, pos)) == -1)
-    return false;
+    return failure();
 
   // atEq(rowIdx, pos) is either -1 or 1.
   assert(atEq(rowIdx, pos) * atEq(rowIdx, pos) == 1);
   DynamicAPInt constVal = -atEq(rowIdx, getNumCols() - 1) / atEq(rowIdx, pos);
   setAndEliminate(pos, constVal);
-  return true;
+  return success();
 }
 
 void IntegerRelation::constantFoldVarRange(unsigned pos, unsigned num) {
   for (unsigned s = pos, t = pos, e = pos + num; s < e; s++) {
-    if (!constantFoldVar(t))
+    if (constantFoldVar(t).failed())
       t++;
   }
 }
@@ -1944,9 +1945,9 @@ void IntegerRelation::fourierMotzkinEliminate(unsigned pos, bool darkShadow,
   for (unsigned r = 0, e = getNumEqualities(); r < e; r++) {
     if (atEq(r, pos) != 0) {
       // Use Gaussian elimination here (since we have an equality).
-      bool ret = gaussianEliminateVar(pos);
+      LogicalResult ret = gaussianEliminateVar(pos);
       (void)ret;
-      assert(ret && "Gaussian elimination guaranteed to succeed");
+      assert(ret.succeeded() && "Gaussian elimination guaranteed to succeed");
       LLVM_DEBUG(llvm::dbgs() << "FM output (through Gaussian elimination):\n");
       LLVM_DEBUG(dump());
       return;
@@ -2173,7 +2174,8 @@ static void getCommonConstraints(const IntegerRelation &a,
 
 // Computes the bounding box with respect to 'other' by finding the min of the
 // lower bounds and the max of the upper bounds along each of the dimensions.
-bool IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
+LogicalResult
+IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
   assert(space.isEqual(otherCst.getSpace()) && "Spaces should match.");
   assert(getNumLocalVars() == 0 && "local ids not supported yet here");
 
@@ -2201,13 +2203,13 @@ bool IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
     if (!extent.has_value())
       // TODO: symbolic extents when necessary.
       // TODO: handle union if a dimension is unbounded.
-      return false;
+      return failure();
 
     auto otherExtent = otherCst.getConstantBoundOnDimSize(
         d, &otherLb, &otherLbFloorDivisor, &otherUb);
     if (!otherExtent.has_value() || lbFloorDivisor != otherLbFloorDivisor)
       // TODO: symbolic extents when necessary.
-      return false;
+      return success();
 
     assert(lbFloorDivisor > 0 && "divisor always expected to be positive");
 
@@ -2227,7 +2229,7 @@ bool IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
       auto constLb = getConstantBound(BoundType::LB, d);
       auto constOtherLb = otherCst.getConstantBound(BoundType::LB, d);
       if (!constLb.has_value() || !constOtherLb.has_value())
-        return false;
+        return failure();
       std::fill(minLb.begin(), minLb.end(), 0);
       minLb.back() = std::min(*constLb, *constOtherLb);
     }
@@ -2243,7 +2245,7 @@ bool IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
       auto constUb = getConstantBound(BoundType::UB, d);
       auto constOtherUb = otherCst.getConstantBound(BoundType::UB, d);
       if (!constUb.has_value() || !constOtherUb.has_value())
-        return false;
+        return failure();
       std::fill(maxUb.begin(), maxUb.end(), 0);
       maxUb.back() = std::max(*constUb, *constOtherUb);
     }
@@ -2281,7 +2283,7 @@ bool IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
   // union (since the above are just the union along dimensions); we shouldn't
   // be discarding any other constraints on the symbols.
 
-  return true;
+  return success();
 }
 
 bool IntegerRelation::isColZero(unsigned pos) const {
diff --git a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
index 5c4965c919ac3..e284ca82420ba 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/LogicalResult.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <functional>
@@ -753,18 +754,18 @@ class presburger::SetCoalescer {
   ///     \___\|/            \_____/
   ///
   ///
-  bool coalescePairCutCase(unsigned i, unsigned j);
+  LogicalResult coalescePairCutCase(unsigned i, unsigned j);
 
   /// Types the inequality `ineq` according to its `IneqType` for `simp` into
   /// `redundantIneqsB` and `cuttingIneqsB`. Returns success, if no separate
   /// inequalities were encountered. Otherwise, returns failure.
-  bool typeInequality(ArrayRef<DynamicAPInt> ineq, Simplex &simp);
+  LogicalResult typeInequality(ArrayRef<DynamicAPInt> ineq, Simplex &simp);
 
   /// Types the equality `eq`, i.e. for `eq` == 0, types both `eq` >= 0 and
   /// -`eq` >= 0 according to their `IneqType` for `simp` into
   /// `redundantIneqsB` and `cuttingIneqsB`. Returns success, if no separate
   /// inequalities were encountered. Otherwise, returns failure.
-  bool typeEquality(ArrayRef<DynamicAPInt> eq, Simplex &simp);
+  LogicalResult typeEquality(ArrayRef<DynamicAPInt> eq, Simplex &simp);
 
   /// Replaces the element at position `i` with the last element and erases
   /// the last element for both `disjuncts` and `simplices`.
@@ -775,7 +776,7 @@ class presburger::SetCoalescer {
   /// successfully coalesced. The simplices in `simplices` need to be the ones
   /// constructed from `disjuncts`. At this point, there are no empty
   /// disjuncts in `disjuncts` left.
-  bool coalescePair(unsigned i, unsigned j);
+  LogicalResult coalescePair(unsigned i, unsigned j);
 };
 
 /// Constructs a `SetCoalescer` from a `PresburgerRelation`. Only adds non-empty
@@ -818,7 +819,7 @@ PresburgerRelation SetCoalescer::coalesce() {
       cuttingIneqsB.clear();
       if (i == j)
         continue;
-      if (coalescePair(i, j)) {
+      if (coalescePair(i, j).succeeded()) {
         broken = true;
         break;
       }
@@ -902,7 +903,7 @@ void SetCoalescer::addCoalescedDisjunct(unsigned i, unsigned j,
 ///     \___\|/            \_____/
 ///
 ///
-bool SetCoalescer::coalescePairCutCase(unsigned i, unsigned j) {
+LogicalResult SetCoalescer::coalescePairCutCase(unsigned i, unsigned j) {
   /// All inequalities of `b` need to be redundant. We already know that the
   /// redundant ones are, so only the cutting ones remain to be checked.
   Simplex &simp = simplices[i];
@@ -910,7 +911,7 @@ bool SetCoalescer::coalescePairCutCase(unsigned i, unsigned j) {
   if (llvm::any_of(cuttingIneqsA, [this, &simp](ArrayRef<DynamicAPInt> curr) {
         return !isFacetContained(curr, simp);
       }))
-    return false;
+    return failure();
   IntegerRelation newSet(disjunct.getSpace());
 
   for (ArrayRef<DynamicAPInt> curr : redundantIneqsA)
@@ -920,23 +921,25 @@ bool SetCoalescer::coalescePairCutCase(unsigned i, unsigned j) {
     newSet.addInequality(curr);
 
   addCoalescedDisjunct(i, j, newSet);
-  return true;
+  return success();
 }
 
-bool SetCoalescer::typeInequality(ArrayRef<DynamicAPInt> ineq, Simplex &simp) {
+LogicalResult SetCoalescer::typeInequality(ArrayRef<DynamicAPInt> ineq,
+                                           Simplex &simp) {
   Simplex::IneqType type = simp.findIneqType(ineq);
   if (type == Simplex::IneqType::Redundant)
     redundantIneqsB.push_back(ineq);
   else if (type == Simplex::IneqType::Cut)
     cuttingIneqsB.push_back(ineq);
   else
-    return false;
-  return true;
+    return failure();
+  return success();
 }
 
-bool SetCoalescer::typeEquality(ArrayRef<DynamicAPInt> eq, Simplex &simp) {
-  if (!typeInequality(eq, simp))
-    return false;
+LogicalResult SetCoalescer::typeEquality(ArrayRef<DynamicAPInt> eq,
+                                         Simplex &simp) {
+  if (typeInequality(eq, simp).failed())
+    return failure();
   negEqs.push_back(getNegatedCoeffs(eq));
   ArrayRef<DynamicAPInt> inv(negEqs.back());
   return typeInequality(inv, simp);
@@ -951,7 +954,7 @@ void SetCoalescer::eraseDisjunct(unsigned i) {
   simplices.pop_back();
 }
 
-bool SetCoalescer::coalescePair(unsigned i, unsigned j) {
+LogicalResult SetCoalescer::coalescePair(unsigned i, unsigned j) {
 
   IntegerRelation &a = disjuncts[i];
   IntegerRelation &b = disjuncts[j];
@@ -959,7 +962,7 @@ bool SetCoalescer::coalescePair(unsigned i, unsigned j) {
   /// skipped.
   /// TODO: implement local id support.
   if (a.getNumLocalVars() != 0 || b.getNumLocalVars() != 0)
-    return false;
+    return failure();
   Simplex &simpA = simplices[i];
   Simplex &simpB = simplices[j];
 
@@ -969,34 +972,34 @@ bool SetCoalescer::coalescePair(unsigned i, unsigned j) {
   // inequality is encountered during typing, the two IntegerRelations
   // cannot be coalesced.
   for (int k = 0, e = a.getNumInequalities(); k < e; ++k)
-    if (!typeInequality(a.getInequality(k), simpB))
-      return false;
+    if (typeInequality(a.getInequality(k), simpB).failed())
+      return failure();
 
   for (int k = 0, e = a.getNumEqualities(); k < e; ++k)
-    if (!typeEquality(a.getEquality(k), simpB))
-      return false;
+    if (typeEquality(a.getEquality(k), simpB).failed())
+      return failure();
 
   std::swap(redundantIneqsA, redundantIneqsB);
   std::swap(cuttingIneqsA, cuttingIneqsB);
 
   for (int k = 0, e = b.getNumInequalities(); k < e; ++k)
-    if (!typeInequality(b.getInequality(k), simpA))
-      return false;
+    if (typeInequality(b.getInequality(k), simpA).failed())
+      return failure();
 
   for (int k = 0, e = b.getNumEqualities(); k < e; ++k)
-    if (!typeEquality(b.getEquality(k), simpA))
-      return false;
+    if (typeEquality(b.getEquality(k), simpA).failed())
+      return failure();
 
   // If there are no cutting inequalities of `a`, `b` is contained
   // within `a`.
   if (cuttingIneqsA.empty()) {
     eraseDisjunct(j);
-    return true;
+    return success();
   }
 
   // Try to apply the cut case
-  if (coalescePairCutCase(i, j))
-    return true;
+  if (coalescePairCutCase(i, j).succeeded())
+    return success();
 
   // Swap the vectors to compare the pair (j,i) instead of (i,j).
   std::swap(redundantIneqsA, redundantIneqsB);
@@ -1006,7 +1009,7 @@ bool SetCoalescer::coalescePair(unsigned i, unsigned j) {
   // within `a`.
   if (cuttingIneqsA.empty()) {
     eraseDisjunct(i);
-    return true;
+    return success();
   }
 
   // Try to apply the cut case
diff --git a/mlir/lib/Analysis/Presburger/Simplex.cpp b/mlir/lib/Analysis/Presburger/Simplex.cpp
index 4efc7a3755014..bebbf0325f430 100644
--- a/mlir/lib/Analysis/Presburger/Simplex.cpp
+++ b/mlir/lib/Analysis/Presburger/Simplex.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/LogicalResult.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <functional>
@@ -229,7 +230,7 @@ Direction flippedDirection(Direction direction) {
 /// add these to the set of ignored columns and continue to the next row. If we
 /// run out of rows, then A*y is zero and we are done.
 MaybeOptimum<SmallVector<Fraction, 8>> LexSimplex::findRationalLexMin() {
-  if (!restoreRationalConsistency()) {
+  if (restoreRationalConsistency().failed()) {
     markEmpty();
     return OptimumKind::Empty;
   }
@@ -274,7 +275,7 @@ MaybeOptimum<SmallVector<Fraction, 8>> LexSimplex::findRationalLexMin() {
 ///
 /// The constraint is violated when added (it would be useless otherwise)
 /// so we immediately try to move it to a column.
-bool LexSimplexBase::addCut(unsigned row) {
+LogicalResult LexSimplexBase::addCut(unsigned row) {
   DynamicAPInt d = tableau(row, 0);
   unsigned cutRow = addZeroRow(/*makeRestricted=*/true);
   tableau(cutRow, 0) = d;
@@ -301,7 +302,7 @@ std::optional<unsigned> LexSimplex::maybeGetNonIntegralVarRow() const {
 
 MaybeOptimum<SmallVector<DynamicAPInt, 8>> LexSimplex::findIntegerLexMin() {
   // We first try to make the tableau consistent.
-  if (!restoreRationalConsistency())
+  if (restoreRationalConsistency().failed())
     return OptimumKind::Empty;
 
   // Then, if the sample value is integral, we are done.
@@ -316,9 +317,9 @@ MaybeOptimum<SmallVector<DynamicAPInt, 8>> LexSimplex::findIntegerLexMin() {
     //
     // Failure indicates that the tableau became empty, which occurs when the
     // polytope is integer empty.
-    if (!addCut(*maybeRow))
+    if (addCut(*maybeRow).failed())
       return OptimumKind::Empty;
-    if (!restoreRationalConsistency())
+    if (restoreRationalConsistency().failed())
       return OptimumKind::Empty;
   }
 
@@ -411,7 +412,7 @@ bool SymbolicLexSimplex::isSymbolicSampleIntegral(unsigned row) const {
 /// (sum_i (b_i%d)y_i - (-c%d) - sum_i (-a_i%d)s_i + q*d)/d >= 0
 /// This constraint is violated when added so we immediately try to move it to a
 /// column.
-bool SymbolicLexSimplex::addSymbolicCut(unsigned row) {
+LogicalResult SymbolicLexSimplex::addSymbolicCut(unsigned row) {
   DynamicAPInt d = tableau(row, 0);
   if (isRangeDivisibleBy(tableau.getRow(row).slice(3, nSymbol), d)) {
     // The coefficients of symbols in the symbol numerator are divisible
@@ -523,11 +524,11 @@ std::optional<unsigned> SymbolicLexSimplex::maybeGetNonIntegralVarRow() {
 
 /// The non-branching pivots are just the ones moving the rows
 /// that are always violated in the symbol domain.
-bool SymbolicLexSimplex::doNonBranchingPivots() {
+LogicalResult SymbolicLexSimplex::doNonBranchingPivots() {
   while (std::optional<unsigned> row = maybeGetAlwaysViolatedRow())
-    if ...
[truncated]

Copy link
Member

@Superty Superty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this! Please make sure the part I pointed out is correct before landing

@artagnon artagnon merged commit f819302 into llvm:main Jul 3, 2024
7 checks passed
@artagnon artagnon deleted the presburger-logicalresult branch July 3, 2024 09:51
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
Follow up on a desire post-landing d0fee98 (mlir/Presburger: strip
dependency on MLIRSupport) to reinstate the use of LogicalResult in
Presburger. Since db791b2 (mlir/LogicalResult: move into llvm),
LogicalResult is in LLVM, and fulfilling this desire is possible while
still maintaining the goal of stripping the Presburger library of mlir
dependencies.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Follow up on a desire post-landing d0fee98 (mlir/Presburger: strip
dependency on MLIRSupport) to reinstate the use of LogicalResult in
Presburger. Since db791b2 (mlir/LogicalResult: move into llvm),
LogicalResult is in LLVM, and fulfilling this desire is possible while
still maintaining the goal of stripping the Presburger library of mlir
dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants