This patch adds hermite normal form computation to Matrix. Part of this algorithm
lived in LinearTransform, being used for compuing column echelon form. This
patch moves the implementation to Matrix::hermiteNormalForm and generalises it
to compute the hermite normal form.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I would assume that the computation works fast and should not bottleneck anything assuming that the number of columns is not too huge (which is generally true for polyhedra).
I still tried to run some experiments:
For a 10x10 matrix with almost 4 non-zero elements per row (generally polyhedra 2-3 non-zero elements per row in inequality/equality matrix):
{
Matrix mat = makeMatrix(10, 10,
{
{1, 0, 0, 0, 1, 0, 0, 0, 0, 0},
{0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
{0, 0, 0, 3, 0, 0, 4, 0, 0, -1},
{0, 0, 1, 0, 0, -1, 0, 0, 0, 0},
{0, 0, 0, -1, 0, 0, 0, 0, 0, -1},
{0, 0, 0, 1, 0, 0, 0, 0, 2, 10},
{0, 0, 0, 0, 3, 0, 0, 2, 0, 0},
{0, 1, 0, 0, 0, 0, 5, 0, 0, 0},
{0, 0, 0, 1, 0, 1, 0, 0, 1, 1},
{0, 0, 1, 0, 0, 1, 0, 1, 0, 0},
});
auto [h, u] = mat.hermiteNormalForm();
}Running this computation 100 times takes 2ms in a debug build. I would expect it to be faster in a release build.
I also tried the computation for dense 10x10 matrices, but while it runs fast, it is incorrect due to overflows (which should be fixed by https://reviews.llvm.org/D129510)
Are there any specific numbers that you are looking for? Or are these fine?
| mlir/include/mlir/Analysis/Presburger/Matrix.h | ||
|---|---|---|
| 166 | ||
| 168 | use std::pair. | |
| 168 | ||
| mlir/lib/Analysis/Presburger/LinearTransform.cpp | ||
| 20–21 | ||
| 24 | ||
| mlir/lib/Analysis/Presburger/Matrix.cpp | ||
| 348 | ||
| 350–364 | It would be better to just support negative targetCol in modEntryColumnOperation and use that | |
| mlir/unittests/Analysis/Presburger/MatrixTest.cpp | ||
| 201 | ||
| 229–239 | Please write a function to do this | |
| mlir/lib/Analysis/Presburger/Matrix.cpp | ||
|---|---|---|
| 348 | This is not required because the scale is always assigned a value before use. | |
| mlir/lib/Analysis/Presburger/Matrix.cpp | ||
|---|---|---|
| 264–271 | ratio should just be floorDiv(m(row, targetCol), m(row, sourceCol)) right? (with scale =-ratio below) | |
| mlir/lib/Analysis/Presburger/Matrix.cpp | ||
|---|---|---|
| 263 | Maybe add some documentation explaining this. | |