[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:06 PST 2026
https://github.com/jumerckx created https://github.com/llvm/llvm-project/pull/181507
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?
>From 1b39f5788bac7fa74aff16c90ce6dd32c27249a4 Mon Sep 17 00:00:00 2001
From: jumerckx <31353884+jumerckx at users.noreply.github.com>
Date: Sat, 14 Feb 2026 17:22:12 +0100
Subject: [PATCH 1/7] fix docstring
---
mlir/include/mlir/IR/OperationSupport.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 1ff7c56ddca38..a05e0ae387164 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -1348,8 +1348,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
>From 081f22bd31f902958d404230c488a7ca684e6ada Mon Sep 17 00:00:00 2001
From: jumerckx <31353884+jumerckx at users.noreply.github.com>
Date: Sat, 14 Feb 2026 17:22:41 +0100
Subject: [PATCH 2/7] IgnoreCommutativity flag
---
mlir/include/mlir/IR/OperationSupport.h | 6 +++++-
mlir/lib/IR/OperationSupport.cpp | 6 ++++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index a05e0ae387164..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.
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 2a37f3860fe00..8e113377ba355 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())
@@ -854,7 +855,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();
>From 7e9346e97e227a70d9480dd18a10d7f12a431cdb Mon Sep 17 00:00:00 2001
From: jumerckx <31353884+jumerckx at users.noreply.github.com>
Date: Sat, 14 Feb 2026 17:22:51 +0100
Subject: [PATCH 3/7] test IgnoreCommutativity
---
mlir/test/IR/operation-equality.mlir | 28 ++++++++++++++++++++++++
mlir/test/lib/IR/TestOperationEquals.cpp | 2 ++
2 files changed, 30 insertions(+)
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";
>From fed581a0f0dfe38ab27927725a1b35bd99754ef5 Mon Sep 17 00:00:00 2001
From: jumerckx <julesmerckx12 at gmail.com>
Date: Sat, 14 Feb 2026 20:07:11 +0100
Subject: [PATCH 4/7] propagate checkCommutativeEquivalent
---
mlir/lib/IR/OperationSupport.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 8e113377ba355..e060c5ed3ba3f 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -890,7 +890,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;
>From f0769d19963d0433e0e3b5669b781183b8525546 Mon Sep 17 00:00:00 2001
From: jumerckx <julesmerckx12 at gmail.com>
Date: Sat, 14 Feb 2026 20:07:24 +0100
Subject: [PATCH 5/7] fully check commutative equivalence
---
mlir/lib/IR/OperationSupport.cpp | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index e060c5ed3ba3f..adcf366885dfb 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -797,7 +797,16 @@ struct ValueEquivalenceCache {
};
auto lhsSorted = sortValues({lhsIt, lhsRange.end()});
auto rhsSorted = sortValues({rhsIt, rhsRange.end()});
- return success(lhsSorted == rhsSorted);
+ if (lhsSorted == rhsSorted) {
+ return success();
+ }
+ for (auto operandPair : llvm::zip(lhsSorted, rhsSorted)) {
+ Value lhs = std::get<0>(operandPair);
+ Value rhs = std::get<1>(operandPair);
+ if (failed(checkEquivalent(lhs, rhs)))
+ return failure();
+ }
+ return success();
}
void markEquivalent(Value lhsResult, Value rhsResult) {
auto insertion = equivalentValues.insert({lhsResult, rhsResult});
>From 7f669898628fdaa949c07efc0d49bb63cafb7e8a Mon Sep 17 00:00:00 2001
From: jumerckx <julesmerckx12 at gmail.com>
Date: Sat, 14 Feb 2026 20:33:25 +0100
Subject: [PATCH 6/7] update test
---
mlir/test/Dialect/Func/duplicate-function-elimination.mlir | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
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
// -----
>From d559872ca095240179918cd655e5ad7f0fb67b3c Mon Sep 17 00:00:00 2001
From: jumerckx <julesmerckx12 at gmail.com>
Date: Sat, 14 Feb 2026 21:43:28 +0100
Subject: [PATCH 7/7] check full commutativity by using equivalentValues as a
union-find
---
mlir/lib/IR/OperationSupport.cpp | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index adcf366885dfb..a7a67744b65d5 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -785,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();
});
@@ -800,13 +801,7 @@ struct ValueEquivalenceCache {
if (lhsSorted == rhsSorted) {
return success();
}
- for (auto operandPair : llvm::zip(lhsSorted, rhsSorted)) {
- Value lhs = std::get<0>(operandPair);
- Value rhs = std::get<1>(operandPair);
- if (failed(checkEquivalent(lhs, rhs)))
- return failure();
- }
- return success();
+ return failure();
}
void markEquivalent(Value lhsResult, Value rhsResult) {
auto insertion = equivalentValues.insert({lhsResult, rhsResult});
More information about the Mlir-commits
mailing list