[Mlir-commits] [mlir] [MLIR][Presburger] Implement Matrix::moveColumns (PR #68362)

Bharathi Ramana Joshi llvmlistbot at llvm.org
Tue Oct 10 11:40:22 PDT 2023


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

>From 28eb165691e123728a485d06a37beffd82166825 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/3] [MLIR][Presburger] Implement Matrix::moveColumns

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

diff --git a/mlir/include/mlir/Analysis/Presburger/Matrix.h b/mlir/include/mlir/Analysis/Presburger/Matrix.h
index bed3a5f75e396c5..0325d5ebd017bbe 100644
--- a/mlir/include/mlir/Analysis/Presburger/Matrix.h
+++ b/mlir/include/mlir/Analysis/Presburger/Matrix.h
@@ -189,6 +189,23 @@ static_assert(std::is_same_v<T,MPInt> || std::is_same_v<T,Fraction>, "T must be
   /// invariants satisfied.
   bool hasConsistentState() const;
 
+  /// Shift the columns in the range [srcPos, srcPos + num] by the specified
+  /// offset, i.e. to [srcPos + offset, srcPos + num + offset], while moving
+  /// the columns adjacent to the range to the left/right of the shifted
+  /// columns.
+  ///
+  /// For a positive offset (i.e. shifting the columns right), columns that
+  /// were at positions [0, srcPos) will stay where they are; columns that were
+  /// at positions [srcPos + num, srcPos + num + offset) will be moved to
+  /// [srcPos, srcPos + offset); and columns that were at positions
+  /// (src + num + offset, 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|.
+  ///
+  /// Similarly, a negative offset results in a left shift of the columns in
+  /// the range [srcPos, srcPos + num].
+  void moveColumns(unsigned srcPos, unsigned num, int offset);
+
 private:
   /// 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 f0bcb09fb28f7b1..4c8d63055fc5eb3 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -192,6 +192,30 @@ template <typename T> void Matrix<T>::fillRow(unsigned row, const T &value) {
     at(row, col) = value;
 }
 
+template <typename T> void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, int offset) {
+  assert(num != 0 && "num must be non-zero");
+  assert(offset != 0 && "offset must be non-zero");
+  assert(0 <= srcPos + offset && srcPos + num + offset <= getNumColumns() &&
+      "invalid move offset");
+
+  unsigned insertCount = offset > 0 ? offset : -offset,
+           insertPos = offset > 0 ? srcPos : srcPos + num,
+           deletePos = offset > 0 ? srcPos + num : srcPos + offset;
+  // 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) {
   addToRow(targetRow, getRow(sourceRow), scale);
@@ -390,4 +414,4 @@ MPInt IntMatrix::normalizeRow(unsigned row, unsigned cols) {
 
 MPInt IntMatrix::normalizeRow(unsigned row) {
   return normalizeRow(row, getNumColumns());
-}
\ No newline at end of file
+}
diff --git a/mlir/unittests/Analysis/Presburger/MatrixTest.cpp b/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
index 6b23cedabf624ec..57e0fe627bec5af 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; row < m1.getNumRows(); row++)
+    for (unsigned col = 0; col < m2.getNumColumns(); 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) {
@@ -248,3 +256,48 @@ TEST(MatrixTest, computeHermiteNormalForm) {
     checkHermiteNormalForm(mat, hermiteForm);
   }
 }
+
+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, 2);
+    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 6f153fc67e8d445f119ccf3a9f771cbd38076973 Mon Sep 17 00:00:00 2001
From: iambrj <joshibharathiramana at gmail.com>
Date: Tue, 10 Oct 2023 21:05:07 +0400
Subject: [PATCH 2/3] [MLIR][Presburger] Rewrite Matrix::moveColumns to use
 dstPos

