[MLIR][Presburger] Fix bug in Identifier::isEqual assert

[iambrj]

Make identifiers::isEqual return false instead of failing assertion when identifiers are not equal.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Bharathi Ramana Joshi (iambrj)

Changes

Make identifiers::isEqual return false instead of failing assertion when identifiers are not equal.

---

Full diff: https://github.com/llvm/llvm-project/pull/76380.diff

2 Files Affected:

• (modified) mlir/lib/Analysis/Presburger/PresburgerSpace.cpp (+3-2)
• (modified) mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp (+14)

diff --git a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
index cf1b3befbc89f8..185da462aa4453 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
@@ -18,8 +18,9 @@ using namespace presburger;
 bool Identifier::isEqual(const Identifier &other) const {
   if (value == nullptr || other.value == nullptr)
     return false;
-  assert(value == other.value && idType == other.idType &&
-         "Values of Identifiers are equal but their types do not match.");
+  assert(value != other.value ||
+         (value == other.value && idType == other.idType &&
+          "Values of Identifiers are equal but their types do not match."));
   return value == other.value;
 }
 
diff --git a/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp b/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp
index dd06d462f54bee..8229199b233471 100644
--- a/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp
@@ -110,6 +110,20 @@ TEST(PresburgerSpaceTest, removeVarRangeIdentifier) {
   EXPECT_EQ(space.getId(VarKind::Range, 1), Identifier(&identifiers[5]));
 }
 
+TEST(PresburgerSpaceTest, IdentifierIsEqual) {
+  PresburgerSpace space = PresburgerSpace::getRelationSpace(1, 2, 0, 0);
+  space.resetIds();
+
+  int identifiers[2] = {0, 1};
+  space.getId(VarKind::Domain, 0) = Identifier(&identifiers[0]);
+  space.getId(VarKind::Range, 0) = Identifier(&identifiers[0]);
+  space.getId(VarKind::Range, 1) = Identifier(&identifiers[1]);
+
+  EXPECT_EQ(space.getId(VarKind::Domain, 0), space.getId(VarKind::Range, 0));
+  EXPECT_FALSE(
+      space.getId(VarKind::Range, 0).isEqual(space.getId(VarKind::Range, 1)));
+}
+
 TEST(PresburgerSpaceTest, convertVarKind) {
   PresburgerSpace space = PresburgerSpace::getRelationSpace(2, 2, 0, 0);
   space.resetIds();

[llvmbot]

@llvm/pr-subscribers-mlir-presburger

Author: Bharathi Ramana Joshi (iambrj)

Changes

Make identifiers::isEqual return false instead of failing assertion when identifiers are not equal.

---

Full diff: https://github.com/llvm/llvm-project/pull/76380.diff

2 Files Affected:

• (modified) mlir/lib/Analysis/Presburger/PresburgerSpace.cpp (+3-2)
• (modified) mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp (+14)

diff --git a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
index cf1b3befbc89f8..185da462aa4453 100644
--- a/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
+++ b/mlir/lib/Analysis/Presburger/PresburgerSpace.cpp
@@ -18,8 +18,9 @@ using namespace presburger;
 bool Identifier::isEqual(const Identifier &other) const {
   if (value == nullptr || other.value == nullptr)
     return false;
-  assert(value == other.value && idType == other.idType &&
-         "Values of Identifiers are equal but their types do not match.");
+  assert(value != other.value ||
+         (value == other.value && idType == other.idType &&
+          "Values of Identifiers are equal but their types do not match."));
   return value == other.value;
 }
 
diff --git a/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp b/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp
index dd06d462f54bee..8229199b233471 100644
--- a/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp
+++ b/mlir/unittests/Analysis/Presburger/PresburgerSpaceTest.cpp
@@ -110,6 +110,20 @@ TEST(PresburgerSpaceTest, removeVarRangeIdentifier) {
   EXPECT_EQ(space.getId(VarKind::Range, 1), Identifier(&identifiers[5]));
 }
 
+TEST(PresburgerSpaceTest, IdentifierIsEqual) {
+  PresburgerSpace space = PresburgerSpace::getRelationSpace(1, 2, 0, 0);
+  space.resetIds();
+
+  int identifiers[2] = {0, 1};
+  space.getId(VarKind::Domain, 0) = Identifier(&identifiers[0]);
+  space.getId(VarKind::Range, 0) = Identifier(&identifiers[0]);
+  space.getId(VarKind::Range, 1) = Identifier(&identifiers[1]);
+
+  EXPECT_EQ(space.getId(VarKind::Domain, 0), space.getId(VarKind::Range, 0));
+  EXPECT_FALSE(
+      space.getId(VarKind::Range, 0).isEqual(space.getId(VarKind::Range, 1)));
+}
+
 TEST(PresburgerSpaceTest, convertVarKind) {
   PresburgerSpace space = PresburgerSpace::getRelationSpace(2, 2, 0, 0);
   space.resetIds();

[Groverkss]

LGTM