mlir/Presburger: strip dependency on MLIRSupport #96517

artagnon · 2024-06-24T17:13:51Z

Strip the Presburger library's dependency on the MLIR Support library, as well as the headers, in the interest of making it leaner.

This patch is part of a project to move the Presburger library into LLVM.

Strip the Presburger library's dependency on the MLIR Support library, as well as the headers, in the interest of making it leaner. This patch is part of a project to move the Presburger library into LLVM.

llvmbot · 2024-06-24T17:14:21Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-presburger

Author: Ramkumar Ramachandra (artagnon)

Changes

Strip the Presburger library's dependency on the MLIR Support library, as well as the headers, in the interest of making it leaner.

This patch is part of a project to move the Presburger library into LLVM.

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

18 Files Affected:

(modified) mlir/include/mlir/Analysis/Presburger/Barvinok.h (-1)
(modified) mlir/include/mlir/Analysis/Presburger/IntegerRelation.h (+6-5)
(modified) mlir/include/mlir/Analysis/Presburger/Matrix.h (+4-3)
(modified) mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h (+7-5)
(modified) mlir/include/mlir/Analysis/Presburger/Simplex.h (+6-11)
(modified) mlir/include/mlir/Analysis/Presburger/Utils.h (+1-3)
(modified) mlir/lib/Analysis/FlatLinearValueConstraints.cpp (+3-2)
(modified) mlir/lib/Analysis/Presburger/Barvinok.cpp (-1)
(modified) mlir/lib/Analysis/Presburger/CMakeLists.txt (+1-5)
(modified) mlir/lib/Analysis/Presburger/IntegerRelation.cpp (+12-16)
(modified) mlir/lib/Analysis/Presburger/LinearTransform.cpp (-1)
(modified) mlir/lib/Analysis/Presburger/Matrix.cpp (-1)
(modified) mlir/lib/Analysis/Presburger/PWMAFunction.cpp (-1)
(modified) mlir/lib/Analysis/Presburger/PresburgerRelation.cpp (+30-39)
(modified) mlir/lib/Analysis/Presburger/QuasiPolynomial.cpp (-1)
(modified) mlir/lib/Analysis/Presburger/Simplex.cpp (+28-30)
(modified) mlir/lib/Analysis/Presburger/Utils.cpp (+14-22)
(modified) mlir/unittests/Analysis/Presburger/Utils.h (-5)

diff --git a/mlir/include/mlir/Analysis/Presburger/Barvinok.h b/mlir/include/mlir/Analysis/Presburger/Barvinok.h
index c9a1645b5e632..b09617f0fe88e 100644
--- a/mlir/include/mlir/Analysis/Presburger/Barvinok.h
+++ b/mlir/include/mlir/Analysis/Presburger/Barvinok.h
@@ -29,7 +29,6 @@
 #include "mlir/Analysis/Presburger/Matrix.h"
 #include "mlir/Analysis/Presburger/PresburgerRelation.h"
 #include "mlir/Analysis/Presburger/QuasiPolynomial.h"
