[mlir] [presburger] Add IntegerRelation::rangeProduct#148092
Merged
[mlir] [presburger] Add IntegerRelation::rangeProduct#148092
Conversation
j2kun
changed the title
Member
|
@llvm/pr-subscribers-mlir Author: Jeremy Kun (j2kun) ChangesThis is intended to match I'd like to add some more tests, so hoping for a brief early review to make sure I'm going in the right direction. Full diff: https://github.com/llvm/llvm-project/pull/148092.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
index b68262f09f485..ae213181f2c3b 100644
--- a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
+++ b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
@@ -707,6 +707,21 @@ class IntegerRelation {
/// this for uniformity with `applyDomain`.
void applyRange(const IntegerRelation &rel);
+ /// Let the relation `this` be R1, and the relation `rel` be R2. Requires
+ /// R1 and R2 to have the same domain.
+ ///
+ /// This operation computes the relation whose domain is the same as R1 and
+ /// whose range is the product of the ranges of R1 and R2, and whose
+ /// constraints are the conjunction of the constraints of R1 and R2 applied
+ /// to the relevant subspaces of the range.
+ ///
+ /// Example:
+ ///
+ /// R1: (i, j) -> k : f(i, j, k) = 0
+ /// R2: (i, j) -> l : g(i, j, l) = 0
+ /// R1.rangeProduct(R2): (i, j) -> (k, l) : f(i, j, k) = 0 and g(i, j, l) = 0
+ IntegerRelation rangeProduct(const IntegerRelation &rel);
+
/// Given a relation `other: (A -> B)`, this operation merges the symbol and
/// local variables and then takes the composition of `other` on `this: (B ->
/// C)`. The resulting relation represents tuples of the form: `A -> C`.
diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index 631e085574fd0..87cec81e86b59 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -2481,6 +2481,41 @@ void IntegerRelation::applyDomain(const IntegerRelation &rel) {
void IntegerRelation::applyRange(const IntegerRelation &rel) { compose(rel); }
+IntegerRelation IntegerRelation::rangeProduct(const IntegerRelation &rel) {
+ /// R1: (i, j) -> k : f(i, j, k) = 0
+ /// R2: (i, j) -> l : g(i, j, l) = 0
+ /// R1.rangeProduct(R2): (i, j) -> (k, l) : f(i, j, k) = 0 and g(i, j, l) = 0
+ assert(getNumDomainVars() == rel.getNumDomainVars() &&
+ "Range product is only defined for relations with equal domains");
+
+ // explicit copy of the context relation
+ IntegerRelation result = *this;
+ unsigned srcOffset = getVarKindOffset(VarKind::Range);
+ unsigned newNumRangeVars = rel.getNumRangeVars();
+
+ result.appendVar(VarKind::Range, newNumRangeVars);
+
+ for (unsigned i = 0; i < rel.getNumEqualities(); ++i) {
+ // Add a new equality that uses the new range variables.
+ // The old equality is a list of coefficients of the variables
+ // from `rel`, and so the range variables need to be shifted
+ // right by the number of range variables added to `result`.
+ SmallVector<DynamicAPInt> copy =
+ SmallVector<DynamicAPInt>(rel.getEquality(i));
+ copy.insert(copy.begin() + srcOffset, newNumRangeVars, DynamicAPInt(0));
+ result.addEquality(copy);
+ }
+
+ for (unsigned i = 0; i < rel.getNumInequalities(); ++i) {
+ SmallVector<DynamicAPInt> copy =
+ SmallVector<DynamicAPInt>(rel.getInequality(i));
+ copy.insert(copy.begin() + srcOffset, newNumRangeVars, DynamicAPInt(0));
+ result.addInequality(copy);
+ }
+
+ return result;
+}
+
void IntegerRelation::printSpace(raw_ostream &os) const {
space.print(os);
os << getNumConstraints() << " constraints\n";
diff --git a/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp b/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
index 7df500bc9568a..dd8b9e3f03330 100644
--- a/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
@@ -608,3 +608,17 @@ TEST(IntegerRelationTest, convertVarKindToLocal) {
EXPECT_EQ(space.getId(VarKind::Symbol, 0), Identifier(&identifiers[3]));
EXPECT_EQ(space.getId(VarKind::Symbol, 1), Identifier(&identifiers[4]));
}
+
+TEST(IntegerRelationTest, rangeProduct) {
+ IntegerRelation r1 = parseRelationFromSet(
+ "(i, j, k) : (2*i + 3*k == 0, i >= 0, j >= 0, k >= 0)", 2);
+ IntegerRelation r2 = parseRelationFromSet(
+ "(i, j, l) : (4*i + 6*j + 9*l == 0, i >= 0, j >= 0, l >= 0)", 2);
+
+ IntegerRelation rangeProd = r1.rangeProduct(r2);
+ IntegerRelation expected = parseRelationFromSet(
+ "(i, j, k, l) : (2*i + 3*k == 0, 4*i + 6*j + 9*l == 0, i >= 0, j >= 0, k >= 0, l >= 0)", 2);
+
+ EXPECT_TRUE(expected.isEqual(rangeProd));
+}
+
|
Member
|
@llvm/pr-subscribers-mlir-presburger Author: Jeremy Kun (j2kun) ChangesThis is intended to match I'd like to add some more tests, so hoping for a brief early review to make sure I'm going in the right direction. Full diff: https://github.com/llvm/llvm-project/pull/148092.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
index b68262f09f485..ae213181f2c3b 100644
--- a/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
+++ b/mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
@@ -707,6 +707,21 @@ class IntegerRelation {
/// this for uniformity with `applyDomain`.
void applyRange(const IntegerRelation &rel);
+ /// Let the relation `this` be R1, and the relation `rel` be R2. Requires
+ /// R1 and R2 to have the same domain.
+ ///
+ /// This operation computes the relation whose domain is the same as R1 and
+ /// whose range is the product of the ranges of R1 and R2, and whose
+ /// constraints are the conjunction of the constraints of R1 and R2 applied
+ /// to the relevant subspaces of the range.
+ ///
+ /// Example:
+ ///
+ /// R1: (i, j) -> k : f(i, j, k) = 0
+ /// R2: (i, j) -> l : g(i, j, l) = 0
+ /// R1.rangeProduct(R2): (i, j) -> (k, l) : f(i, j, k) = 0 and g(i, j, l) = 0
+ IntegerRelation rangeProduct(const IntegerRelation &rel);
+
/// Given a relation `other: (A -> B)`, this operation merges the symbol and
/// local variables and then takes the composition of `other` on `this: (B ->
/// C)`. The resulting relation represents tuples of the form: `A -> C`.
diff --git a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
index 631e085574fd0..87cec81e86b59 100644
--- a/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
+++ b/mlir/lib/Analysis/Presburger/IntegerRelation.cpp
@@ -2481,6 +2481,41 @@ void IntegerRelation::applyDomain(const IntegerRelation &rel) {
void IntegerRelation::applyRange(const IntegerRelation &rel) { compose(rel); }
+IntegerRelation IntegerRelation::rangeProduct(const IntegerRelation &rel) {
+ /// R1: (i, j) -> k : f(i, j, k) = 0
+ /// R2: (i, j) -> l : g(i, j, l) = 0
+ /// R1.rangeProduct(R2): (i, j) -> (k, l) : f(i, j, k) = 0 and g(i, j, l) = 0
+ assert(getNumDomainVars() == rel.getNumDomainVars() &&
+ "Range product is only defined for relations with equal domains");
+
+ // explicit copy of the context relation
+ IntegerRelation result = *this;
+ unsigned srcOffset = getVarKindOffset(VarKind::Range);
+ unsigned newNumRangeVars = rel.getNumRangeVars();
+
+ result.appendVar(VarKind::Range, newNumRangeVars);
+
+ for (unsigned i = 0; i < rel.getNumEqualities(); ++i) {
+ // Add a new equality that uses the new range variables.
+ // The old equality is a list of coefficients of the variables
+ // from `rel`, and so the range variables need to be shifted
+ // right by the number of range variables added to `result`.
+ SmallVector<DynamicAPInt> copy =
+ SmallVector<DynamicAPInt>(rel.getEquality(i));
+ copy.insert(copy.begin() + srcOffset, newNumRangeVars, DynamicAPInt(0));
+ result.addEquality(copy);
+ }
+
+ for (unsigned i = 0; i < rel.getNumInequalities(); ++i) {
+ SmallVector<DynamicAPInt> copy =
+ SmallVector<DynamicAPInt>(rel.getInequality(i));
+ copy.insert(copy.begin() + srcOffset, newNumRangeVars, DynamicAPInt(0));
+ result.addInequality(copy);
+ }
+
+ return result;
+}
+
void IntegerRelation::printSpace(raw_ostream &os) const {
space.print(os);
os << getNumConstraints() << " constraints\n";
diff --git a/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp b/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
index 7df500bc9568a..dd8b9e3f03330 100644
--- a/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/IntegerRelationTest.cpp
@@ -608,3 +608,17 @@ TEST(IntegerRelationTest, convertVarKindToLocal) {
EXPECT_EQ(space.getId(VarKind::Symbol, 0), Identifier(&identifiers[3]));
EXPECT_EQ(space.getId(VarKind::Symbol, 1), Identifier(&identifiers[4]));
}
+
+TEST(IntegerRelationTest, rangeProduct) {
+ IntegerRelation r1 = parseRelationFromSet(
+ "(i, j, k) : (2*i + 3*k == 0, i >= 0, j >= 0, k >= 0)", 2);
+ IntegerRelation r2 = parseRelationFromSet(
+ "(i, j, l) : (4*i + 6*j + 9*l == 0, i >= 0, j >= 0, l >= 0)", 2);
+
+ IntegerRelation rangeProd = r1.rangeProduct(r2);
+ IntegerRelation expected = parseRelationFromSet(
+ "(i, j, k, l) : (2*i + 3*k == 0, 4*i + 6*j + 9*l == 0, i >= 0, j >= 0, k >= 0, l >= 0)", 2);
+
+ EXPECT_TRUE(expected.isEqual(rangeProd));
+}
+
|
|
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
|
I didn't read any implementation details yet |
superty
reviewed
Jul 15, 2025
Member
superty
left a comment
superty
left a comment
There was a problem hiding this comment.
Thanks for the PR! Implementation looks good to me. I left some nitpick comments.
j2kun
force-pushed
the
flat_range_product
branch
from
1107d86 to
1636a92
Compare
Contributor
Author
|
Fixed an indexing bug and added some more tests. Should be ready for a final review now. |
superty
reviewed
Jul 16, 2025
superty
reviewed
Jul 16, 2025
superty
reviewed
Jul 16, 2025
superty
approved these changes
Jul 16, 2025
Member
superty
left a comment
superty
left a comment
There was a problem hiding this comment.
Okay to merge once you address the comments, assuming tests pass. Thanks again!
This is intended to match `isl::map`'s `flat_range_product`.
j2kun
force-pushed
the
flat_range_product
branch
from
8ce5827 to
cf868a9
Compare
This was referenced Jul 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is intended to match
isl::map'sflat_range_product.I'd like to add some more tests, so hoping for a brief early review to make sure I'm going in the right direction.