---
 .../include/mlir/Analysis/Presburger/Matrix.h | 25 ++++++++++---------
 mlir/lib/Analysis/Presburger/Matrix.cpp       | 13 +++++++---
 .../Analysis/Presburger/MatrixTest.cpp        | 10 +++++---
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/mlir/include/mlir/Analysis/Presburger/Matrix.h b/mlir/include/mlir/Analysis/Presburger/Matrix.h
index 0325d5ebd017bbe..9b69ad1eefce5b6 100644
--- a/mlir/include/mlir/Analysis/Presburger/Matrix.h
+++ b/mlir/include/mlir/Analysis/Presburger/Matrix.h
@@ -189,22 +189,23 @@ static_assert(std::is_same_v<T,MPInt> || std::is_same_v<T,Fraction>, "T must be
   /// invariants satisfied.
   bool hasConsistentState() const;
 
-  /// Shift the columns in the range [srcPos, srcPos + num] by the specified
-  /// offset, i.e. to [srcPos + offset, srcPos + num + offset], while moving
-  /// the columns adjacent to the range to the left/right of the shifted
+  /// 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.
   ///
-  /// For a positive offset (i.e. shifting the columns right), columns that
-  /// were at positions [0, srcPos) will stay where they are; columns that were
-  /// at positions [srcPos + num, srcPos + num + offset) will be moved to
-  /// [srcPos, srcPos + offset); and columns that were at positions
-  /// (src + num + offset, 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|.
+  /// 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|.
   ///
   /// Similarly, a negative offset results in a left shift of the columns in
-  /// the range [srcPos, srcPos + num].
-  void moveColumns(unsigned srcPos, unsigned num, int offset);
+  /// the range [srcPos, srcPos + num).
+  void moveColumns(unsigned srcPos, unsigned num, unsigned dstPos);
 
 private:
   /// The current number of rows, columns, and reserved columns. The underlying
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 4c8d63055fc5eb3..d3184c769cc6e8b 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -192,11 +192,16 @@ template <typename T> void Matrix<T>::fillRow(unsigned row, const T &value) {
     at(row, col) = value;
 }
 
-template <typename T> void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, int offset) {
-  assert(num != 0 && "num must be non-zero");
-  assert(offset != 0 && "offset must be non-zero");
+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 offset");
+      "invalid move num");
 
   unsigned insertCount = offset > 0 ? offset : -offset,
            insertPos = offset > 0 ? srcPos : srcPos + num,
diff --git a/mlir/unittests/Analysis/Presburger/MatrixTest.cpp b/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
index 57e0fe627bec5af..4045750fa99721d 100644
--- a/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/MatrixTest.cpp
@@ -269,7 +269,7 @@ TEST(MatrixTest, moveColumns) {
        {4, 7, 5, 6},
        {8, 2, 9, 4}});
 
-    movedMat.moveColumns(2, 2, -1);
+    movedMat.moveColumns(2, 2, 1);
     checkMatEqual(mat, movedMat);
   }
 
@@ -279,7 +279,7 @@ TEST(MatrixTest, moveColumns) {
        {4, 7, 5, 6},
        {8, 2, 9, 4}});
 
-    movedMat.moveColumns(1, 1, 2);
+    movedMat.moveColumns(1, 1, 3);
     checkMatEqual(mat, movedMat);
   }
 
@@ -294,8 +294,10 @@ TEST(MatrixTest, moveColumns) {
   }
 
   {
-    IntMatrix movedMat =
-        makeIntMatrix(3, 4, {{1, 0, 2, 3}, {5, 4, 6, 7}, {9, 8, 4, 2}});
+    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 d3fe8609bee8a039990b69e5cd2cab4bdbfca01f Mon Sep 17 00:00:00 2001
From: iambrj <joshibharathiramana at gmail.com>
Date: Tue, 10 Oct 2023 22:40:03 +0400
Subject: [PATCH 3/3] [MLIR][Presburger] Add TODO

---
 mlir/lib/Analysis/Presburger/Matrix.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index d3184c769cc6e8b..03a919c491c87ec 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -206,6 +206,7 @@ template <typename T> void Matrix<T>::moveColumns(unsigned srcPos, unsigned num,
   unsigned insertCount = offset > 0 ? offset : -offset,
            insertPos = offset > 0 ? srcPos : srcPos + num,
            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);



More information about the Mlir-commits mailing list