-#include <bitset>
 #include <optional>
 
 namespace mlir {
diff --git a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
index ad26e9786d0b0..5e5cd898b7518 100644
--- a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
+++ b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
@@ -19,14 +19,15 @@
 #include "mlir/Analysis/Presburger/Matrix.h"
 #include "mlir/Analysis/Presburger/PresburgerSpace.h"
 #include "mlir/Analysis/Presburger/Utils.h"
-#include "mlir/Support/LogicalResult.h"
 #include "llvm/ADT/DynamicAPInt.h"
+#include "llvm/ADT/SmallVector.h"
 #include <optional>
 
 namespace mlir {
 namespace presburger {
 using llvm::DynamicAPInt;
 using llvm::int64fromDynamicAPInt;
+using llvm::SmallVectorImpl;
 
 class IntegerRelation;
 class IntegerPolyhedron;
@@ -477,7 +478,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.
-  LogicalResult constantFoldVar(unsigned pos);
+  bool constantFoldVar(unsigned pos);
 
   /// This method calls `constantFoldVar` for the specified range of variables,
   /// `num` variables starting at position `pos`.
@@ -500,7 +501,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}
-  LogicalResult unionBoundingBox(const IntegerRelation &other);
+  bool 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
@@ -773,8 +774,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 LogicalResult gaussianEliminateVar(unsigned position) {
-    return success(gaussianEliminateVars(position, position + 1) == 1);
+  inline bool gaussianEliminateVar(unsigned position) {
+    return gaussianEliminateVars(position, position + 1) == 1;
   }
 
   /// Removes local variables using equalities. Each equality is checked if it
diff --git a/mlir/include/mlir/Analysis/Presburger/Matrix.h b/mlir/include/mlir/Analysis/Presburger/Matrix.h
index e232ecd5e1509..054eb7b26d06e 100644
--- a/mlir/include/mlir/Analysis/Presburger/Matrix.h
+++ b/mlir/include/mlir/Analysis/Presburger/Matrix.h
@@ -16,15 +16,16 @@
 #define MLIR_ANALYSIS_PRESBURGER_MATRIX_H
 
 #include "mlir/Analysis/Presburger/Fraction.h"
-#include "mlir/Support/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/raw_ostream.h"
-
-#include <bitset>
 #include <cassert>
 
 namespace mlir {
 namespace presburger {
+using llvm::ArrayRef;
+using llvm::MutableArrayRef;
+using llvm::raw_ostream;
+using llvm::SmallVector;
 
 /// This is a class to represent a resizable matrix.
 ///
diff --git a/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h b/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
index 9a41e4aee61d0..cff7957989871 100644
--- a/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
+++ b/mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
@@ -14,14 +14,16 @@
 #ifndef MLIR_ANALYSIS_PRESBURGER_PRESBURGERSPACE_H
 #define MLIR_ANALYSIS_PRESBURGER_PRESBURGERSPACE_H
 
-#include "mlir/Support/TypeID.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/Support/ErrorHandling.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
+#include "llvm/Support/TypeName.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace mlir {
 namespace presburger {
+using llvm::ArrayRef;
+using llvm::SmallVector;
 
 /// Kind of variable. Implementation wise SetDims are treated as Range
 /// vars, and spaces with no distinction between dimension vars are treated
@@ -74,7 +76,7 @@ class Identifier {
   explicit Identifier(T value)
       : value(llvm::PointerLikeTypeTraits<T>::getAsVoidPointer(value)) {
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-    idType = TypeID::get<T>();
+    idType = llvm::getTypeName<T>();
 #endif
   }
 
@@ -83,7 +85,7 @@ class Identifier {
   template <typename T>
   T getValue() const {
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
-    assert(TypeID::get<T>() == idType &&
+    assert(llvm::getTypeName<T>() == idType &&
            "Identifier was initialized with a different type than the one used "
            "to retrieve it.");
 #endif
@@ -108,7 +110,7 @@ class Identifier {
 
 #ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
   /// TypeID of the identifiers in space. This should be used in asserts only.
-  TypeID idType = TypeID::get<void>();
+  llvm::StringRef idType;
 #endif
 };
 
diff --git a/mlir/include/mlir/Analysis/Presburger/Simplex.h b/mlir/include/mlir/Analysis/Presburger/Simplex.h
index ff26e94e019c8..f413636e06910 100644
--- a/mlir/include/mlir/Analysis/Presburger/Simplex.h
+++ b/mlir/include/mlir/Analysis/Presburger/Simplex.h
@@ -20,12 +20,7 @@
 #include "mlir/Analysis/Presburger/Matrix.h"
 #include "mlir/Analysis/Presburger/PWMAFunction.h"
 #include "mlir/Analysis/Presburger/Utils.h"
-#include "mlir/Support/LogicalResult.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallBitVector.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/StringSaver.h"
-#include "llvm/Support/raw_ostream.h"
 #include <optional>
 
 namespace mlir {
@@ -450,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.
-  LogicalResult moveRowUnknownToColumn(unsigned row);
+  bool 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
@@ -464,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.
-  LogicalResult addCut(unsigned row);
+  bool addCut(unsigned row);
 
   /// Undo the addition of the last constraint. This is only called while
   /// rolling back.
@@ -516,7 +511,7 @@ class LexSimplex : public LexSimplexBase {
   MaybeOptimum<SmallVector<Fraction, 8>> getRationalSample() const;
 
   /// Make the tableau configuration consistent.
-  LogicalResult restoreRationalConsistency();
+  bool restoreRationalConsistency();
 
   /// Return whether the specified row is violated;
   bool rowIsViolated(unsigned row) const;
@@ -631,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.
-  LogicalResult doNonBranchingPivots();
+  bool doNonBranchingPivots();
 
   /// Get a row that is always violated in the current domain, if one exists.
   std::optional<unsigned> maybeGetAlwaysViolatedRow();
@@ -652,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.)
-  LogicalResult addSymbolicCut(unsigned row);
+  bool 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.
@@ -825,7 +820,7 @@ class Simplex : public SimplexBase {
   ///
   /// Returns success if the unknown was successfully restored to a non-negative
   /// sample value, failure otherwise.
-  LogicalResult restoreRow(Unknown &u);
+  bool 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/include/mlir/Analysis/Presburger/Utils.h b/mlir/include/mlir/Analysis/Presburger/Utils.h
index 9b93e52b48490..d3c0802c240bc 100644
--- a/mlir/include/mlir/Analysis/Presburger/Utils.h
+++ b/mlir/include/mlir/Analysis/Presburger/Utils.h
@@ -13,12 +13,10 @@
 #ifndef MLIR_ANALYSIS_PRESBURGER_UTILS_H
 #define MLIR_ANALYSIS_PRESBURGER_UTILS_H
 
-#include "mlir/Support/LLVM.h"
+#include "mlir/Analysis/Presburger/Matrix.h"
 #include "llvm/ADT/DynamicAPInt.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallBitVector.h"
-
-#include "mlir/Analysis/Presburger/Matrix.h"
 #include <optional>
 
 namespace mlir {
diff --git a/mlir/lib/Analysis/FlatLinearValueConstraints.cpp b/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
index e628fb152b52f..18047a613c620 100644
--- a/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
+++ b/mlir/lib/Analysis/FlatLinearValueConstraints.cpp
@@ -16,6 +16,7 @@
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/IntegerSet.h"
 #include "mlir/Support/LLVM.h"
+#include "mlir/Support/LogicalResult.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -1247,10 +1248,10 @@ LogicalResult FlatLinearValueConstraints::unionBoundingBox(
   if (!areVarsAligned(*this, otherCst)) {
     FlatLinearValueConstraints otherCopy(otherCst);
     mergeAndAlignVars(/*offset=*/getNumDimVars(), this, &otherCopy);
-    return IntegerPolyhedron::unionBoundingBox(otherCopy);
+    return success(IntegerPolyhedron::unionBoundingBox(otherCopy));
   }
 
-  return IntegerPolyhedron::unionBoundingBox(otherCst);
+  return success(IntegerPolyhedron::unionBoundingBox(otherCst));
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Analysis/Presburger/Barvinok.cpp b/mlir/lib/Analysis/Presburger/Barvinok.cpp
index e16b9269b75fc..dae840e00ff2e 100644
--- a/mlir/lib/Analysis/Presburger/Barvinok.cpp
+++ b/mlir/lib/Analysis/Presburger/Barvinok.cpp
@@ -10,7 +10,6 @@
 #include "mlir/Analysis/Presburger/Utils.h"
 #include "llvm/ADT/Sequence.h"
 #include <algorithm>
-#include <bitset>
 
 using namespace mlir;
 using namespace presburger;
diff --git a/mlir/lib/Analysis/Presburger/CMakeLists.txt b/mlir/lib/Analysis/Presburger/CMakeLists.txt
index 1d30dd38ccd1b..6b9842a6160ab 100644
--- a/mlir/lib/Analysis/Presburger/CMakeLists.txt
+++ b/mlir/lib/Analysis/Presburger/CMakeLists.txt
@@ -8,8 +8,4 @@ add_mlir_library(MLIRPresburger
   PWMAFunction.cpp
   QuasiPolynomial.cpp
   Simplex.cpp
-  Utils.cpp
-
-  LINK_LIBS PUBLIC
-  MLIRSupport
-  )
+  Utils.cpp)
diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index 75215fbab5282..10c6a023cef40 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -20,8 +20,6 @@
 #include "mlir/Analysis/Presburger/PresburgerSpace.h"
 #include "mlir/Analysis/Presburger/Simplex.h"
 #include "mlir/Analysis/Presburger/Utils.h"
-#include "mlir/Support/LLVM.h"
-#include "mlir/Support/LogicalResult.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
@@ -1554,22 +1552,22 @@ static int findEqualityToConstant(const IntegerRelation &cst, unsigned pos,
   return -1;
 }
 
-LogicalResult IntegerRelation::constantFoldVar(unsigned pos) {
+bool IntegerRelation::constantFoldVar(unsigned pos) {
   assert(pos < getNumVars() && "invalid position");
   int rowIdx;
   if ((rowIdx = findEqualityToConstant(*this, pos)) == -1)
-    return failure();
+    return false;
 
   // 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 success();
+  return true;
 }
 
 void IntegerRelation::constantFoldVarRange(unsigned pos, unsigned num) {
   for (unsigned s = pos, t = pos, e = pos + num; s < e; s++) {
-    if (failed(constantFoldVar(t)))
+    if (!constantFoldVar(t))
       t++;
   }
 }
@@ -1946,9 +1944,8 @@ 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).
-      LogicalResult ret = gaussianEliminateVar(pos);
-      (void)ret;
-      assert(succeeded(ret) && "Gaussian elimination guaranteed to succeed");
+      bool ret = gaussianEliminateVar(pos);
+      assert(ret && "Gaussian elimination guaranteed to succeed");
       LLVM_DEBUG(llvm::dbgs() << "FM output (through Gaussian elimination):\n");
       LLVM_DEBUG(dump());
       return;
@@ -2175,8 +2172,7 @@ 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.
-LogicalResult
-IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
+bool IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
   assert(space.isEqual(otherCst.getSpace()) && "Spaces should match.");
   assert(getNumLocalVars() == 0 && "local ids not supported yet here");
 
@@ -2204,13 +2200,13 @@ IntegerRelation::unionBoundingBox(const IntegerRelation &otherCst) {
     if (!extent.has_value())
       // TODO: symbolic extents when necessary.
       // TODO: handle union if a dimension is unbounded.
-      return failure();
+      return false;
 
     auto otherExtent = otherCst.getConstantBoundOnDimSize(
         d, &otherLb, &otherLbFloorDivisor, &otherUb);
     if (!otherExtent.has_value() || lbFloorDivisor != otherLbFloorDivisor)
       // TODO: symbolic extents when necessary.
-      return failure();
+      return false;
 
     assert(lbFloorDivisor > 0 && "divisor always expected to be positive");
 
@@ -2230,7 +2226,7 @@ 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 failure();
+        return false;
       std::fill(minLb.begin(), minLb.end(), 0);
       minLb.back() = std::min(*constLb, *constOtherLb);
     }
@@ -2246,7 +2242,7 @@ 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 failure();
+        return false;
       std::fill(maxUb.begin(), maxUb.end(), 0);
       maxUb.back() = std::max(*constUb, *constOtherUb);
     }
@@ -2284,7 +2280,7 @@ 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 success();
+  return true;
 }
 
 bool IntegerRelation::isColZero(unsigned pos) const {
diff --git a/mlir/lib/Analysis/Presburger/LinearTransform.cpp b/mlir/lib/Analysis/Presburger/LinearTransform.cpp
index ecab634967694..cccbf4c9991d3 100644
--- a/mlir/lib/Analysis/Presburger/LinearTransform.cpp
+++ b/mlir/lib/Analysis/Presburger/LinearTransform.cpp
@@ -9,7 +9,6 @@
 #include "mlir/Analysis/Presburger/LinearTransform.h"
 #include "mlir/Analysis/Presburger/IntegerRelation.h"
 #include "mlir/Analysis/Presburger/Matrix.h"
-#include "mlir/Support/LLVM.h"
 #include <utility>
 
 using namespace mlir;
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 134b805648d9f..110c5df1af37c 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -9,7 +9,6 @@
 #include "mlir/Analysis/Presburger/Matrix.h"
 #include "mlir/Analysis/Presburger/Fraction.h"
 #include "mlir/Analysis/Presburger/Utils.h"
-#include "mlir/Support/LLVM.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
diff --git a/mlir/lib/Analysis/Presburger/PWMAFunction.cpp b/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
index 664670d506d53..f78eb7d2d98ce 100644
--- a/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
+++ b/mlir/lib/Analysis/Presburger/PWMAFunction.cpp
@@ -11,7 +11,6 @@
 #include "mlir/Analysis/Presburger/PresburgerRelation.h"
 #include "mlir/Analysis/Presburger/PresburgerSpace.h"
 #include "mlir/Analysis/Presburger/Utils.h"
-#include "mlir/Support/LLVM.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallVector.h"
diff --git a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
index 6173f774d0475..5c4965c919ac3 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerRelation.cpp
@@ -12,8 +12,6 @@
 #include "mlir/Analysis/Presburger/PresburgerSpace.h"
 #include "mlir/Analysis/Presburger/Simplex.h"
 #include "mlir/Analysis/Presburger/Utils.h"
-#include "mlir/Support/LLVM.h"
-#include "mlir/Support/LogicalResult.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallVector.h"
@@ -755,18 +753,18 @@ class presburger::SetCoalescer {
   ///     \___\|/            \_____/
   ///
   ///
-  LogicalResult coalescePairCutCase(unsigned i, unsigned j);
+  bool 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.
-  LogicalResult typeInequality(ArrayRef<DynamicAPInt> ineq, Simplex &simp);
+  bool 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.
-  LogicalResult typeEquality(ArrayRef<DynamicAPInt> eq, Simplex &simp);
+  bool 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`.
@@ -777,7 +775,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.
-  LogicalResult coalescePair(unsigned i, unsigned j);
+  bool coalescePair(unsigned i, unsigned j);
 };
 
 /// Constructs a `SetCoalescer` from a `PresburgerRelation`. Only adds non-empty
@@ -820,7 +818,7 @@ PresburgerRelation SetCoalescer::coalesce() {
       cuttingIneqsB.clear();
       if (i == j)
         continue;
-      if (coalescePair(i, j).succeeded()) {
+      if (coalescePair(i, j)) {
         broken = true;
         break;
       }
@@ -904,7 +902,7 @@ void SetCoalescer::addCoalescedDisjunct(unsigned i, unsigned j,
 ///     \___\|/            \_____/
 ///
 ///
-LogicalResult SetCoalescer::coalescePairCutCase(unsigned i, unsigned j) {
+bool 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];
@@ -912,7 +910,7 @@ LogicalResult SetCoalescer::coalescePairCutCase(unsigned i, unsigned j) {
   if (llvm::any_of(cuttingIneqsA, [this, &simp](ArrayRef<DynamicAPInt> curr) {
         ...
[truncated]

Groverkss

Can we depend on LLVMSupport instead?

artagnon · 2024-06-24T17:35:52Z

We already are depending on LLVMSupport implicitly (add_mlir_library in MLIR CMake already includes relevant LLVM libs). Further, we do include a lot of LLVM Support headers already.

artagnon · 2024-06-24T17:38:53Z

Further, the only reason we depend on MLIRSupport is for TypeID.cpp, which is a completely unnecessary dependency. The only other Support headers we depend are LLVM.h and LogicalResult.h, which are also eliminated trivially.

nikic

LGTM

mlir/lib/Analysis/Presburger/IntegerRelation.cpp

Superty

Thanks for your patch. Please fix these two issues:

many comments refer to returns failure if... or returns success if.... Please fix to say returns true/false.
some functions don't say anything about the return but the type of LogicalResult previously made it clear that the return denotes success/failure, which now becomes unclear. Please add a line to the header doc in such cases mentioning that the function returns true if successful.

joker-eph · 2024-06-30T21:04:38Z

some functions don't say anything about the return but the type of LogicalResult previously made it clear that the return denotes success/failure, which now becomes unclear. Please add a line to the header doc in such cases mentioning that the function returns true if successful.

Better: can we please keep LogicalResult!! (move the header to LLVMSupport instead)

Strip the Presburger library's dependency on the MLIR Support library, as well as the headers, in the interest of making it leaner. This patch is part of a project to move the Presburger library into LLVM.

mlir/Presburger: strip dependency on MLIRSupport

68155dd

Strip the Presburger library's dependency on the MLIR Support library, as well as the headers, in the interest of making it leaner. This patch is part of a project to move the Presburger library into LLVM.

artagnon requested review from ftynse and joker-eph June 24, 2024 17:13

artagnon requested review from Groverkss and Superty as code owners June 24, 2024 17:13

llvmbot added mlir mlir:presburger labels Jun 24, 2024

Groverkss requested changes Jun 24, 2024

View reviewed changes

artagnon requested review from jayfoad and nikic June 26, 2024 21:11

nikic approved these changes Jun 29, 2024

View reviewed changes

mlir/lib/Analysis/Presburger/IntegerRelation.cpp Show resolved Hide resolved

mlir/Presburger: squelch unused var warning

e0a1871

artagnon merged commit d0fee98 into llvm:main Jun 29, 2024
4 of 6 checks passed

artagnon deleted the presburger-strip-support branch June 29, 2024 11:23

Superty reviewed Jun 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mlir/Presburger: strip dependency on MLIRSupport #96517

mlir/Presburger: strip dependency on MLIRSupport #96517

Uh oh!

artagnon commented Jun 24, 2024

Uh oh!

llvmbot commented Jun 24, 2024 •

edited

Loading

Uh oh!

Groverkss left a comment

Uh oh!

artagnon commented Jun 24, 2024

Uh oh!

artagnon commented Jun 24, 2024

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Uh oh!

Superty left a comment

Uh oh!

joker-eph commented Jun 30, 2024

Uh oh!

Uh oh!

mlir/Presburger: strip dependency on MLIRSupport #96517

mlir/Presburger: strip dependency on MLIRSupport #96517

Uh oh!

Conversation

artagnon commented Jun 24, 2024

Uh oh!

llvmbot commented Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Groverkss left a comment

Choose a reason for hiding this comment

Uh oh!

artagnon commented Jun 24, 2024

Uh oh!

artagnon commented Jun 24, 2024

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Superty left a comment

Choose a reason for hiding this comment

Uh oh!

joker-eph commented Jun 30, 2024

Uh oh!

Uh oh!

llvmbot commented Jun 24, 2024 •

edited

Loading