[MLIR][ODS] Fix properties tablegen wrapper to allow ADL lookup for hash_value #141023
+23
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ash_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.
|
@llvm/pr-subscribers-mlir-ods @llvm/pr-subscribers-mlir Author: Mehdi Amini (joker-eph) Changesllvm::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. Full diff: https://github.com/llvm/llvm-project/pull/141023.diff 4 Files Affected:
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;
}
|
|
@llvm/pr-subscribers-mlir-core Author: Mehdi Amini (joker-eph) Changesllvm::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. Full diff: https://github.com/llvm/llvm-project/pull/141023.diff 4 Files Affected:
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;
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.