[Mlir-commits] [mlir] [mlir][OperationEquivalence] Add an extra callback to hook operation equivalence check (PR #73455)

[Mehdi Amini]

================
@@ -776,24 +775,39 @@ OperationEquivalence::isRegionEquivalentTo(Region *lhs, Region *rhs,
       flags);
 }
 
+/*static*/ llvm::hash_code OperationEquivalence::simpleOpHash(Operation *op) {
+  return llvm::hash_combine(op->getName(), op->getResultTypes(),
+                            op->hashProperties(),
+                            op->getDiscardableAttrDictionary());
+}
+
+/*static*/ LogicalResult
+OperationEquivalence::simpleOpEquivalence(Operation *lhs, Operation *rhs) {
+  return LogicalResult::success(
+      lhs->getName() == rhs->getName() &&
+      lhs->getDiscardableAttrDictionary() ==
+          rhs->getDiscardableAttrDictionary() &&
+      lhs->getNumRegions() == rhs->getNumRegions() &&
+      lhs->getNumSuccessors() == rhs->getNumSuccessors() &&
+      lhs->getNumOperands() == rhs->getNumOperands() &&
+      lhs->getNumResults() == rhs->getNumResults() &&
+      lhs->getName().compareOpProperties(lhs->getPropertiesStorage(),
+                                         rhs->getPropertiesStorage()));
+}
----------------
joker-eph wrote:

I'm not convinced this is the right level of granularity in terms of "flexibility" vs "potential footgun" and "ease of use".

As a user who wants to just ignore discardable attributes for example, I shouldn't have to redefine all of this. The comparison of properties here is using quite low-level APIs for example. This seems a bit tedious and error prone.


Also we're diverging from the approach we used for Locations, and ideally we should keep some sort of unified mechanism (probably callback based?).




https://github.com/llvm/llvm-project/pull/73455