[Mlir-commits] [mlir] fd82478 - [mlir][IR] Combine location hash if required in OperationEquivalence::computeHash

[Hideto Ueno]

Author: Hideto Ueno
Date: 2023-06-28T18:48:45-07:00
New Revision: fd82478239a95e1e5a67a86f832d097fd83f5e1b

URL: https://github.com/llvm/llvm-project/commit/fd82478239a95e1e5a67a86f832d097fd83f5e1b
DIFF: https://github.com/llvm/llvm-project/commit/fd82478239a95e1e5a67a86f832d097fd83f5e1b.diff

LOG: [mlir][IR] Combine location hash if required in OperationEquivalence::computeHash

This fixes a bug that `OperationEquivalence::computeHash` doesn't
combine hash of operation locations even when `IgnoreLocations` is false.
Added a unit test which fails at the current trunk.

Reviewed By: mehdi_amini

Differential Revision: https://reviews.llvm.org/D154015

Added: 
    

Modified: 
    mlir/lib/IR/OperationSupport.cpp
    mlir/unittests/IR/OperationSupportTest.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index c353188d964e37..4c1eeda79ef922 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -656,6 +656,10 @@ llvm::hash_code OperationEquivalence::computeHash(
       llvm::hash_combine(op->getName(), op->getDiscardableAttrDictionary(),
                          op->getResultTypes(), op->hashProperties());
 
+  //   - Location if required
+  if(!(flags & Flags::IgnoreLocations))
+    hash = llvm::hash_combine(hash, op->getLoc());
+
   //   - Operands
   ValueRange operands = op->getOperands();
   SmallVector<Value> operandStorage;

diff  --git a/mlir/unittests/IR/OperationSupportTest.cpp b/mlir/unittests/IR/OperationSupportTest.cpp
index 81340d3bfdf709..e8ba6d07ac610c 100644
--- a/mlir/unittests/IR/OperationSupportTest.cpp
+++ b/mlir/unittests/IR/OperationSupportTest.cpp
@@ -290,4 +290,26 @@ TEST(OperandStorageTest, PopulateDefaultAttrs) {
 
   op->destroy();
 }
+
+TEST(OperationEquivalenceTest, HashWorksWithFlags) {
+  MLIRContext context;
+  context.getOrLoadDialect<test::TestDialect>();
+
+  auto op1 = createOp(&context);
+  // `op1` has an unknown loc.
+  auto op2 = createOp(&context);
+  op2->setLoc(NameLoc::get(StringAttr::get(&context, "foo")));
+  auto getHash = [](Operation *op, OperationEquivalence::Flags flags) {
+    return OperationEquivalence::computeHash(
+        op, OperationEquivalence::ignoreHashValue,
+        OperationEquivalence::ignoreHashValue, flags);
+  };
+  EXPECT_EQ(getHash(op1, OperationEquivalence::IgnoreLocations),
+            getHash(op2, OperationEquivalence::IgnoreLocations));
+  EXPECT_NE(getHash(op1, OperationEquivalence::None),
+            getHash(op2, OperationEquivalence::None));
+  op1->destroy();
+  op2->destroy();
+}
+
 } // namespace