[Mlir-commits] [mlir] [MLIR][ODS] Fix properties tablegen wrapper to allow ADL lookup for hash_value (PR #141023)

[Mehdi Amini]

https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/141023

llvm::hash_value() is meant to be redefined in the relevant namespace and looked up through ADL. However this can't work with a fully qualified call.

>From 3b305a04ac9d5b9e7ba5dd76453e4fc06910a65c Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Thu, 22 May 2025 01:36:21 -0700
Subject: [PATCH] [MLIR][ODS] Fix properties tablegen wrapper to allow ADL
 lookup for hash_value

llvm::hash_value() is meant to be redefined in the relevant namespace and
looked up through ADL. However this can't work with a fully qualified call.
---
 mlir/include/mlir/IR/Properties.td          |  2 +-
 mlir/test/lib/Dialect/Test/TestOps.h        |  5 ++++-
 mlir/test/lib/Dialect/Test/TestOps.td       | 14 ++++++++++++++
 mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp |  6 ++++--
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td
index 15b72969dc92f..dc4a5c6f2ace9 100644
--- a/mlir/include/mlir/IR/Properties.td
+++ b/mlir/include/mlir/IR/Properties.td
@@ -762,7 +762,7 @@ class OptionalProp<Property p, bit canDelegateParsing = 1>
   }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode);
 
   let hashProperty = !if(!empty(p.hashProperty), p.hashProperty,
-    [{ ::llvm::hash_value($_storage.has_value() ? std::optional<::llvm::hash_code>{}] #
+    [{ hash_value($_storage.has_value() ? std::optional<::llvm::hash_code>{}] #
       !subst("$_storage", "(*($_storage))", p.hashProperty) #[{} : std::nullopt) }]);
   assert !or(!not(delegatesParsing), !eq(defaultValue, "std::nullopt")),
     "For delegated parsing to be used, the default value must be nullopt. " #
diff --git a/mlir/test/lib/Dialect/Test/TestOps.h b/mlir/test/lib/Dialect/Test/TestOps.h
index 9d5b225b219cd..c2ee5f9ab9a57 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.h
+++ b/mlir/test/lib/Dialect/Test/TestOps.h
@@ -86,7 +86,7 @@ mlir::ParseResult customParseProperties(mlir::OpAsmParser &parser,
 //===----------------------------------------------------------------------===//
 // MyPropStruct
 //===----------------------------------------------------------------------===//
-
+namespace test_properties {
 class MyPropStruct {
 public:
   std::string content;
@@ -101,6 +101,9 @@ class MyPropStruct {
     return content == rhs.content;
   }
 };
+inline llvm::hash_code hash_value(const MyPropStruct &S) { return S.hash(); }
+} // namespace test_properties
+using test_properties::MyPropStruct;
 
 llvm::LogicalResult readFromMlirBytecode(mlir::DialectBytecodeReader &reader,
                                          MyPropStruct &prop);
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index cdc0f393b4761..6955775264a48 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -3189,6 +3189,20 @@ def TestOpWithWrappedProperties : TEST_Op<"with_wrapped_properties"> {
   );
 }
 
+// Same as above, but without a custom `hashProperty` field, checking
+// that ADL is correctly working.
+def MyStructProperty2 : Property<"MyPropStruct"> {
+  let convertToAttribute = "return $_storage.asAttribute($_ctxt);";
+  let convertFromAttribute = "return MyPropStruct::setFromAttr($_storage, $_attr, $_diag);";
+}
+
+def TestOpWithWrappedProperties2 : TEST_Op<"with_wrapped_properties2"> {
+  let assemblyFormat = "prop-dict attr-dict";
+  let arguments = (ins
+    MyStructProperty2:$prop
+  );
+}
+
 def TestOpWithEmptyProperties : TEST_Op<"empty_properties"> {
   let assemblyFormat = "prop-dict attr-dict";
   let arguments = (ins);
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 373d3762cbb1a..06247b390160f 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -1588,6 +1588,7 @@ void OpEmitter::genPropertiesSupport() {
 
   const char *propHashFmt = R"decl(
   auto hash_{0} = [] (const auto &propStorage) -> llvm::hash_code {
+    using ::llvm::hash_value;
     return {1};
   };
 )decl";
@@ -1605,6 +1606,7 @@ void OpEmitter::genPropertiesSupport() {
       }
     }
   }
+  hashMethod << "  using llvm::hash_value;\n";
   hashMethod << "  return llvm::hash_combine(";
   llvm::interleaveComma(
       attrOrProperties, hashMethod, [&](const ConstArgument &attrOrProp) {
@@ -1614,8 +1616,8 @@ void OpEmitter::genPropertiesSupport() {
             hashMethod << "\n    hash_" << namedProperty->name << "(prop."
                        << namedProperty->name << ")";
           } else {
-            hashMethod << "\n    ::llvm::hash_value(prop."
-                       << namedProperty->name << ")";
+            hashMethod << "\n    hash_value(prop." << namedProperty->name
+                       << ")";
           }
           return;
         }