Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 10 additions & 17 deletions mlir/lib/Analysis/Presburger/IntegerRelation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,25 +229,17 @@ PresburgerRelation IntegerRelation::computeReprWithOnlyDivLocals() const {
// SymbolicLexOpt requires these to form a contiguous range.
//
// Take a copy so we can perform mutations.
IntegerRelation copy = *this;
std::vector<MaybeLocalRepr> reprs(getNumLocalVars());
copy.getLocalReprs(&reprs);
this->getLocalReprs(&reprs);

// Iterate through all the locals. The last `numNonDivLocals` are the locals
// that have been scanned already and do not have division representations.
unsigned numNonDivLocals = 0;
unsigned offset = copy.getVarKindOffset(VarKind::Local);
for (unsigned i = 0, e = copy.getNumLocalVars(); i < e - numNonDivLocals;) {
if (!reprs[i]) {
// Whenever we come across a local that does not have a division
// representation, we swap it to the `numNonDivLocals`-th last position
// and increment `numNonDivLocal`s. `reprs` also needs to be swapped.
copy.swapVar(offset + i, offset + e - numNonDivLocals - 1);
std::swap(reprs[i], reprs[e - numNonDivLocals - 1]);
++numNonDivLocals;
llvm::SmallBitVector isSymbol(getNumVars(), true);
unsigned offset = getVarKindOffset(VarKind::Local);
for (unsigned i = 0, e = getNumLocalVars(); i < e; ++i) {
if (reprs[i])
continue;
}
++i;
isSymbol[offset + i] = false;
++numNonDivLocals;
}

// If there are no non-div locals, we're done.
Expand All @@ -266,9 +258,10 @@ PresburgerRelation IntegerRelation::computeReprWithOnlyDivLocals() const {
// and the returned set of assignments to the "symbols" that makes the lexmin
// unbounded.
SymbolicLexOpt lexminResult =
SymbolicLexSimplex(copy, /*symbolOffset*/ 0,
SymbolicLexSimplex(*this,
IntegerPolyhedron(PresburgerSpace::getSetSpace(
/*numDims=*/copy.getNumVars() - numNonDivLocals)))
/*numDims=*/getNumVars() - numNonDivLocals)),
isSymbol)
.computeSymbolicIntegerLexMin();
PresburgerRelation result =
lexminResult.lexopt.getDomain().unionSet(lexminResult.unboundedDomain);
Expand Down
15 changes: 8 additions & 7 deletions mlir/lib/Analysis/Presburger/Simplex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,14 @@ SimplexBase::SimplexBase(unsigned nVar, bool mustUseBigM,
const llvm::SmallBitVector &isSymbol)
: SimplexBase(nVar, mustUseBigM) {
assert(isSymbol.size() == nVar && "invalid bitmask!");
// Invariant: nSymbol is the number of symbols that have been marked
// already and these occupy the columns
// [getNumFixedCols(), getNumFixedCols() + nSymbol).
for (unsigned symbolIdx : isSymbol.set_bits()) {
var[symbolIdx].isSymbol = true;
swapColumns(var[symbolIdx].pos, getNumFixedCols() + nSymbol);
++nSymbol;
// Iterate through all the variables. Move symbols to the left and non-symbols
// to the right while preserving relative ordering.
for (unsigned i = 0; i < nVar; ++i) {
if (isSymbol[i]) {
var[i].isSymbol = true;
swapColumns(var[i].pos, getNumFixedCols() + nSymbol);
nSymbol++;
}
Copy link
Member

Choose a reason for hiding this comment

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

The new code looks equivalent to the old -- am I missing someting?

Copy link
Member

Choose a reason for hiding this comment

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

I suppose set bits doesn't guarantee the order of indices returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it was equivalent. I updated the code to use a stable partitioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(The old code in computeReprWithOnlyDivLocals was not preserving the relative order of symbols while this SimplexBase constructor was not preserving the relative order of non-symbols. The updated code ensures the partitioning is now stable.)

}
}

Expand Down
14 changes: 14 additions & 0 deletions mlir/unittests/Analysis/Presburger/PresburgerSetTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,20 @@ TEST(SetTest, computeReprWithOnlyDivLocals) {
PresburgerSet(parseIntegerPolyhedron(
{"(x) : (x - 3*(x floordiv 3) == 0)"})),
/*numToProject=*/2);

testComputeRepr(
parseIntegerPolyhedron("(e, a, b, c)[] : ("
"a >= 0,"
"b >= 0,"
"c >= 0,"
"e >= 0,"
"15 - a >= 0,"
"7 - b >= 0,"
"5 - c >= 0,"
"e - a * 192 - c * 32 - b * 4 >= 0,"
"3 - e + a * 192 + c * 32 + b * 4 >= 0)"),
parsePresburgerSet({"(i) : (i >= 0, i <= 3071)"}),
/*numToProject=*/3);
}

TEST(SetTest, subtractOutputSizeRegression) {
Expand Down
Loading