[Mlir-commits] [mlir] [MLIR][Presburger] Preserve identifiers in IntegerRelation::convertVa… (PR #67909)

Bharathi Ramana Joshi llvmlistbot at llvm.org
Thu Nov 2 20:27:18 PDT 2023


https://github.com/iambrj updated https://github.com/llvm/llvm-project/pull/67909

>From 79db644b56bc02e728c7925308b1bf228f3881ce Mon Sep 17 00:00:00 2001
From: iambrj <joshibharathiramana at gmail.com>
Date: Fri, 6 Oct 2023 02:09:58 +0400
Subject: [PATCH 1/2] [MLIR][Presburger] Implement Matrix::moveColumns

---
 .../include/mlir/Analysis/Presburger/Matrix.h | 17 +++++++
 mlir/lib/Analysis/Presburger/Matrix.cpp       | 33 +++++++++++-
 .../Analysis/Presburger/MatrixTest.cpp        | 51 +++++++++++++++++--
 3 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/mlir/include/mlir/Analysis/Presburger/Matrix.h b/mlir/include/mlir/Analysis/Presburger/Matrix.h
index 4d9f13832e0692a..f193504ec80e9fd 100644
--- a/mlir/include/mlir/Analysis/Presburger/Matrix.h
+++ b/mlir/include/mlir/Analysis/Presburger/Matrix.h
@@ -189,6 +189,23 @@ class Matrix {
   /// invariants satisfied.
   bool hasConsistentState() const;
 
+  /// Shift the columns in the source range [srcPos, srcPos + num) to the
+  /// specified destination, i.e. to [dstPos, dstPos + num), while moving the
+  /// columns adjacent to the source range to the left/right of the shifted
+  /// columns.
+  ///
+  /// When shifting the source columns right (i.e. dstPos > srcPos), columns
+  /// that were at positions [0, srcPos) will stay where they are; columns that
+  /// were at positions [srcPos, srcPos + num) will be moved to
+  /// [dstPos, dstPos + num); columns that were at positions
+  /// [srcPos + num, dstPos + num) will be moved to [srcPos, srcPos + num);
+  /// and columns that were at positions [dstPos + num, nCols) will remain
+  /// where they were. For example, if m = |0 1 2 3 4 5| then
+  /// m.moveColumns(1, 2, 3) will result in m = |0 3 4 5 1 2|.
+  ///
+  /// The left shift operation (i.e. dstPos < srcPos) works in a similar way.
+  void moveColumns(unsigned srcPos, unsigned num, unsigned dstPos);
+
 protected:
   /// The current number of rows, columns, and reserved columns. The underlying
   /// data vector is viewed as an nRows x nReservedColumns matrix, of which the
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index fe461a1f95819f2..a5b898405fa70f6 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -230,6 +230,37 @@ void Matrix<T>::fillRow(unsigned row, const T &value) {
     at(row, col) = value;
 }
 
+template <typename T>
+void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, unsigned dstPos) {
+  if (num == 0)
+    return;
+
+  int offset = dstPos - srcPos;
+  if (offset == 0)
+    return;
+
+  assert(0 <= srcPos + offset && srcPos + num + offset <= getNumColumns() &&
+         "invalid move num");
+
+  unsigned insertCount = offset > 0 ? offset : -offset;
+  unsigned insertPos = offset > 0 ? srcPos : srcPos + num;
+  unsigned deletePos = offset > 0 ? srcPos + num : srcPos + offset;
+  // TODO: This can be done using std::rotate.
+  // Insert new zero columns in the positions where the adjacent columns are to
+  // be moved.
+  insertColumns(insertPos, insertCount);
+  // Update deletePos if insertion of new columns invalidates it.
+  if (insertPos < deletePos)
+    deletePos += insertCount;
+
+  // Swap the adjacent columns with inserted zero columns.
+  for (unsigned i = 0; i < insertCount; ++i)
+    swapColumns(insertPos + i, deletePos + i);
+
+  // Delete the now redundant zero columns.
+  removeColumns(deletePos, insertCount);
+}
+
 template <typename T>
 void Matrix<T>::addToRow(unsigned sourceRow, unsigned targetRow,
                          const T &scale) {
@@ -554,4 +585,4 @@ Fraction FracMatrix::determinant(FracMatrix *inverse) const {
     determinant *= m.at(i, i);
 
   return determinant;
-}
\ No newline at end of file
+}
diff --git a/mlir/unittests/Analysis/Presburger/MatrixTest.cpp b/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
index d05b05e004c5c5f..a2023eba519c737 100644
--- a/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
@@ -194,13 +194,21 @@ TEST(MatrixTest, resize) {
       EXPECT_EQ(mat(row, col), row >= 3 || col >= 3 ? 0 : int(10 * row + col));
 }
 
+template <typename T>
+static void checkMatEqual(const Matrix<T> m1, const Matrix<T> m2) {
+  EXPECT_EQ(m1.getNumRows(), m2.getNumRows());
+  EXPECT_EQ(m1.getNumColumns(), m2.getNumColumns());
+
+  for (unsigned row = 0, rows = m1.getNumRows(); row < rows; ++row)
+    for (unsigned col = 0, cols = m1.getNumColumns(); col < cols; ++col)
+      EXPECT_EQ(m1(row, col), m2(row, col));
+}
+
 static void checkHermiteNormalForm(const IntMatrix &mat,
                                    const IntMatrix &hermiteForm) {
   auto [h, u] = mat.computeHermiteNormalForm();
 
-  for (unsigned row = 0; row < mat.getNumRows(); row++)
-    for (unsigned col = 0; col < mat.getNumColumns(); col++)
-      EXPECT_EQ(h(row, col), hermiteForm(row, col));
+  checkMatEqual(h, hermiteForm);
 }
 
 TEST(MatrixTest, computeHermiteNormalForm) {
@@ -310,3 +318,40 @@ TEST(MatrixTest, intInverse) {
 
   EXPECT_EQ(det, 0);
 }
+
+TEST(MatrixTest, moveColumns) {
+  IntMatrix mat =
+      makeIntMatrix(3, 4, {{0, 1, 2, 3}, {4, 5, 6, 7}, {8, 9, 4, 2}});
+
+  {
+    IntMatrix movedMat =
+        makeIntMatrix(3, 4, {{0, 3, 1, 2}, {4, 7, 5, 6}, {8, 2, 9, 4}});
+
+    movedMat.moveColumns(2, 2, 1);
+    checkMatEqual(mat, movedMat);
+  }
+
+  {
+    IntMatrix movedMat =
+        makeIntMatrix(3, 4, {{0, 3, 1, 2}, {4, 7, 5, 6}, {8, 2, 9, 4}});
+
+    movedMat.moveColumns(1, 1, 3);
+    checkMatEqual(mat, movedMat);
+  }
+
+  {
+    IntMatrix movedMat =
+        makeIntMatrix(3, 4, {{1, 2, 0, 3}, {5, 6, 4, 7}, {9, 4, 8, 2}});
+
+    movedMat.moveColumns(0, 2, 1);
+    checkMatEqual(mat, movedMat);
+  }
+
+  {
+    IntMatrix movedMat =
+        makeIntMatrix(3, 4, {{1, 0, 2, 3}, {5, 4, 6, 7}, {9, 8, 4, 2}});
+
+    movedMat.moveColumns(0, 1, 1);
+    checkMatEqual(mat, movedMat);
+  }
+}

>From e0a8a6978718c0a926ce16c87ecec611f79747ef Mon Sep 17 00:00:00 2001
From: iambrj <joshibharathiramana at gmail.com>
Date: Fri, 6 Oct 2023 02:09:58 +0400
Subject: [PATCH 2/2] [MLIR][Presburger][WIP] Preserve identifiers in
 IntegerRelation::convertVarKind

---
 .../Analysis/Presburger/IntegerRelation.cpp   | 24 +++---
 .../Presburger/IntegerRelationTest.cpp        | 73 +++++++++++++++++++
 2 files changed, 82 insertions(+), 15 deletions(-)

diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index 6b26e09158208e4..82879701199119d 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -1410,23 +1410,17 @@ void IntegerRelation::convertVarKind(VarKind srcKind, unsigned varStart,
   if (varStart >= varLimit)
     return;
 
-  // Append new local variables corresponding to the dimensions to be converted.
+  unsigned srcOffset = getVarKindOffset(srcKind);
+  unsigned dstOffset = getVarKindOffset(dstKind);
   unsigned convertCount = varLimit - varStart;
-  unsigned newVarsBegin = insertVar(dstKind, pos, convertCount);
+  int forwardMoveOffset = dstOffset > srcOffset ? -convertCount : 0;
 
-  // Swap the new local variables with dimensions.
-  //
-  // Essentially, this moves the information corresponding to the specified ids
-  // of kind `srcKind` to the `convertCount` newly created ids of kind
-  // `dstKind`. In particular, this moves the columns in the constraint
-  // matrices, and zeros out the initially occupied columns (because the newly
-  // created ids we're swapping with were zero-initialized).
-  unsigned offset = getVarKindOffset(srcKind);
-  for (unsigned i = 0; i < convertCount; ++i)
-    swapVar(offset + varStart + i, newVarsBegin + i);
-
-  // Complete the move by deleting the initially occupied columns.
-  removeVarRange(srcKind, varStart, varLimit);
+  equalities.moveColumns(srcOffset + varStart, convertCount,
+                         dstOffset + pos + forwardMoveOffset);
+  inequalities.moveColumns(srcOffset + varStart, convertCount,
+                           dstOffset + pos + forwardMoveOffset);
+
+  space.convertVarKind(srcKind, varStart, varLimit - varStart, dstKind, pos);
 }
 
 void IntegerRelation::addBound(BoundType type, unsigned pos,
diff --git a/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp b/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
index 287f7c7c56549ff..8c45d47fb19d6de 100644
--- a/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
@@ -167,3 +167,76 @@ TEST(IntegerRelationTest, symbolicLexmax) {
   EXPECT_TRUE(lexmax3.unboundedDomain.isIntegerEmpty());
   EXPECT_TRUE(lexmax3.lexopt.isEqual(expectedLexmax3));
 }
+
+TEST(IntegerRelationTest, convertVarKind) {
+  PresburgerSpace space = PresburgerSpace::getSetSpace(3, 3, 0);
+  space.resetIds();
+
+  // Attach identifiers.
+  int identifiers[6] = {0, 1, 2, 3, 4, 5};
+  space.getId(VarKind::SetDim, 0) = Identifier(&identifiers[0]);
+  space.getId(VarKind::SetDim, 1) = Identifier(&identifiers[1]);
+  space.getId(VarKind::SetDim, 2) = Identifier(&identifiers[2]);
+  space.getId(VarKind::Symbol, 0) = Identifier(&identifiers[3]);
+  space.getId(VarKind::Symbol, 1) = Identifier(&identifiers[4]);
+  space.getId(VarKind::Symbol, 2) = Identifier(&identifiers[5]);
+
+  // Cannot call parseIntegerRelation to test convertVarKind as
+  // parseIntegerRelation uses convertVarKind.
+  IntegerRelation rel = parseIntegerPolyhedron(
+      // 0  1  2  3  4  5
+      "(x, y, a)[U, V, W] : (x - U == 0, y + a - W == 0, U - V >= 0,"
+      "y - a >= 0)");
+  rel.setSpace(space);
+
+  // Make a few kind conversions.
+  rel.convertVarKind(VarKind::Symbol, 1, 2, VarKind::Domain, 0);
+  rel.convertVarKind(VarKind::Range, 2, 3, VarKind::Domain, 0);
+  rel.convertVarKind(VarKind::Range, 0, 2, VarKind::Symbol, 1);
+  rel.convertVarKind(VarKind::Domain, 1, 2, VarKind::Range, 0);
+  rel.convertVarKind(VarKind::Domain, 0, 1, VarKind::Range, 1);
+
+  space = rel.getSpace();
+
+  // Expected rel.
+  IntegerRelation expectedRel = parseIntegerPolyhedron(
+      "(V, a)[U, x, y, W] : (x - U == 0, y + a - W == 0, U - V >= 0,"
+      "y - a >= 0)");
+  expectedRel.setSpace(space);
+
+  EXPECT_TRUE(rel.isEqual(expectedRel));
+
+  EXPECT_EQ(space.getId(VarKind::SetDim, 0), Identifier(&identifiers[4]));
+  EXPECT_EQ(space.getId(VarKind::SetDim, 1), Identifier(&identifiers[2]));
+  EXPECT_EQ(space.getId(VarKind::Symbol, 0), Identifier(&identifiers[3]));
+  EXPECT_EQ(space.getId(VarKind::Symbol, 1), Identifier(&identifiers[0]));
+  EXPECT_EQ(space.getId(VarKind::Symbol, 2), Identifier(&identifiers[1]));
+  EXPECT_EQ(space.getId(VarKind::Symbol, 3), Identifier(&identifiers[5]));
+}
+
+TEST(IntegerRelationTest, convertVarKindToLocal) {
+  // Convert all range variables to local variables
+  IntegerRelation rel =
+      parseRelationFromSet("(x, y, z) : (x >= 0, y >= 0, -z >= 0)", 1);
+  rel.convertToLocal(VarKind::Range, 0, rel.getNumRangeVars());
+  EXPECT_FALSE(rel.isEmptyByGCDTest());
+
+  // Convert all domain variables to local variables
+  IntegerRelation rel2 =
+      parseRelationFromSet("(x, y, z) : (x >= 0, y >= 0, -z >= 0)", 2);
+  rel2.convertToLocal(VarKind::Domain, 0, rel.getNumDomainVars());
+  EXPECT_FALSE(rel2.isEmptyByGCDTest());
+
+  // Convert a prefix of range variables to local variables
+  IntegerRelation rel3 = parseRelationFromSet(
+      "(x, y, u, v) : (x >= 0, y >= 0, -u >= 0, -v >= 0)", 2);
+  rel3.convertToLocal(VarKind::Range, rel.getNumDomainVars(), 1);
+  EXPECT_FALSE(rel3.isEmptyByGCDTest());
+
+  // Convert a suffix of domain variables to local variables
+  IntegerRelation rel4 = parseRelationFromSet(
+      "(x, y, u, v) : (x >= 0, y >= 0, -u >= 0, -v >= 0)", 2);
+  rel4.convertToLocal(VarKind::Domain, rel.getNumDomainVars() - 1,
+                      rel.getNumDomainVars());
+  EXPECT_FALSE(rel4.isEmptyByGCDTest());
+}



More information about the Mlir-commits mailing list