Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mlir/Presburger: contribute a free-standing parser #94916

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions mlir/include/mlir/Analysis/Presburger/Matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ class Matrix {
/// Set the specified row to `elems`.
void setRow(unsigned row, ArrayRef<T> elems);

/// Add `elems` to the specified row.
void addToRow(unsigned row, ArrayRef<T> elems);

/// Insert columns having positions pos, pos + 1, ... pos + count - 1.
/// Columns that were at positions 0 to pos - 1 will stay where they are;
/// columns that were at positions pos to nColumns - 1 will be pushed to the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,37 @@
//
//===----------------------------------------------------------------------===//

#ifndef MLIR_UNITTESTS_ANALYSIS_PRESBURGER_PARSER_H
#define MLIR_UNITTESTS_ANALYSIS_PRESBURGER_PARSER_H
#ifndef MLIR_ANALYSIS_PRESBURGER_PARSER_H
#define MLIR_ANALYSIS_PRESBURGER_PARSER_H

#include "mlir/Analysis/Presburger/IntegerRelation.h"
#include "mlir/Analysis/Presburger/PWMAFunction.h"
#include "mlir/Analysis/Presburger/PresburgerRelation.h"
#include "mlir/AsmParser/AsmParser.h"
#include "mlir/Dialect/Affine/Analysis/AffineStructures.h"
#include "mlir/IR/AffineExpr.h"
#include "mlir/IR/AffineMap.h"
#include "mlir/IR/IntegerSet.h"

namespace mlir {
namespace presburger {

/// Parses an IntegerPolyhedron from a StringRef. It is expected that the string
/// represents a valid IntegerSet.
inline IntegerPolyhedron parseIntegerPolyhedron(StringRef str) {
MLIRContext context(MLIRContext::Threading::DISABLED);
return affine::FlatAffineValueConstraints(parseIntegerSet(str, &context));
}

namespace mlir::presburger {
using llvm::StringRef;

/// Parse an IntegerPolyhedron from a StringRef.
///
/// integer-set ::= dim-and-symbol-id-lists `:`
/// '(' affine-constraint-conjunction? ')'
/// affine-constraint-conjunction ::= affine-constraint (`,`
/// affine-constraint)*
///
IntegerPolyhedron parseIntegerPolyhedron(StringRef str);

/// Parse a MultiAffineFunction from a StringRef.
///
/// affine-map ::= dim-and-symbol-id-lists `->` multi-dim-affine-expr
///
/// multi-dim-affine-expr ::= `(` `)`
/// multi-dim-affine-expr ::= `(` affine-expr (`,` affine-expr)* `)`
MultiAffineFunction parseMultiAffineFunction(StringRef str);
Comment on lines +24 to +39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a number of documentation comments in this patch that are directly copy pasted from various places in MLIR. Could you please check them and update them based on where they are used?

For example, here, affine-map and integer-set are a MLIR thing. If Presburger ends up moving in LLVM, a user has no clue what an affine-map is. Please instead use the relevant naming here.


/// Parse a list of StringRefs to IntegerRelation and combine them into a
/// PresburgerSet by using the union operation. It is expected that the strings
/// are all valid IntegerSet representation and that all of them have compatible
/// spaces.
/// PresburgerSet by using the union operation. It is expected that the
/// strings are all valid IntegerSet representation and that all of them have
/// compatible spaces.
inline PresburgerSet parsePresburgerSet(ArrayRef<StringRef> strs) {
assert(!strs.empty() && "strs should not be empty");

Expand All @@ -47,25 +52,10 @@ inline PresburgerSet parsePresburgerSet(ArrayRef<StringRef> strs) {
return result;
}

inline MultiAffineFunction parseMultiAffineFunction(StringRef str) {
MLIRContext context(MLIRContext::Threading::DISABLED);

// TODO: Add default constructor for MultiAffineFunction.
MultiAffineFunction multiAff(PresburgerSpace::getRelationSpace(),
IntMatrix(0, 1));
if (getMultiAffineFunctionFromMap(parseAffineMap(str, &context), multiAff)
.failed())
llvm_unreachable(
"Failed to parse MultiAffineFunction because of semi-affinity");
return multiAff;
}

inline PWMAFunction
parsePWMAF(ArrayRef<std::pair<StringRef, StringRef>> pieces) {
assert(!pieces.empty() && "At least one piece should be present.");

MLIRContext context(MLIRContext::Threading::DISABLED);

IntegerPolyhedron initDomain = parseIntegerPolyhedron(pieces[0].first);
MultiAffineFunction initMultiAff = parseMultiAffineFunction(pieces[0].second);

Expand Down Expand Up @@ -100,8 +90,6 @@ parsePresburgerRelationFromPresburgerSet(ArrayRef<StringRef> strs,
result.convertVarKind(VarKind::SetDim, 0, numDomain, VarKind::Domain, 0);
return result;
}
} // namespace mlir::presburger

} // namespace presburger
} // namespace mlir

#endif // MLIR_UNITTESTS_ANALYSIS_PRESBURGER_PARSER_H
#endif // MLIR_ANALYSIS_PRESBURGER_PARSER_H
54 changes: 40 additions & 14 deletions mlir/include/mlir/Analysis/Presburger/PresburgerSpace.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ class PresburgerSpace {
return PresburgerSpace(/*numDomain=*/0, /*numRange=*/numDims, numSymbols,
numLocals);
}
PresburgerSpace() = default;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be a default constructor for PresburgerSpace. PresburgerSpace has members which need to be initialized to maintain invariants (Identifiers). Please remove the default constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I need the default constructor when I initialize ParserImpl. Again, either the existing ParseInfo was fine, or you let me uglify PresburgerSpace for the purposes of the parser.


/// Get the domain/range space of this space. The returned space is a set
/// space.
Expand All @@ -182,18 +183,43 @@ class PresburgerSpace {
/// Get the space without local variables.
PresburgerSpace getSpaceWithoutLocals() const;

unsigned getNumDomainVars() const { return numDomain; }
unsigned getNumRangeVars() const { return numRange; }
unsigned getNumSetDimVars() const { return numRange; }
unsigned getNumSymbolVars() const { return numSymbols; }
unsigned getNumLocalVars() const { return numLocals; }
constexpr unsigned getNumDomainVars() const { return numDomain; }
constexpr unsigned &numDomainVars() { return numDomain; }
constexpr unsigned getNumRangeVars() const { return numRange; }
constexpr unsigned &numRangeVars() { return numRange; }
constexpr unsigned getNumSetDimVars() const { return numRange; }
constexpr unsigned &numSetDimVars() { return numRange; }
constexpr unsigned getNumSymbolVars() const { return numSymbols; }
constexpr unsigned &numSymbolVars() { return numSymbols; }
constexpr unsigned getNumLocalVars() const { return numLocals; }
constexpr unsigned &numLocalVars() { return numLocals; }
constexpr unsigned getNumDimVars() const { return numDomain + numRange; }
constexpr unsigned getNumDimAndSymbolVars() const {
return getNumDimVars() + getNumSymbolVars();
Comment on lines +186 to +198
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These methods should not return a reference. This allows a user to change a field and get into an unspecified place. Please use insertVar/removeVar methods instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those methods are insufficient, as I directly set the variable to the value in some places.

}
constexpr unsigned getNumVars() const {
return getNumDimAndSymbolVars() + getNumLocalVars();
}
constexpr unsigned getNumCols() const { return getNumVars() + 1; }

constexpr unsigned getSetDimStartIdx() const { return 0; }
constexpr unsigned getSymbolStartIdx() const { return getNumDimVars(); }
constexpr unsigned getLocalVarStartIdx() const {
return getNumDimAndSymbolVars();
}
constexpr unsigned getConstantIdx() const { return getNumCols() - 1; }

unsigned getNumDimVars() const { return numDomain + numRange; }
unsigned getNumDimAndSymbolVars() const {
return numDomain + numRange + numSymbols;
constexpr bool isSetDimIdx(unsigned i) const {
return i < getSymbolStartIdx();
}
constexpr bool isSymbolIdx(unsigned i) const {
return i >= getSymbolStartIdx() && i < getLocalVarStartIdx();
}
constexpr bool isLocalVarIdx(unsigned i) const {
return i >= getLocalVarStartIdx() && i < getConstantIdx();
}
unsigned getNumVars() const {
return numDomain + numRange + numSymbols + numLocals;
constexpr bool isConstantIdx(unsigned i) const {
return i == getConstantIdx();
}
Comment on lines -191 to 223
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A user should not assume any ordering of different variable types. This is an internal detail and should not be exposed. I know there are some methods that exist, but they are supposed to be depreciated in future. You shouldn't have to make any ordering assumptions. If you absolutely have to, then please reuse the existing methods below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this "implementation detail" is used by CoefficientVector. Either I've got to have my own ParseInfo, or expose these methods in PresburgerSpace.


/// Get the number of vars of the specified kind.
Expand Down Expand Up @@ -321,18 +347,18 @@ class PresburgerSpace {

private:
// Number of variables corresponding to domain variables.
unsigned numDomain;
unsigned numDomain = 0;

// Number of variables corresponding to range variables.
unsigned numRange;
unsigned numRange = 0;

/// Number of variables corresponding to symbols (unknown but constant for
/// analysis).
unsigned numSymbols;
unsigned numSymbols = 0;

/// Number of variables corresponding to locals (variables corresponding
/// to existentially quantified variables).
unsigned numLocals;
unsigned numLocals = 0;

/// Stores whether or not identifiers are being used in this space.
bool usingIds = false;
Expand Down
2 changes: 2 additions & 0 deletions mlir/lib/Analysis/Presburger/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
add_subdirectory(Parser)

add_mlir_library(MLIRPresburger
Barvinok.cpp
IntegerRelation.cpp
Expand Down
8 changes: 8 additions & 0 deletions mlir/lib/Analysis/Presburger/Matrix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,14 @@ void Matrix<T>::setRow(unsigned row, ArrayRef<T> elems) {
at(row, i) = elems[i];
}

template <typename T>
void Matrix<T>::addToRow(unsigned row, ArrayRef<T> elems) {
assert(elems.size() == getNumColumns() &&
"elems size must match row length!");
for (unsigned i = 0, e = getNumColumns(); i < e; ++i)
at(row, i) += elems[i];
}

template <typename T>
void Matrix<T>::insertColumn(unsigned pos) {
insertColumns(pos, 1);
Expand Down
6 changes: 6 additions & 0 deletions mlir/lib/Analysis/Presburger/Parser/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
add_mlir_library(MLIRPresburgerParser
Flattener.cpp
Lexer.cpp
ParserImpl.cpp
ParseStructs.cpp
Token.cpp)
88 changes: 88 additions & 0 deletions mlir/lib/Analysis/Presburger/Parser/Flattener.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//===- Flattener.cpp - Presburger ParseStruct Flattener ---------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// This file implements the Flattener class for flattening the parse tree
// produced by the parser for the Presburger library.
//
//===----------------------------------------------------------------------===//

#include "Flattener.h"
#include "ParseStructs.h"

using namespace mlir;
using namespace presburger;

void Flattener::visitDiv(const PureAffineExprImpl &div) {
int64_t divisor = div.getDivisor();

// First construct the linear part of the divisor.
auto dividend = div.collectLinearTerms().getPadded(
space.getLocalVarStartIdx() + localExprs.size() + 1);

// Next, insert the non-linear coefficients.
for (const auto &[hash, adjustedMulFactor, adjustedLinearTerm] :
div.getNonLinearCoeffs()) {

// adjustedMulFactor will be mulFactor * -divisor in case of mod, and
// mulFactor in case of floordiv.
dividend[lookupLocal(hash)] = adjustedMulFactor;

// The expansion is either a modExpansion which we previously stored, or the
// adjustedLinearTerm, which is correct in the case when we're encountering
// the innermost mod for the first time.
CoefficientVector expansion =
lookupModExpansion(hash)
.value_or(adjustedLinearTerm)
.getPadded(space.getLocalVarStartIdx() + localExprs.size() + 1);
dividend += expansion;

// If this is a mod, insert the new computed expansion, which is the
// dividend * mulFactor.
if (div.isMod())
localModExpansion.insert({div.hash(), dividend * div.getMulFactor()});
}

cst.addLocalFloorDiv(dividend, divisor);
localExprs.insert(div.hash());
}

void Flattener::flatten(unsigned row, PureAffineExprImpl &div) {
// Visit divs inner to outer.
for (auto &nestedDiv : div.getNestedDivTerms())
flatten(row, *nestedDiv);

if (div.hasDivisor()) {
visitDiv(div);
return;
}

// Hit multiple times every time we have a linear sub-expression, but the
// row is overwritten to consider only the outermost div, which is hit
// last.

// Set the linear part of the row.
setRow(row, div.getLinearDividend());

// Set the non-linear coefficients.
for (const auto &[hash, adjustedMulFactor, adjustedLinearTerm] :
div.getNonLinearCoeffs()) {
flatMatrix(row, lookupLocal(hash)) = adjustedMulFactor;
CoefficientVector expansion =
lookupModExpansion(hash).value_or(adjustedLinearTerm);
addToRow(row, expansion);
}
}

std::pair<IntMatrix, IntegerPolyhedron> Flattener::flatten() {
// Use the same flattener to simplify each expression successively. This way
// local variables / expressions are shared.
for (const auto &[row, expr] : enumerate(exprs))
flatten(row, *expr);

return {flatMatrix, cst};
}
67 changes: 67 additions & 0 deletions mlir/lib/Analysis/Presburger/Parser/Flattener.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//===- Flattener.h - Presburger ParseStruct Flattener -----------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef MLIR_ANALYSIS_PRESBURGER_PARSER_FLATTENER_H
#define MLIR_ANALYSIS_PRESBURGER_PARSER_FLATTENER_H

#include "ParseStructs.h"
#include "mlir/Analysis/Presburger/IntegerRelation.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SetVector.h"

namespace mlir::presburger {
using llvm::SmallDenseMap;
using llvm::SmallSetVector;

class Flattener : public FinalParseResult {
public:
// The final flattened result is stored here.
IntMatrix flatMatrix;

// We maintain a set of divs that we have seen while flattening. The size of
// this set is at most info.numDivs, hitting info.numDivs at the end of the
// flattening, if that expression contains all the possible divs.
SmallSetVector<size_t, 4> localExprs;

// We maintain a mapping between local mods and their expansions. The vector
// is the dividend.
SmallDenseMap<size_t, CoefficientVector, 2> localModExpansion;

Flattener(FinalParseResult &&parseResult)
: FinalParseResult(std::move(parseResult)),
flatMatrix(exprs.size(), space.getNumCols()) {}

std::pair<IntMatrix, IntegerPolyhedron> flatten();

private:
void flatten(unsigned row, PureAffineExprImpl &div);
void visitDiv(const PureAffineExprImpl &div);

void addToRow(unsigned row, const CoefficientVector &l) {
flatMatrix.addToRow(row,
getDynamicAPIntVec(l.getPadded(space.getNumCols())));
}
void setRow(unsigned row, const CoefficientVector &l) {
flatMatrix.setRow(row, getDynamicAPIntVec(l.getPadded(space.getNumCols())));
}

unsigned lookupLocal(size_t hash) {
const auto *it = find(localExprs, hash);
assert(it != localExprs.end() &&
"Local expression not found; walking from inner to outer?");
return space.getLocalVarStartIdx() + it - localExprs.begin();
}
std::optional<CoefficientVector> lookupModExpansion(size_t hash) {
return localModExpansion.contains(hash)
? std::make_optional(localModExpansion.at(hash))
artagnon marked this conversation as resolved.
Show resolved Hide resolved
: std::nullopt;
artagnon marked this conversation as resolved.
Show resolved Hide resolved
}
};
} // namespace mlir::presburger

#endif // MLIR_ANALYSIS_PRESBURGER_PARSER_FLATTENER_H
Loading
Loading