[Mlir-commits] [mlir] [WIP][MLIR][Presburger] Fix a bunch of minor style choices etc. (PR #82782)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Feb 23 08:12:49 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-presburger
@llvm/pr-subscribers-mlir
Author: None (Abhinav271828)
<details>
<summary>Changes</summary>
* Use range-style for-loops
* Use `int` instead of `unsigned`
* Define and use `+=` for QuasiPolynomials
* Use "numSymbols" instead of "numParams"
---
Patch is 35.67 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82782.diff
9 Files Affected:
- (modified) mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h (+14-12)
- (modified) mlir/include/mlir/Analysis/Presburger/QuasiPolynomial.h (+3)
- (modified) mlir/lib/Analysis/Presburger/Barvinok.cpp (+59-33)
- (modified) mlir/lib/Analysis/Presburger/Matrix.cpp (+22-21)
- (modified) mlir/lib/Analysis/Presburger/QuasiPolynomial.cpp (+33-4)
- (modified) mlir/lib/Analysis/Presburger/Utils.cpp (+4-3)
- (modified) mlir/unittests/Analysis/Presburger/BarvinokTest.cpp (+6-6)
- (modified) mlir/unittests/Analysis/Presburger/MatrixTest.cpp (+10-10)
- (modified) mlir/unittests/Analysis/Presburger/Utils.h (+19-19)
``````````diff
diff --git a/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h b/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h
index db5b6b6a959186..d19945d2c5266d 100644
--- a/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h
+++ b/mlir/include/mlir/Analysis/Presburger/GeneratingFunction.h
@@ -16,6 +16,7 @@
#include "mlir/Analysis/Presburger/Fraction.h"
#include "mlir/Analysis/Presburger/Matrix.h"
+#include "llvm/ADT/Sequence.h"
namespace mlir {
namespace presburger {
@@ -50,19 +51,20 @@ using Point = SmallVector<Fraction>;
// g_{ij} \in Q^n are vectors.
class GeneratingFunction {
public:
- GeneratingFunction(unsigned numParam, SmallVector<int> signs,
+ GeneratingFunction(unsigned numSymbols, SmallVector<int> signs,
std::vector<ParamPoint> nums,
std::vector<std::vector<Point>> dens)
- : numParam(numParam), signs(signs), numerators(nums), denominators(dens) {
+ : numSymbols(numSymbols), signs(signs), numerators(nums),
+ denominators(dens) {
#ifndef NDEBUG
for (const ParamPoint &term : numerators)
- assert(term.getNumRows() == numParam + 1 &&
+ assert(term.getNumRows() == numSymbols + 1 &&
"dimensionality of numerator exponents does not match number of "
"parameters!");
#endif // NDEBUG
}
- unsigned getNumParams() const { return numParam; }
+ unsigned getNumSymbols() const { return numSymbols; }
SmallVector<int> getSigns() const { return signs; }
@@ -73,7 +75,7 @@ class GeneratingFunction {
}
GeneratingFunction operator+(const GeneratingFunction &gf) const {
- assert(numParam == gf.getNumParams() &&
+ assert(numSymbols == gf.getNumSymbols() &&
"two generating functions with different numbers of parameters "
"cannot be added!");
SmallVector<int> sumSigns = signs;
@@ -86,12 +88,12 @@ class GeneratingFunction {
std::vector<std::vector<Point>> sumDenominators = denominators;
sumDenominators.insert(sumDenominators.end(), gf.denominators.begin(),
gf.denominators.end());
- return GeneratingFunction(numParam, sumSigns, sumNumerators,
+ return GeneratingFunction(numSymbols, sumSigns, sumNumerators,
sumDenominators);
}
llvm::raw_ostream &print(llvm::raw_ostream &os) const {
- for (unsigned i = 0, e = signs.size(); i < e; i++) {
+ for (int i : llvm::seq<int>(0, signs.size())) {
if (i == 0) {
if (signs[i] == -1)
os << "- ";
@@ -104,20 +106,20 @@ class GeneratingFunction {
os << "x^[";
unsigned r = numerators[i].getNumRows();
- for (unsigned j = 0; j < r - 1; j++) {
+ for (unsigned j = 0; j < r - 1; ++j) {
os << "[";
- for (unsigned k = 0, c = numerators[i].getNumColumns(); k < c - 1; k++)
+ for (int k : llvm::seq<int>(0, numerators[i].getNumColumns() - 1))
os << numerators[i].at(j, k) << ",";
os << numerators[i].getRow(j).back() << "],";
}
os << "[";
- for (unsigned k = 0, c = numerators[i].getNumColumns(); k < c - 1; k++)
+ for (int k : llvm::seq<int>(0, numerators[i].getNumColumns() - 1))
os << numerators[i].at(r - 1, k) << ",";
os << numerators[i].getRow(r - 1).back() << "]]/";
for (const Point &den : denominators[i]) {
os << "(x^[";
- for (unsigned j = 0, e = den.size(); j < e - 1; j++)
+ for (int j : llvm::seq<int>(0, den.size() - 1))
os << den[j] << ",";
os << den.back() << "])";
}
@@ -126,7 +128,7 @@ class GeneratingFunction {
}
private:
- unsigned numParam;
+ unsigned numSymbols;
SmallVector<int> signs;
std::vector<ParamPoint> numerators;
std::vector<std::vector<Point>> denominators;
diff --git a/mlir/include/mlir/Analysis/Presburger/QuasiPolynomial.h b/mlir/include/mlir/Analysis/Presburger/QuasiPolynomial.h
index aeac19e827b44f..a3af219192fbe1 100644
--- a/mlir/include/mlir/Analysis/Presburger/QuasiPolynomial.h
+++ b/mlir/include/mlir/Analysis/Presburger/QuasiPolynomial.h
@@ -58,6 +58,7 @@ class QuasiPolynomial : public PresburgerSpace {
QuasiPolynomial operator-(const QuasiPolynomial &x) const;
QuasiPolynomial operator*(const QuasiPolynomial &x) const;
QuasiPolynomial operator/(const Fraction x) const;
+ void operator+=(const QuasiPolynomial &x);
// Removes terms which evaluate to zero from the expression
// and folds affine functions which are constant into the
@@ -69,6 +70,8 @@ class QuasiPolynomial : public PresburgerSpace {
Fraction getConstantTerm();
+ Fraction evaluateAt(const SmallVector<Fraction> ¶meters);
+
private:
SmallVector<Fraction> coefficients;
std::vector<std::vector<SmallVector<Fraction>>> affine;
diff --git a/mlir/lib/Analysis/Presburger/Barvinok.cpp b/mlir/lib/Analysis/Presburger/Barvinok.cpp
index b6d1f99df8ba55..e5a81230fede7f 100644
--- a/mlir/lib/Analysis/Presburger/Barvinok.cpp
+++ b/mlir/lib/Analysis/Presburger/Barvinok.cpp
@@ -28,12 +28,10 @@ ConeV mlir::presburger::detail::getDual(ConeH cone) {
// is represented as a row [a1, ..., an, b]
// and that b = 0.
- for (auto i : llvm::seq<int>(0, numIneq)) {
+ for (int i : llvm::seq<int>(0, numIneq)) {
assert(cone.atIneq(i, numVar) == 0 &&
"H-representation of cone is not centred at the origin!");
- for (unsigned j = 0; j < numVar; ++j) {
- dual.at(i, j) = cone.atIneq(i, j);
- }
+ dual.setRow(i, cone.getInequality(i).take_front(numVar));
}
// Now dual is of the form [ [a1, ..., an] , ... ]
@@ -130,8 +128,8 @@ mlir::presburger::detail::computeUnimodularConeGeneratingFunction(
unsigned numRows = vertex.getNumRows();
ParamPoint numerator(numColumns, numRows);
SmallVector<Fraction> ithCol(numRows);
- for (auto i : llvm::seq<int>(0, numColumns)) {
- for (auto j : llvm::seq<int>(0, numRows))
+ for (int i : llvm::seq<int>(0, numColumns)) {
+ for (int j : llvm::seq<int>(0, numRows))
ithCol[j] = vertex(j, i);
numerator.setRow(i, transp.preMultiplyWithRow(ithCol));
numerator.negateRow(i);
@@ -176,12 +174,12 @@ mlir::presburger::detail::solveParametricEquations(FracMatrix equations) {
// Perform row operations to make each column all zeros except for the
// diagonal element, which is made to be one.
- for (unsigned i = 0; i < d; ++i) {
+ for (int i : llvm::seq<int>(0, d)) {
// First ensure that the diagonal element is nonzero, by swapping
// it with a row that is non-zero at column i.
if (equations(i, i) != 0)
continue;
- for (unsigned j = i + 1; j < d; ++j) {
+ for (int j : llvm::seq<int>(i + 1, d)) {
if (equations(j, i) == 0)
continue;
equations.swapRows(j, i);
@@ -191,7 +189,7 @@ mlir::presburger::detail::solveParametricEquations(FracMatrix equations) {
Fraction diagElement = equations(i, i);
// Apply row operations to make all elements except the diagonal to zero.
- for (unsigned j = 0; j < d; ++j) {
+ for (int j : llvm::seq<int>(0, d)) {
if (i == j)
continue;
if (equations(j, i) == 0)
@@ -205,7 +203,7 @@ mlir::presburger::detail::solveParametricEquations(FracMatrix equations) {
}
// Rescale diagonal elements to 1.
- for (unsigned i = 0; i < d; ++i)
+ for (int i : llvm::seq<int>(0, d))
equations.scaleRow(i, 1 / equations(i, i));
// Now we have reduced the equations to the form
@@ -237,6 +235,34 @@ mlir::presburger::detail::solveParametricEquations(FracMatrix equations) {
/// generating functions the region that (the sum of) precisely this subset is
/// in, is the intersection of the regions that these are active in,
/// intersected with the complements of the remaining regions.
+///
+/// If the parameter values lie in the intersection of two chambers with
+/// differing vertex sets, the answer is given by considering either one.
+/// Assume that the chambers differ in that one has k+1 vertices and the other
+/// only k – we can then show that the same vertex is counted twice in the
+/// former, and so the sets are in fact the same. This is proved below.
+/// Consider a parametric polyhedron D in n variables and m parameters. We want
+/// to show that if two vertices' chambers intersect, then the vertices collapse
+/// in this region.
+///
+/// Let D' be the polyhedron in combined data-and-parameter space (R^{n+m}).
+/// Then we have from [1] that:
+///
+/// * a vertex is a 0-dimensional face of D, which is equivalent to an m-face
+/// F_i^m of D'.
+/// * a vertex v_i(p) is obtained from F_i^m(D') by projecting down to n-space
+/// (see Fig. 1).
+/// * the region a vertex exists in is given by projecting F_i^m(D') down to
+/// m-space.
+///
+/// Thus, if two chambers intersect, this means that the corresponding faces
+/// (say F_i^m(D') and F_j^m(D')) also have some intersection. Since the
+/// vertices v_i and v_j correspond to the projections of F_i^m(D') and
+/// F_j^m(D'), they must collapse in the intersection.
+///
+/// [1]: Parameterized Polyhedra and Their Vertices (Loechner & Wilde, 1997)
+/// https://link.springer.com/article/10.1023/A:1025117523902
+
std::vector<std::pair<PresburgerSet, GeneratingFunction>>
mlir::presburger::detail::computeChamberDecomposition(
unsigned numSymbols, ArrayRef<std::pair<PresburgerSet, GeneratingFunction>>
@@ -332,7 +358,7 @@ mlir::presburger::detail::computePolytopeGeneratingFunction(
//
// We start with the permutation that takes the last numVars inequalities.
SmallVector<int> indicator(numIneqs);
- for (unsigned i = numIneqs - numVars; i < numIneqs; ++i)
+ for (int i : llvm::seq<int>(numIneqs - numVars, numIneqs))
indicator[i] = 1;
do {
@@ -393,7 +419,7 @@ mlir::presburger::detail::computePolytopeGeneratingFunction(
// Thus we premultiply [X | y] with each row of A2
// and add each row of [B2 | c2].
FracMatrix activeRegion(numIneqs - numVars, numSymbols + 1);
- for (unsigned i = 0; i < numIneqs - numVars; i++) {
+ for (int i : llvm::seq<int>(0, numIneqs - numVars)) {
activeRegion.setRow(i, vertex->preMultiplyWithRow(a2.getRow(i)));
activeRegion.addToRow(i, b2c2.getRow(i), 1);
}
@@ -412,9 +438,9 @@ mlir::presburger::detail::computePolytopeGeneratingFunction(
// We translate the cones to be pointed at the origin by making the
// constant terms zero.
ConeH tangentCone = defineHRep(numVars);
- for (unsigned j = 0, e = subset.getNumRows(); j < e; ++j) {
+ for (int j : llvm::seq<int>(0, subset.getNumRows())) {
SmallVector<MPInt> ineq(numVars + 1);
- for (unsigned k = 0; k < numVars; ++k)
+ for (int k : llvm::seq<int>(0, numVars))
ineq[k] = subset(j, k);
tangentCone.addInequality(ineq);
}
@@ -482,7 +508,7 @@ Point mlir::presburger::detail::getNonOrthogonalVector(
Fraction maxDisallowedValue = -Fraction(1, 0),
disallowedValue = Fraction(0, 1);
- for (unsigned d = 1; d < dim; ++d) {
+ for (int d : llvm::seq<int>(1, dim)) {
// Compute the disallowed values - <x_i[:d-1], vs> / x_i[d] for each i.
maxDisallowedValue = -Fraction(1, 0);
for (const Point &vector : vectors) {
@@ -530,7 +556,7 @@ QuasiPolynomial mlir::presburger::detail::getCoefficientInRationalFunction(
coefficients.reserve(power + 1);
coefficients.push_back(num[0] / den[0]);
- for (unsigned i = 1; i <= power; ++i) {
+ for (unsigned i : llvm::seq<int>(1, power + 1)) {
// If the power is not there in the numerator, the coefficient is zero.
coefficients.push_back(i < num.size() ? num[i]
: QuasiPolynomial(numParam, 0));
@@ -538,7 +564,7 @@ QuasiPolynomial mlir::presburger::detail::getCoefficientInRationalFunction(
// After den.size(), the coefficients are zero, so we stop
// subtracting at that point (if it is less than i).
unsigned limit = std::min<unsigned long>(i, den.size() - 1);
- for (unsigned j = 1; j <= limit; ++j)
+ for (int j : llvm::seq<int>(1, limit + 1))
coefficients[i] = coefficients[i] -
coefficients[i - j] * QuasiPolynomial(numParam, den[j]);
@@ -582,7 +608,7 @@ substituteMuInTerm(unsigned numParams, ParamPoint v, std::vector<Point> ds,
ParamPoint vTranspose = v.transpose();
std::vector<std::vector<SmallVector<Fraction>>> affine;
affine.reserve(numDims);
- for (unsigned j = 0; j < numDims; ++j)
+ for (int j : llvm::seq<int>(0, numDims))
affine.push_back({SmallVector<Fraction>(vTranspose.getRow(j))});
QuasiPolynomial num(numParams, coefficients, affine);
@@ -613,10 +639,10 @@ void normalizeDenominatorExponents(int &sign, QuasiPolynomial &num,
// denominator, and convert them to their absolute values.
unsigned numNegExps = 0;
Fraction sumNegExps(0, 1);
- for (unsigned j = 0, e = dens.size(); j < e; ++j) {
- if (dens[j] < 0) {
+ for (const Fraction &den : dens) {
+ if (den < 0) {
numNegExps += 1;
- sumNegExps += dens[j];
+ sumNegExps += den;
}
}
@@ -641,7 +667,7 @@ std::vector<QuasiPolynomial> getBinomialCoefficients(QuasiPolynomial n,
std::vector<QuasiPolynomial> coefficients;
coefficients.reserve(r + 1);
coefficients.push_back(QuasiPolynomial(numParams, 1));
- for (unsigned j = 1; j <= r; ++j)
+ for (int j : llvm::seq<int>(1, r + 1))
// We use the recursive formula for binomial coefficients here and below.
coefficients.push_back(
(coefficients[j - 1] * (n - QuasiPolynomial(numParams, j - 1)) /
@@ -698,10 +724,10 @@ mlir::presburger::detail::computeNumTerms(const GeneratingFunction &gf) {
allDenominators.insert(allDenominators.end(), den.begin(), den.end());
Point mu = getNonOrthogonalVector(allDenominators);
- unsigned numParams = gf.getNumParams();
+ unsigned numParams = gf.getNumSymbols();
const std::vector<std::vector<Point>> &ds = gf.getDenominators();
QuasiPolynomial totalTerm(numParams, 0);
- for (unsigned i = 0, e = ds.size(); i < e; ++i) {
+ for (int i : llvm::seq<int>(0, ds.size())) {
int sign = gf.getSigns()[i];
// Compute the new exponents of (s+1) for the numerator and the
@@ -722,7 +748,7 @@ mlir::presburger::detail::computeNumTerms(const GeneratingFunction &gf) {
// Then, using the formula for geometric series, we replace each (1 -
// (s+1)^(dens[j])) with
// (-s)(\sum_{0 ≤ k < dens[j]} (s+1)^k).
- for (unsigned j = 0, e = dens.size(); j < e; ++j)
+ for (int j : llvm::seq<int>(0, dens.size()))
dens[j] = abs(dens[j]) - 1;
// Note that at this point, the semantics of `dens[j]` changes to mean
// a term (\sum_{0 ≤ k ≤ dens[j]} (s+1)^k). The denominator is, as before,
@@ -774,14 +800,14 @@ mlir::presburger::detail::computeNumTerms(const GeneratingFunction &gf) {
// of all the terms.
std::vector<Fraction> denominatorCoefficients;
denominatorCoefficients = eachTermDenCoefficients[0];
- for (unsigned j = 1, e = eachTermDenCoefficients.size(); j < e; ++j)
- denominatorCoefficients = multiplyPolynomials(denominatorCoefficients,
- eachTermDenCoefficients[j]);
-
- totalTerm =
- totalTerm + getCoefficientInRationalFunction(r, numeratorCoefficients,
- denominatorCoefficients) *
- QuasiPolynomial(numParams, sign);
+ for (const std::vector<Fraction> &eachTermDenCoefficient :
+ eachTermDenCoefficients)
+ denominatorCoefficients =
+ multiplyPolynomials(denominatorCoefficients, eachTermDenCoefficient);
+
+ totalTerm += getCoefficientInRationalFunction(r, numeratorCoefficients,
+ denominatorCoefficients) *
+ QuasiPolynomial(numParams, sign);
}
return totalTerm.simplify();
diff --git a/mlir/lib/Analysis/Presburger/Matrix.cpp b/mlir/lib/Analysis/Presburger/Matrix.cpp
index 4cb6e6b16bc878..76d4f61d1c0c2a 100644
--- a/mlir/lib/Analysis/Presburger/Matrix.cpp
+++ b/mlir/lib/Analysis/Presburger/Matrix.cpp
@@ -11,6 +11,7 @@
#include "mlir/Analysis/Presburger/MPInt.h"
#include "mlir/Analysis/Presburger/Utils.h"
#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/Sequence.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
@@ -38,7 +39,7 @@ bool Matrix<T>::operator==(const Matrix<T> &m) const {
if (nColumns != m.getNumColumns())
return false;
- for (unsigned i = 0; i < nRows; i++)
+ for (int i : llvm::seq<int>(0, nRows))
if (getRow(i) != m.getRow(i))
return false;
@@ -392,8 +393,8 @@ Matrix<T> Matrix<T>::getSubMatrix(unsigned fromRow, unsigned toRow,
"end of column range must be after beginning!");
assert(toColumn < nColumns && "end of column range out of bounds!");
Matrix<T> subMatrix(toRow - fromRow + 1, toColumn - fromColumn + 1);
- for (unsigned i = fromRow; i <= toRow; ++i)
- for (unsigned j = fromColumn; j <= toColumn; ++j)
+ for (int i : llvm::seq<int>(fromRow, toRow + 1))
+ for (int j : llvm::seq<int>(fromColumn, toColumn + 1))
subMatrix(i - fromRow, j - fromColumn) = at(i, j);
return subMatrix;
}
@@ -413,7 +414,7 @@ template <typename T>
std::pair<Matrix<T>, Matrix<T>>
Matrix<T>::splitByBitset(ArrayRef<int> indicator) {
Matrix<T> rowsForOne(0, nColumns), rowsForZero(0, nColumns);
- for (unsigned i = 0; i < nRows; i++) {
+ for (int i : llvm::seq<int>(0, nRows)) {
if (indicator[i] == 1)
rowsForOne.appendExtraRow(getRow(i));
else
@@ -566,8 +567,8 @@ MPInt IntMatrix::determinant(IntMatrix *inverse) const {
return detM;
*inverse = IntMatrix(nRows, nColumns);
- for (unsigned i = 0; i < nRows; i++)
- for (unsigned j = 0; j < nColumns; j++)
+ for (int i : llvm::seq<int>(0, nRows))
+ for (int j : llvm::seq<int>(0, nColumns))
inverse->at(i, j) = (fracInverse.at(i, j) * detM).getAsInteger();
return detM;
@@ -579,8 +580,8 @@ FracMatrix FracMatrix::identity(unsigned dimension) {
FracMatrix::FracMatrix(IntMatrix m)
: FracMatrix(m.getNumRows(), m.getNumColumns()) {
- for (unsigned i = 0, r = m.getNumRows(); i < r; i++)
- for (unsigned j = 0, c = m.getNumColumns(); j < c; j++)
+ for (int i : llvm::seq<int>(0, m.getNumRows()))
+ for (int j : llvm::seq<int>(0, m.getNumColumns()))
this->at(i, j) = m.at(i, j);
}
@@ -602,12 +603,12 @@ Fraction FracMatrix::determinant(FracMatrix *inverse) const {
// which is initially identity.
// Either way, the product of the diagonal elements
// is then the determinant.
- for (unsigned i = 0; i < nRows; i++) {
+ for (int i : llvm::seq<int>(0, nRows)) {
if (m(i, i) == 0)
// First ensure that the diagonal
// element is nonzero, by swapping
// it with a nonzero row.
- for (unsigned j = i + 1; j < nRows; j++) {
+ for (int j : llvm::seq<int>(i + 1, nRows)) {
if (m(j, i) != 0) {
m.swapRows(j, i);
if (inverse)
@@ -623,7 +624,7 @@ Fraction FracMatrix::determinant(FracMatrix *inverse) const {
// Set all elements above the
// diagonal to zero.
if (inverse) {
- for (unsigned j = 0; j < i; j++) {
+ for (int j : llvm::seq<int>(0, i)) {
if (m.at(j, i) == 0)
continue;
a = m.at(j, i);
@@ -637,7 +638,7 @@ Fraction FracMatrix::determinant(FracMatrix *inverse) const {
// Set all elements below the
// diagonal to zero.
- for (unsigned j = i + 1; j < nRows; j++) {
+ for (int j : llvm::seq<int>(i + 1, nRows)) {
if (m.at(j, i) == 0)
continue;
a = m.at(j, i);
@@ -655,15 +656,15 @@ Fraction FracMatrix::determinant(FracMatrix *inverse) const {
// normalize them and apply the same scale to the inverse matrix.
// For efficiency we skip scaling m and just scale tempInv appropriately.
if (inverse) {
- for (unsigned i = 0; i < nRows; i++)
- for (unsigned j = 0; j < nRows; j++)
+ for (int i : llvm::seq<int>(0, nRows))
+ for (int j : llvm::seq<int>(0, nRows))
tempInv.at(i, j) = tempInv.at(i, j) / m(i, i);
*inverse = std::move(tempInv);
}
Fraction determinant = 1;
- for (unsigned i = 0; i < nRows; i++)
+ for (int i : llvm::seq<int>(0, nRows))
determinant *= m.at(i, i);
return determinant;
@@ -678,8 +679,8 @@ Fr...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/82782
More information about the Mlir-commits
mailing list