Skip to content

Conversation

@linuxlonelyeagle
Copy link
Member

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2025

@llvm/pr-subscribers-mlir

Author: lonely eagle (linuxlonelyeagle)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/168243.diff

1 Files Affected:

  • (modified) mlir/lib/Analysis/Presburger/Matrix.cpp (+20-24)
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index bb6056487512a..7933bec8d6da1 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -255,13 +255,7 @@ void Matrix<T>::fillRow(unsigned row, const T &value) {
 }
 
 // moveColumns is implemented by moving the columns adjacent to the source range
-// to their final position. When moving right (i.e. dstPos > srcPos), the range
-// of the adjacent columns is [srcPos + num, dstPos + num). When moving left
-// (i.e. dstPos < srcPos) the range of the adjacent columns is [dstPos, srcPos).
-// First, zeroed out columns are inserted in the final positions of the adjacent
-// columns. Then, the adjacent columns are moved to their final positions by
-// swapping them with the zeroed columns. Finally, the now zeroed adjacent
-// columns are deleted.
+// to their final position.
 template <typename T>
 void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, unsigned dstPos) {
   if (num == 0)
@@ -276,23 +270,25 @@ void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, unsigned dstPos) {
   assert(dstPos + num <= getNumColumns() &&
          "move destination range exceeds matrix columns");
 
-  unsigned insertCount = offset > 0 ? offset : -offset;
-  unsigned finalAdjStart = offset > 0 ? srcPos : srcPos + num;
-  unsigned curAdjStart = offset > 0 ? srcPos + num : dstPos;
-  // TODO: This can be done using std::rotate.
-  // Insert new zero columns in the positions where the adjacent columns are to
-  // be moved.
-  insertColumns(finalAdjStart, insertCount);
-  // Update curAdjStart if insertion of new columns invalidates it.
-  if (finalAdjStart < curAdjStart)
-    curAdjStart += insertCount;
-
-  // Swap the adjacent columns with inserted zero columns.
-  for (unsigned i = 0; i < insertCount; ++i)
-    swapColumns(finalAdjStart + i, curAdjStart + i);
-
-  // Delete the now redundant zero columns.
-  removeColumns(curAdjStart, insertCount);
+  unsigned numRows = getNumRows();
+  unsigned numCols = getNumReservedColumns();
+
+  if (offset > 0) {
+    // shift the matrix left, see
+    // https://en.cppreference.com/w/cpp/algorithm/rotate.html.
+    for (unsigned i = 0; i < numRows; ++i) {
+      auto begin = data.begin() + i * numCols + srcPos;
+      auto end = data.begin() + i * numCols + dstPos + num;
+      std::rotate(begin, begin + num, end);
+    }
+  } else {
+    // shift the matrix right.
+    for (unsigned i = 0; i < numRows; ++i) {
+      auto begin = data.begin() + i * numCols + srcPos + num;
+      auto end = data.begin() + i * numCols + dstPos;
+      std::rotate(end, begin - num, begin);
+    }
+  }
 }
 
 template <typename T>

@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2025

@llvm/pr-subscribers-mlir-presburger

Author: lonely eagle (linuxlonelyeagle)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/168243.diff

1 Files Affected:

  • (modified) mlir/lib/Analysis/Presburger/Matrix.cpp (+20-24)
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index bb6056487512a..7933bec8d6da1 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -255,13 +255,7 @@ void Matrix<T>::fillRow(unsigned row, const T &value) {
 }
 
 // moveColumns is implemented by moving the columns adjacent to the source range
-// to their final position. When moving right (i.e. dstPos > srcPos), the range
-// of the adjacent columns is [srcPos + num, dstPos + num). When moving left
-// (i.e. dstPos < srcPos) the range of the adjacent columns is [dstPos, srcPos).
-// First, zeroed out columns are inserted in the final positions of the adjacent
-// columns. Then, the adjacent columns are moved to their final positions by
-// swapping them with the zeroed columns. Finally, the now zeroed adjacent
-// columns are deleted.
+// to their final position.
 template <typename T>
 void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, unsigned dstPos) {
   if (num == 0)
@@ -276,23 +270,25 @@ void Matrix<T>::moveColumns(unsigned srcPos, unsigned num, unsigned dstPos) {
   assert(dstPos + num <= getNumColumns() &&
          "move destination range exceeds matrix columns");
 
-  unsigned insertCount = offset > 0 ? offset : -offset;
-  unsigned finalAdjStart = offset > 0 ? srcPos : srcPos + num;
-  unsigned curAdjStart = offset > 0 ? srcPos + num : dstPos;
-  // TODO: This can be done using std::rotate.
-  // Insert new zero columns in the positions where the adjacent columns are to
-  // be moved.
-  insertColumns(finalAdjStart, insertCount);
-  // Update curAdjStart if insertion of new columns invalidates it.
-  if (finalAdjStart < curAdjStart)
-    curAdjStart += insertCount;
-
-  // Swap the adjacent columns with inserted zero columns.
-  for (unsigned i = 0; i < insertCount; ++i)
-    swapColumns(finalAdjStart + i, curAdjStart + i);
-
-  // Delete the now redundant zero columns.
-  removeColumns(curAdjStart, insertCount);
+  unsigned numRows = getNumRows();
+  unsigned numCols = getNumReservedColumns();
+
+  if (offset > 0) {
+    // shift the matrix left, see
+    // https://en.cppreference.com/w/cpp/algorithm/rotate.html.
+    for (unsigned i = 0; i < numRows; ++i) {
+      auto begin = data.begin() + i * numCols + srcPos;
+      auto end = data.begin() + i * numCols + dstPos + num;
+      std::rotate(begin, begin + num, end);
+    }
+  } else {
+    // shift the matrix right.
+    for (unsigned i = 0; i < numRows; ++i) {
+      auto begin = data.begin() + i * numCols + srcPos + num;
+      auto end = data.begin() + i * numCols + dstPos;
+      std::rotate(end, begin - num, begin);
+    }
+  }
 }
 
 template <typename T>

@linuxlonelyeagle linuxlonelyeagle requested review from Groverkss, ftynse, joker-eph and krzysz00 and removed request for Groverkss November 15, 2025 21:33
@linuxlonelyeagle
Copy link
Member Author

ping for review @Superty @Groverkss , thank you.

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 your contribution. I left some comments!

@linuxlonelyeagle
Copy link
Member Author

Hello, I am researching something about presburger and I would like to fully understand the behavior from presburger to ValueBoundsconstrantSet.I am only sorting out the code a little bit now. Do you have any learning suggestions?

@linuxlonelyeagle
Copy link
Member Author

ping for review @Superty, thank you.

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.

LGTM if you fix the tiny comments I left now. Can you elaborate on what you mean by your other question?

@Superty Superty enabled auto-merge (squash) December 2, 2025 01:57
@Superty Superty merged commit 8dc6abb into llvm:main Dec 2, 2025
10 checks passed
@linuxlonelyeagle
Copy link
Member Author

LGTM if you fix the tiny comments I left now. Can you elaborate on what you mean by your other question?

After thinking about it, I actually want to gain a deeper understanding of the implementation of range inference code in MLIR.
I'm thinking about which part of the code I should study (Presburger has too much code).

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.

3 participants