[Mlir-commits] [mlir] [mlir] Add option to ignore commutativity in OperationEquality (PR #181507)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Sat Feb 14 12:52:37 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir
Author: None (jumerckx)
<details>
<summary>Changes</summary>
This started as me wanting to be able to disable commutativity-aware operation equivalence (we want this in [Tamagoyaki](https://github.com/jumerckx/Tamagoyaki)).
Simply extending `test-operations-equality` to test this does not suffice because commutativity equivalence did not actually fully check if operand lists were commutatively equivalent. (It gave up if operand lists are not permutations of each other).
I believe that by mapping values onto their entry in `equivalentValues` (`equivalentValues.lookup_or(a, a)`), it is safe to compare pointers and get the full commutativity equivalence check.
I understand that `checkCommutativeEquivalent` has become more expensive with this change. If people are against this change but are not opposed to the ignoreCommutativity flag, I can also split that off and figure out a different way to test?
---
Full diff: https://github.com/llvm/llvm-project/pull/181507.diff
5 Files Affected:
- (modified) mlir/include/mlir/IR/OperationSupport.h (+7-3)
- (modified) mlir/lib/IR/OperationSupport.cpp (+15-9)
- (modified) mlir/test/Dialect/Func/duplicate-function-elimination.mlir (+2-3)
- (modified) mlir/test/IR/operation-equality.mlir (+28)
- (modified) mlir/test/lib/IR/TestOperationEquals.cpp (+2)
``````````diff
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 1ff7c56ddca38..9aab65b6fb5a5 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -1331,7 +1331,11 @@ struct OperationEquivalence {
// When provided, the properties attached to the operation are ignored.
IgnoreProperties = 4,
- LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ IgnoreProperties)
+ // When provided, the commutativity of the operation is ignored, and operands
+ // are compared in an order-sensitive way.
+ IgnoreCommutativity = 8,
+
+ LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ IgnoreCommutativity)
};
/// Compute a hash for the given operation.
@@ -1348,8 +1352,8 @@ struct OperationEquivalence {
/// Helper that can be used with `computeHash` above to ignore operation
/// operands/result mapping.
static llvm::hash_code ignoreHashValue(Value) { return llvm::hash_code{}; }
- /// Helper that can be used with `computeHash` above to ignore operation
- /// operands/result mapping.
+ /// Helper that can be used with `computeHash` to compute the hash value
+ /// of operands/results directly.
static llvm::hash_code directHashValue(Value v) { return hash_value(v); }
/// Compare two operations (including their regions) and return if they are
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 2a37f3860fe00..a7a67744b65d5 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -689,7 +689,8 @@ llvm::hash_code OperationEquivalence::computeHash(
hash = llvm::hash_combine(hash, op->getLoc());
// - Operands
- if (op->hasTrait<mlir::OpTrait::IsCommutative>() &&
+ if (!(flags & Flags::IgnoreCommutativity) &&
+ op->hasTrait<mlir::OpTrait::IsCommutative>() &&
op->getNumOperands() > 0) {
size_t operandHash = hashOperands(op->getOperand(0));
for (auto operand : op->getOperands().drop_front())
@@ -784,11 +785,12 @@ struct ValueEquivalenceCache {
if (lhsIt == lhsRange.end())
return success();
- // Handle another simple case where operands are just a permutation.
- // Note: This is not sufficient, this handles simple cases relatively
- // cheaply.
- auto sortValues = [](ValueRange values) {
- SmallVector<Value> sortedValues = llvm::to_vector(values);
+ // Replace values with their entry in equivalentValues if they're in there
+ // that way, a sorted pointer comparison is enough to determine commutativity.
+ auto sortValues = [this](ValueRange values) {
+ SmallVector<Value> sortedValues = llvm::map_to_vector(values, [this](Value a) {
+ return equivalentValues.lookup_or(a, a);
+ });
llvm::sort(sortedValues, [](Value a, Value b) {
return a.getAsOpaquePointer() < b.getAsOpaquePointer();
});
@@ -796,7 +798,10 @@ struct ValueEquivalenceCache {
};
auto lhsSorted = sortValues({lhsIt, lhsRange.end()});
auto rhsSorted = sortValues({rhsIt, rhsRange.end()});
- return success(lhsSorted == rhsSorted);
+ if (lhsSorted == rhsSorted) {
+ return success();
+ }
+ return failure();
}
void markEquivalent(Value lhsResult, Value rhsResult) {
auto insertion = equivalentValues.insert({lhsResult, rhsResult});
@@ -854,7 +859,8 @@ OperationEquivalence::isRegionEquivalentTo(Region *lhs, Region *rhs,
return false;
// 2. Compare operands.
- if (checkCommutativeEquivalent &&
+ if (!(flags & IgnoreCommutativity) &&
+ checkCommutativeEquivalent &&
lhs->hasTrait<mlir::OpTrait::IsCommutative>()) {
auto lhsRange = lhs->getOperands();
auto rhsRange = rhs->getOperands();
@@ -888,7 +894,7 @@ OperationEquivalence::isRegionEquivalentTo(Region *lhs, Region *rhs,
for (auto regionPair : llvm::zip(lhs->getRegions(), rhs->getRegions()))
if (!isRegionEquivalentTo(&std::get<0>(regionPair),
&std::get<1>(regionPair), checkEquivalent,
- markEquivalent, flags))
+ markEquivalent, flags, checkCommutativeEquivalent))
return false;
return true;
diff --git a/mlir/test/Dialect/Func/duplicate-function-elimination.mlir b/mlir/test/Dialect/Func/duplicate-function-elimination.mlir
index bc04e8fa9cd23..4d00d8a954d17 100644
--- a/mlir/test/Dialect/Func/duplicate-function-elimination.mlir
+++ b/mlir/test/Dialect/Func/duplicate-function-elimination.mlir
@@ -58,11 +58,10 @@ func.func @user(%arg0: f32, %arg1: f32) -> f32 {
// CHECK: @add_lr
// CHECK-NOT: @also_add_lr
-// CHECK: @add_rl
+// CHECK-NOT: @add_rl
// CHECK-NOT: @also_add_rl
// CHECK: @user
-// CHECK-2: call @add_lr
-// CHECK-2: call @add_rl
+// CHECK-4: call @add_lr
// -----
diff --git a/mlir/test/IR/operation-equality.mlir b/mlir/test/IR/operation-equality.mlir
index f382d7d0fbf1b..ac713ab66c077 100644
--- a/mlir/test/IR/operation-equality.mlir
+++ b/mlir/test/IR/operation-equality.mlir
@@ -184,3 +184,31 @@
%0:2 = "test.producer"() : () -> (i32, i32)
"test.consumer"(%0#1, %0#0) : (i32, i32) -> ()
}) : () -> ()
+
+// -----
+
+// CHECK-LABEL: test.commutatively_equal
+// CHECK-SAME: compares equals
+
+"test.commutatively_equal"() ({
+ ^bb0(%arg0 : i32, %arg1 : i32):
+ arith.addi %arg0, %arg1 : i32
+ }) : () -> ()
+"test.commutatively_equal"() ({
+ ^bb0(%arg0 : i32, %arg1 : i32):
+ arith.addi %arg1, %arg0 : i32
+ }) : () -> ()
+
+// -----
+
+// CHECK-LABEL: test.ignore_commutatively_equal
+// CHECK-SAME: compares NOT equals
+
+"test.ignore_commutatively_equal"() ({
+ ^bb0(%arg0 : i32, %arg1 : i32):
+ arith.addi %arg0, %arg1 : i32
+ }) { ignore_commutativity } : () -> ()
+"test.ignore_commutatively_equal"() ({
+ ^bb0(%arg0 : i32, %arg1 : i32):
+ arith.addi %arg1, %arg0 : i32
+ }) { ignore_commutativity } : () -> ()
\ No newline at end of file
diff --git a/mlir/test/lib/IR/TestOperationEquals.cpp b/mlir/test/lib/IR/TestOperationEquals.cpp
index 03cf5f4facf82..a8ff7752ebb2c 100644
--- a/mlir/test/lib/IR/TestOperationEquals.cpp
+++ b/mlir/test/lib/IR/TestOperationEquals.cpp
@@ -35,6 +35,8 @@ struct TestOperationEqualPass
OperationEquivalence::Flags flags{};
if (!first->hasAttr("strict_loc_check"))
flags |= OperationEquivalence::IgnoreLocations;
+ if (first->hasAttr("ignore_commutativity"))
+ flags |= OperationEquivalence::IgnoreCommutativity;
if (OperationEquivalence::isEquivalentTo(first, &module.getBody()->back(),
flags))
llvm::outs() << " compares equals.\n";
``````````
</details>
https://github.com/llvm/llvm-project/pull/181507
More information about the Mlir-commits
mailing list