Skip to content

Commit 4bf9cbc

Browse files
committed
[MLIR][Presburger] subtract: improve redundant constraint detection
When constraints in the two operands make each other redundant, prefer constraints of the second because this affects the number of sets in the output at each level; reducing these can help prevent exponential blowup. This is accomplished by adding extra overloads to Simplex::detectRedundant that only scan a subrange of the constraints for redundancy. Reviewed By: Groverkss Differential Revision: https://reviews.llvm.org/D127237
1 parent f28e48f commit 4bf9cbc

File tree

4 files changed

+46
-5
lines changed

4 files changed

+46
-5
lines changed

mlir/include/mlir/Analysis/Presburger/Simplex.h

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,28 @@ class Simplex : public SimplexBase {
705705
/// the set of solutions does not change if these constraints are removed.
706706
/// Marks these constraints as redundant. Whether a specific constraint has
707707
/// been marked redundant can be queried using isMarkedRedundant.
708-
void detectRedundant();
708+
///
709+
/// The first overload only tries to find redundant constraints with indices
710+
/// in the range [offset, offset + count), by scanning constraints from left
711+
/// to right in this range. If `count` is not provided, all constraints
712+
/// starting at `offset` are scanned, and if neither are provided, all
713+
/// constraints are scanned, starting from 0 and going to the last constraint.
714+
///
715+
/// As an example, in the set (x) : (x >= 0, x >= 0, x >= 0), calling
716+
/// `detectRedundant` with no parameters will result in the first two
717+
/// constraints being marked redundant. All copies cannot be marked redundant
718+
/// because removing all the constraints changes the set. The first two are
719+
/// the ones marked redundant because we scan from left to right. Thus, when
720+
/// there is some preference among the constraints as to which should be
721+
/// marked redundant with priority when there are multiple possibilities, this
722+
/// could be accomplished by succesive calls to detectRedundant(offset,
723+
/// count).
724+
void detectRedundant(unsigned offset, unsigned count);
725+
void detectRedundant(unsigned offset) {
726+
assert(offset <= con.size() && "invalid offset!");
727+
detectRedundant(offset, con.size() - offset);
728+
}
729+
void detectRedundant() { detectRedundant(0, con.size()); }
709730

710731
/// Returns a (min, max) pair denoting the minimum and maximum integer values
711732
/// of the given expression. If no integer value exists, both results will be

mlir/lib/Analysis/Presburger/PresburgerRelation.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,11 +291,23 @@ static PresburgerRelation getSetDifference(IntegerRelation b,
291291
continue;
292292
}
293293

294-
simplex.detectRedundant();
295-
296294
// Equalities are added to simplex as a pair of inequalities.
297295
unsigned totalNewSimplexInequalities =
298296
2 * sI.getNumEqualities() + sI.getNumInequalities();
297+
// Look for redundant constraints among the constraints of sI. We don't
298+
// care about redundant constraints in `b` at this point.
299+
//
300+
// When there are two copies of a constraint in `simplex`, i.e., among the
301+
// constraints of `b` and `sI`, only one of them can be marked redundant.
302+
// (Assuming no other constraint makes these redundant.)
303+
//
304+
// In a case where there is one copy in `b` and one in `sI`, we want the
305+
// one in `sI` to be marked, not the one in `b`. Therefore, it's not
306+
// enough to ignore the constraints of `b` when checking which
307+
// constraints `detectRedundant` has marked redundant; we explicitly tell
308+
// `detectRedundant` to only mark constraints from `sI` as being
309+
// redundant.
310+
simplex.detectRedundant(offset, totalNewSimplexInequalities);
299311
for (unsigned j = 0; j < totalNewSimplexInequalities; j++)
300312
canIgnoreIneq[j] = simplex.isMarkedRedundant(offset + j);
301313
simplex.rollback(snapshotBeforeIntersect);

mlir/lib/Analysis/Presburger/Simplex.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,8 @@ void Simplex::markRowRedundant(Unknown &u) {
13871387
}
13881388

13891389
/// Find a subset of constraints that is redundant and mark them redundant.
1390-
void Simplex::detectRedundant() {
1390+
void Simplex::detectRedundant(unsigned offset, unsigned count) {
1391+
assert(offset + count <= con.size() && "invalid range!");
13911392
// It is not meaningful to talk about redundancy for empty sets.
13921393
if (empty)
13931394
return;
@@ -1401,7 +1402,8 @@ void Simplex::detectRedundant() {
14011402
// two identical constraints both being marked redundant since each is
14021403
// redundant given the other one. In this example, only the first of the
14031404
// constraints that is processed will get marked redundant, as it should be.
1404-
for (Unknown &u : con) {
1405+
for (unsigned i = 0; i < count; ++i) {
1406+
Unknown &u = con[offset + i];
14051407
if (u.orientation == Orientation::Column) {
14061408
unsigned column = u.pos;
14071409
Optional<unsigned> pivotRow = findPivotRow({}, Direction::Down, column);

mlir/unittests/Analysis/Presburger/PresburgerSetTest.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,4 +763,10 @@ TEST(SetTest, subtractOutputSizeRegression) {
763763
// Previously, the subtraction result was producing an extra empty set, which
764764
// is correct, but bad for output size.
765765
EXPECT_EQ(result.getNumDisjuncts(), 1u);
766+
767+
PresburgerSet subtractSelf = set1.subtract(set1);
768+
EXPECT_TRUE(subtractSelf.isIntegerEmpty());
769+
// Previously, the subtraction result was producing several unnecessary empty
770+
// sets, which is correct, but bad for output size.
771+
EXPECT_EQ(subtractSelf.getNumDisjuncts(), 0u);
766772
}

0 commit comments

Comments
 (0)