[MLIR][Presburger] Fix stale pivot in Smith normal form

[AdUhTkJm]

The pivot used to fix divisibility in Smith normal form is stale. This will not affect correctness, but can lower efficiency since the outer loop will be executed more times.

Thanks for @benquike of discovering this.

[llvmbot]

@llvm/pr-subscribers-mlir-presburger

Author: Yue Huang (AdUhTkJm)

Changes

The pivot used to fix divisibility in Smith normal form is stale. This will not affect correctness, but can lower efficiency since the outer loop will be executed more times.

Thanks for @benquike of discovering this.

---

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

1 Files Affected:

• (modified) mlir/lib/Analysis/Presburger/Matrix.cpp (+1-3)

diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 6ea04543146b7..94e9eb5792dab 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -653,8 +653,6 @@ IntMatrix::computeSmithNormalForm() const {
         u.negateRow(i);
       }
 
-      DynamicAPInt pivot = d(i, i);
-
       // Clear other entries in row i and column i with Euclid's algorithm.
       for (unsigned r = i + 1; r < numRows; ++r) {
         while (d(r, i) != 0) {
@@ -684,7 +682,7 @@ IntMatrix::computeSmithNormalForm() const {
         }
       }
 
-      if (auto row = findNonMultipleRow(d, i + 1, pivot)) {
+      if (auto row = findNonMultipleRow(d, i + 1, d(i, i))) {
         // Add the row (r) to row i. This brings d(r, c) into the i-th row,
         // creating a new value at d(i, c) that will be used to reduce the
         // pivot size.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Yue Huang (AdUhTkJm)

Changes

The pivot used to fix divisibility in Smith normal form is stale. This will not affect correctness, but can lower efficiency since the outer loop will be executed more times.

Thanks for @benquike of discovering this.

---

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

1 Files Affected:

• (modified) mlir/lib/Analysis/Presburger/Matrix.cpp (+1-3)

diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 6ea04543146b7..94e9eb5792dab 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -653,8 +653,6 @@ IntMatrix::computeSmithNormalForm() const {
         u.negateRow(i);
       }
 
-      DynamicAPInt pivot = d(i, i);
-
       // Clear other entries in row i and column i with Euclid's algorithm.
       for (unsigned r = i + 1; r < numRows; ++r) {
         while (d(r, i) != 0) {
@@ -684,7 +682,7 @@ IntMatrix::computeSmithNormalForm() const {
         }
       }
 
-      if (auto row = findNonMultipleRow(d, i + 1, pivot)) {
+      if (auto row = findNonMultipleRow(d, i + 1, d(i, i))) {
         // Add the row (r) to row i. This brings d(r, c) into the i-th row,
         // creating a new value at d(i, c) that will be used to reduce the
         // pivot size.

[superty]

Thanks for looking into it!