[Mlir-commits] [mlir] [MLIR][ODS] Parse prop-dict fields with custom parsers (PR #217590)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Aug 20 03:52:49 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-core
Author: Mehdi Amini (joker-eph)
<details>
<summary>Changes</summary>
Teach generated operation parsers to accept a key-value spelling for prop-dict and dispatch known fields through their ODS parsers. Keep the DictionaryAttr spelling as a compatibility path and use attribute conversion when a property has no usable FieldParser.
See #<!-- -->155475
Assisted-by: Codex
---
Patch is 45.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/217590.diff
12 Files Affected:
- (modified) mlir/docs/DefiningDialects/Operations.md (+11-3)
- (modified) mlir/include/mlir/IR/DialectImplementation.h (+76-4)
- (modified) mlir/include/mlir/IR/OpDefinition.h (+18-4)
- (modified) mlir/include/mlir/TableGen/Property.h (+4)
- (modified) mlir/lib/TableGen/Property.cpp (+15)
- (added) mlir/test/IR/properties-invalid.mlir (+79)
- (modified) mlir/test/IR/properties.mlir (+94-1)
- (modified) mlir/test/lib/Dialect/Test/TestOps.h (+34)
- (modified) mlir/test/lib/Dialect/Test/TestOps.td (+264)
- (modified) mlir/test/mlir-tblgen/enums-gen.td (+15)
- (modified) mlir/tools/mlir-tblgen/EnumsGen.cpp (+7-1)
- (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+218)
``````````diff
diff --git a/mlir/docs/DefiningDialects/Operations.md b/mlir/docs/DefiningDialects/Operations.md
index 1ef96130d836d..419041a52b0da 100644
--- a/mlir/docs/DefiningDialects/Operations.md
+++ b/mlir/docs/DefiningDialects/Operations.md
@@ -760,9 +760,17 @@ The available directives are as follows:
* `prop-dict`
- - Represents the properties of the operation converted to a dictionary.
- - Any property or inherent attribute that are not used elsewhere in the
- format are parsed and printed as part of this dictionary.
+ - Represents the properties of the operation. The generated parser
+ accepts a `<key = value, ...>` list. Explicit property parsers and
+ inherent-attribute parsers must consume exactly one value and leave the
+ comma separating it from the next entry unconsumed. Properties relying
+ on the default parser use attribute conversion instead when no
+ `FieldParser` specialization is available or when the selected
+ specialization declares `isKeyValueCompositional = false`.
+ - The legacy `<{key = attribute, ...}>` dictionary spelling is also
+ accepted when parsing and is used by the generated printer.
+ - Any property or inherent attribute that is not used elsewhere in the
+ format is parsed and printed as part of this list.
- If present, the `attr-dict` will not contain any inherent attributes.
* `custom < UserDirective > ( Params )`
diff --git a/mlir/include/mlir/IR/DialectImplementation.h b/mlir/include/mlir/IR/DialectImplementation.h
index 0b4f91cd750b8..2652114b508e3 100644
--- a/mlir/include/mlir/IR/DialectImplementation.h
+++ b/mlir/include/mlir/IR/DialectImplementation.h
@@ -70,6 +70,15 @@ class DialectAsmParser : public AsmParser {
/// Provide a template class that can be specialized by users to dispatch to
/// parsers. Auto-generated parsers generate calls to `FieldParser<T>::parse`,
/// where `T` is the parameter storage type, to parse custom types.
+///
+/// A parser is key-value compositional only if it consumes exactly one value
+/// and leaves the comma separating the next key unconsumed. For example, an
+/// undelimited array parser for `values = 1, 2, next = 9` cannot distinguish
+/// its element commas from the comma before `next` and may try to parse `next`
+/// as another element. Marking it non-compositional lets a keyed property list
+/// use a self-delimiting attribute such as `array<i64: 1, 2>` instead.
+/// Specializations with this behavior, or that may succeed without consuming a
+/// token, should define `isKeyValueCompositional` as false.
template <typename T, typename = T>
struct FieldParser;
@@ -131,6 +140,8 @@ struct FieldParser<
std::optional<AttributeT>,
std::enable_if_t<std::is_base_of<Attribute, AttributeT>::value,
std::optional<AttributeT>>> {
+ static constexpr bool isKeyValueCompositional = false;
+
static FailureOr<std::optional<AttributeT>> parse(AsmParser &parser) {
if constexpr (HasStaticDialectName<AttributeT>::value) {
parser.getContext()->getOrLoadDialect(AttributeT::dialectName);
@@ -151,6 +162,8 @@ template <typename IntT>
struct FieldParser<
std::optional<IntT>,
std::enable_if_t<std::is_integral<IntT>::value, std::optional<IntT>>> {
+ static constexpr bool isKeyValueCompositional = false;
+
static FailureOr<std::optional<IntT>> parse(AsmParser &parser) {
IntT value;
OptionalParseResult result = parser.parseOptionalInteger(value);
@@ -167,14 +180,51 @@ namespace detail {
template <typename T>
using has_push_back_t = decltype(std::declval<T>().push_back(
std::declval<typename T::value_type &&>()));
+
+template <typename StorageType, typename = void>
+struct HasFieldParser : std::false_type {};
+
+template <typename StorageType>
+struct HasFieldParser<StorageType,
+ std::void_t<decltype(sizeof(FieldParser<StorageType>)),
+ decltype(FieldParser<StorageType>::parse(
+ std::declval<OpAsmParser &>()))>>
+ : std::true_type {};
+
+template <typename ContainerT, typename = void>
+struct HasFieldParserContainer : std::false_type {};
+
+template <typename ContainerT>
+struct HasFieldParserContainer<ContainerT,
+ std::void_t<has_push_back_t<ContainerT>>>
+ : HasFieldParser<typename ContainerT::value_type> {};
+
+template <typename Parser, typename = void>
+struct IsKeyValueCompositional : std::true_type {};
+
+template <typename Parser>
+struct IsKeyValueCompositional<
+ Parser, std::void_t<decltype(Parser::isKeyValueCompositional)>>
+ : std::bool_constant<Parser::isKeyValueCompositional> {};
+
+/// Whether the selected FieldParser consumes exactly one value in a keyed
+/// property list. Parser specializations may set isKeyValueCompositional to
+/// false if they can succeed without consuming a token or consume an
+/// undelimited comma-separated list.
+template <typename StorageType>
+struct HasKeyValueFieldParser
+ : std::conjunction<HasFieldParser<StorageType>,
+ IsKeyValueCompositional<FieldParser<StorageType>>> {};
} // namespace detail
/// Parse any container that supports back insertion as a list.
template <typename ContainerT>
-struct FieldParser<ContainerT,
- std::enable_if_t<llvm::is_detected<detail::has_push_back_t,
- ContainerT>::value,
- ContainerT>> {
+struct FieldParser<
+ ContainerT,
+ std::enable_if_t<detail::HasFieldParserContainer<ContainerT>::value,
+ ContainerT>> {
+ static constexpr bool isKeyValueCompositional = false;
+
using ElementT = typename ContainerT::value_type;
static FailureOr<ContainerT> parse(AsmParser &parser) {
ContainerT elements;
@@ -202,6 +252,28 @@ struct FieldParser<AffineMap> {
}
};
+namespace detail {
+/// Parse a property with its FieldParser when one is available, otherwise
+/// fall back to the property's attribute conversion.
+template <typename StorageType, typename ConvertFromAttribute>
+ParseResult
+parsePropertyWithFallback(OpAsmParser &parser, StorageType &storage,
+ ConvertFromAttribute convertFromAttribute) {
+ if constexpr (HasKeyValueFieldParser<StorageType>::value) {
+ auto value = FieldParser<StorageType>::parse(parser);
+ if (failed(value))
+ return failure();
+ storage = std::move(*value);
+ return success();
+ } else {
+ Attribute attr;
+ if (parser.parseAttribute(attr))
+ return failure();
+ return convertFromAttribute(storage, attr);
+ }
+}
+} // namespace detail
+
} // namespace mlir
#endif // MLIR_IR_DIALECTIMPLEMENTATION_H
diff --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index fe2fa0a0ccd23..075d39194ed97 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -1872,6 +1872,16 @@ class Op : public OpState, public Traits<ConcreteType>... {
using detect_has_parse_properties =
llvm::is_detected<has_parse_properties, T>;
+ /// Trait to check if T provides a generated parser for the key-value
+ /// spelling of `prop-dict`.
+ template <typename T, typename... Args>
+ using has_parse_properties_from_key_value_list =
+ decltype(T::parsePropertiesFromKeyValueList(
+ std::declval<OpAsmParser &>(), std::declval<OperationState &>()));
+ template <typename T>
+ using detect_has_parse_properties_from_key_value_list =
+ llvm::is_detected<has_parse_properties_from_key_value_list, T>;
+
/// Trait to check if T provides a 'ConcreteEntity' type alias.
template <typename T>
using has_concrete_entity_t = typename T::ConcreteEntity;
@@ -2039,10 +2049,11 @@ class Op : public OpState, public Traits<ConcreteType>... {
p, ConcreteType::getPropertiesAsAttr(ctx, properties), elidedProps);
}
- /// Parses 'prop-dict' for the operation. Unless overridden, the method will
- /// parse the properties using the generic property dictionary using the
- /// '<{ ... }>' syntax. The resulting properties are stored within the
- /// property structure of 'result', accessible via 'getOrAddProperties'.
+ /// Parses 'prop-dict' for the operation. Generated parsers accept a keyed
+ /// list whose values use their custom assembly parsers, as well as the
+ /// legacy generic '<{ ... }>' dictionary syntax. The resulting properties
+ /// are stored within the property structure of 'result', accessible via
+ /// 'getOrAddProperties'.
template <typename T = ConcreteType>
static ParseResult parseProperties(OpAsmParser &parser,
OperationState &result) {
@@ -2051,6 +2062,9 @@ class Op : public OpState, public Traits<ConcreteType>... {
parser, result.getOrAddProperties<InferredProperties<T>>());
}
+ if constexpr (detect_has_parse_properties_from_key_value_list<T>::value)
+ return T::parsePropertiesFromKeyValueList(parser, result);
+
Attribute propertyDictionary;
if (genericParseProperties(parser, propertyDictionary))
return failure();
diff --git a/mlir/include/mlir/TableGen/Property.h b/mlir/include/mlir/TableGen/Property.h
index 81e6d85720829..5877b54147f99 100644
--- a/mlir/include/mlir/TableGen/Property.h
+++ b/mlir/include/mlir/TableGen/Property.h
@@ -94,6 +94,10 @@ class Property : public PropConstraint {
// Returns the method call which parses this property from textual MLIR.
StringRef getParserCall() const { return parserCall; }
+ // Returns true if this property uses the parser inherited from the base
+ // Property class.
+ bool usesDefaultParser() const;
+
// Returns true if this property has defined an optional parser.
bool hasOptionalParser() const { return !optionalParserCall.empty(); }
diff --git a/mlir/lib/TableGen/Property.cpp b/mlir/lib/TableGen/Property.cpp
index b003d74c7bdee..88a64891b8b86 100644
--- a/mlir/lib/TableGen/Property.cpp
+++ b/mlir/lib/TableGen/Property.cpp
@@ -104,6 +104,21 @@ Pred Property::getPredicate() const {
return Pred(maybePred->getValue());
}
+bool Property::usesDefaultParser() const {
+ const Record *propertyClass = def->getRecords().getClass("Property");
+ if (const auto *baseInit =
+ llvm::dyn_cast<DefInit>(def->getValueInit("baseProperty"))) {
+ Property baseProperty(baseInit);
+ if (getParserCall() == baseProperty.getParserCall())
+ return baseProperty.usesDefaultParser();
+ }
+ // RecordVal retains the source location of the initializer that supplied a
+ // field. An inherited parser therefore points at Property::parser, while an
+ // explicit `let parser` points at the override without inspecting its text.
+ return def->getValue("parser")->getLoc().getPointer() ==
+ propertyClass->getValue("parser")->getLoc().getPointer();
+}
+
Property Property::getBaseProperty() const {
if (const auto *defInit =
llvm::dyn_cast<llvm::DefInit>(def->getValueInit("baseProperty"))) {
diff --git a/mlir/test/IR/properties-invalid.mlir b/mlir/test/IR/properties-invalid.mlir
new file mode 100644
index 0000000000000..a6adf7ed98f8a
--- /dev/null
+++ b/mlir/test/IR/properties-invalid.mlir
@@ -0,0 +1,79 @@
+// RUN: mlir-opt %s -split-input-file -verify-diagnostics
+
+// expected-error @below {{properties dictionary is missing required property: prop}}
+test.with_custom_prop_dict <attr = 1>
+
+// -----
+
+// expected-error @below {{properties dictionary is missing required attribute: attr}}
+test.with_custom_prop_dict <prop = 2>
+
+// -----
+
+// expected-error @below {{duplicate or unknown property in properties dictionary: prop}}
+test.with_custom_prop_dict <attr = 1, prop = 2, prop = 3>
+
+// -----
+
+// expected-error @below {{duplicate or unknown property in properties dictionary: unknown}}
+test.with_custom_prop_dict <attr = 1, prop = 2, unknown = 3>
+
+// -----
+
+// A required property dictionary cannot be omitted entirely.
+// expected-error @below {{properties dictionary is missing required property: prop}}
+test.with_custom_prop_dict
+
+// -----
+
+// expected-error @below {{expected integer value}}
+test.with_custom_prop_dict <attr = 1, prop = bad>
+
+// -----
+
+// expected-error @below {{invalid value for property prop}}
+test.with_wrapped_properties <prop = 1 : i64>
+
+// -----
+
+// A required keyed value may not succeed without consuming a token.
+// expected-error @below {{expected attribute value}}
+test.with_key_value_parser_boundaries <values = array<i64: 1>, maybe = >
+
+// -----
+
+%c0 = arith.constant 0 : i64
+// A segment-size property inferred later in the parser must not be accepted
+// and then silently overwritten.
+// expected-error @below {{unknown property in properties dictionary: operandSegmentSizes}}
+test.variadic_segment_prop %c0 : %c0 : i64 : i64 <operandSegmentSizes = [1, 1]> end
+
+// -----
+
+%c0 = arith.constant 0 : i64
+// expected-error @below {{unknown property in properties dictionary: resultSegmentSizes}}
+test.variadic_segment_prop %c0 : %c0 : i64 : i64 <resultSegmentSizes = [1, 1]> end
+
+// -----
+
+%c0 = arith.constant 0 : i64
+// expected-error @below {{properties dictionary is missing required property: operandSegmentSizes}}
+test.variadic_segment_prop_bulk_type(%c0, %c0, %c0) : (i64, i64, i64) -> (i64, i64, i64) <resultSegmentSizes = [2, 1]>
+
+// -----
+
+%c0 = arith.constant 0 : i64
+// expected-error @below {{properties dictionary is missing required property: resultSegmentSizes}}
+test.variadic_segment_prop_bulk_type(%c0, %c0, %c0) : (i64, i64, i64) -> (i64, i64, i64) <operandSegmentSizes = [2, 1]>
+
+// -----
+
+%c0 = arith.constant 0 : i64
+// expected-error @below {{expected 2 entries for operandSegmentSizes}}
+test.variadic_segment_prop_bulk_type(%c0, %c0, %c0) : (i64, i64, i64) -> (i64, i64, i64) <operandSegmentSizes = [3], resultSegmentSizes = [2, 1]>
+
+// -----
+
+%c0 = arith.constant 0 : i64
+// expected-error @below {{expected 2 entries for resultSegmentSizes}}
+test.variadic_segment_prop_bulk_type(%c0, %c0, %c0) : (i64, i64, i64) -> (i64, i64, i64) <operandSegmentSizes = [2, 1], resultSegmentSizes = [3]>
diff --git a/mlir/test/IR/properties.mlir b/mlir/test/IR/properties.mlir
index 4d83038f31cdd..64548e41dc111 100644
--- a/mlir/test/IR/properties.mlir
+++ b/mlir/test/IR/properties.mlir
@@ -23,6 +23,85 @@ test.with_wrapped_properties <{prop = "content for properties"}>
// GENERIC: "test.empty_properties"()
test.empty_properties
+// An explicitly empty key-value list is also accepted.
+// CHECK: test.empty_properties
+// GENERIC: "test.empty_properties"()
+test.empty_properties <>
+
+// The key-value spelling uses the custom parsers for both attributes and
+// properties. Until the custom printer is enabled, it round-trips to the
+// generic DictionaryAttr spelling.
+// CHECK: test.with_custom_prop_dict <{attr = 1 : i32, prop = 2 : i64}>
+// GENERIC: "test.with_custom_prop_dict"()
+// GENERIC-SAME: <{attr = 1 : i32, defaulted = 42 : i64, prop = 2 : i64, unit = false}>
+test.with_custom_prop_dict <attr = 1, prop = 2>
+
+// The generic DictionaryAttr spelling remains accepted for compatibility.
+// CHECK: test.with_custom_prop_dict <{attr = 3 : i32, prop = 4 : i64}>
+// GENERIC: "test.with_custom_prop_dict"()
+// GENERIC-SAME: <{attr = 3 : i32, defaulted = 42 : i64, prop = 4 : i64, unit = false}>
+test.with_custom_prop_dict <{attr = 3 : i32, prop = 4 : i64}>
+
+// Entries are order-independent, and optional/default-valued entries use
+// their custom parsers when present.
+// CHECK: test.with_custom_prop_dict <{attr = 5 : i32, defaulted = 43 : i64, optional = "set", prop = 6 : i64}>
+// GENERIC: "test.with_custom_prop_dict"()
+// GENERIC-SAME: <{attr = 5 : i32, defaulted = 43 : i64, optional = "set", prop = 6 : i64, unit = false}>
+test.with_custom_prop_dict <optional = "set", defaulted = 43, prop = 6, attr = 5>
+
+// A field name that is also the start of an attribute must not be consumed by
+// the legacy DictionaryAttr compatibility probe.
+// CHECK: test.with_custom_prop_dict <{attr = 7 : i32, prop = 8 : i64, unit}>
+// GENERIC: "test.with_custom_prop_dict"()
+// GENERIC-SAME: <{attr = 7 : i32, defaulted = 42 : i64, prop = 8 : i64, unit}>
+test.with_custom_prop_dict <unit = unit, attr = 7, prop = 8>
+
+// Properties bound elsewhere in the assembly format are excluded from the
+// key-value list.
+// CHECK: test.with_properties_and_attr 7 <{rhs = 8 : i64}>
+// GENERIC: "test.with_properties_and_attr"()
+// GENERIC-SAME: <{lhs = 7 : i32, rhs = 8 : i64}>
+test.with_properties_and_attr 7 <rhs = 8>
+
+// A property without a usable custom parser falls back to its attribute
+// conversion for this compatibility spelling.
+// CHECK: test.with_wrapped_properties <{prop = "custom spelling"}>
+// GENERIC: "test.with_wrapped_properties"()
+// GENERIC-SAME: <{prop = "custom spelling"}>
+test.with_wrapped_properties <prop = "custom spelling">
+
+// Forwarding property wrappers preserve whether their base uses the default
+// FieldParser, so a wrapped custom storage type still uses attribute fallback.
+// CHECK: test.with_default_wrapped_properties
+// GENERIC: "test.with_default_wrapped_properties"()
+// GENERIC-SAME: <{prop = "wrapped default spelling"}>
+test.with_default_wrapped_properties <prop = "wrapped default spelling">
+
+// A container FieldParser is unavailable when its element parser is
+// unavailable, so the complete property also falls back to conversion.
+// CHECK: test.with_wrapped_array_properties
+// GENERIC: "test.with_wrapped_array_properties"()
+// GENERIC-SAME: <{prop = ["first", "second"]}>
+test.with_wrapped_array_properties <prop = ["first", "second"]>
+
+// Default optional and container FieldParsers do not delimit exactly one
+// property value, so they use attribute conversion in a key-value list. The
+// following scalar key also checks that the container does not consume the
+// outer comma.
+// CHECK: test.with_key_value_parser_boundaries
+// CHECK-SAME: <{maybe = [], maybeEnum = [], next = 9 : i64, specializedMaybe = [7 : i16], specializedValues = array<i32: 3, 4>, values = array<i64: 1, 2>}>
+// GENERIC: "test.with_key_value_parser_boundaries"()
+// GENERIC-SAME: <{maybe = [], maybeEnum = [], next = 9 : i64, specializedMaybe = [7 : i16], specializedValues = array<i32: 3, 4>, values = array<i64: 1, 2>}>
+test.with_key_value_parser_boundaries <specializedValues = [3, 4], specializedMaybe = some<7>, values = array<i64: 1, 2>, maybe = [], maybeEnum = [], next = 9>
+
+// A comma-separated bit-enum FieldParser is not compositional with the outer
+// list, so prop-dict uses its attribute conversion before parsing another key.
+// CHECK: test.op_with_bit_enum_prop_dict
+// CHECK-SAME: <{flags = 3 : i32, next = 9 : i64}>
+// GENERIC: "test.op_with_bit_enum_prop_dict"()
+// GENERIC-SAME: <{flags = 3 : i32, next = 9 : i64}>
+test.op_with_bit_enum_prop_dict <flags = 3 : i32, next = 9>
+
// CHECK: test.using_property_in_custom
// CHECK-SAME: [1, 4, 20]{{$}}
// GENERIC: "test.using_property_in_custom"()
@@ -55,7 +134,7 @@ test.variadic_segment_prop %ci64, %ci64 : %ci64 : i64, i64 : i64 end
// key 'operandSegmentSizes' in dictionary attribute".
// CHECK: test.variadic_segment_prop_bulk_type(%[[CI64]], %[[CI64]], %[[CI64]]) : (i64, i64, i64) -> (i64, i64, i64) <{operandSegmentSizes = array<i32: 2, 1>, resultSegmentSizes = array<i32: 2, 1>}>
// GENERIC: "test.variadic_segment_prop_bulk_type"(%[[CI64]], %[[CI64]], %[[CI64]]) <{operandSegmentSizes = array<i32: 2, 1>, resultSegmentSizes = array<i32: 2, 1>}> : (i64, i64, i64) -> (i64, i64, i64)
-test.variadic_segment_prop_bulk_type(%ci64, %ci64, %ci64) : (i64, i64, i64) -> (i64, i64, i64) <{operandSegmentSizes = array<i32: 2, 1>, resultSegmentSizes = array<i32: 2, 1>}>
+test.variadic_segment_prop_bulk_type(%ci64, %ci64, %ci64) : (i64, i64, i64) -> (i64, i64, i64) <operandSegmentSizes = [2, 1], resultSegmentSizes = [2, 1]>
// CHECK: test.with_default_valued_properties na{{$}}
// GENERIC: "test.with_default_valued_properties"()
@@ -121,6 +200,20 @@ test.op_with_property_predicates <{
non_empty_constrained = [1],
unconstrained = 0 : i64}>
+// Keyed parsing composes optional and aggregate property parsers with a
+// following outer dictionary entry.
+// CHECK: test.op_with_property_predicates
+// CHECK-SAME: array ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/217590
More information about the Mlir-commits
mailing list