[Mlir-commits] [mlir] [mlir]Add resultSegmentSizes/operandSegmentSizes to prop-dict (PR #212486)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Jul 28 06:24:40 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-core
Author: Natanael Cintean (natanael-cintean)
<details>
<summary>Changes</summary>
PR merged on main llvm.
Fixes https://github.com/llvm/llvm-project/issues/211220
setPropertiesFromParsedAttr (generated by OpFormatGen.cpp for ops using a
custom assemblyFormat) rejects the trait-injected operandSegmentSizes /
resultSegmentSizes properties when they appear in a prop-dict, even though
the printer emits them there for ops whose format uses a bulk operand/result
type directive (type(operands), type(results),
functional-type(operands, results)).
This breaks round-tripping: any op combining AttrSizedOperandSegments /
AttrSizedResultSegments with such a bulk directive fails to re-parse its own
printed output, because printer-side elision of these keys
(introduced in https://github.com/llvm/llvm-project/pull/115930) is intentionally skipped in that case — the sizes
can't be reconstructed from individually-typed operand/result groups, so they
must survive in the text, but the custom parser was never taught to read them
back.
setPropertiesFromAttr (used by the generic-form parser, bytecode, and C++
construction) already special-cases these two keys, so this change makes
setPropertiesFromParsedAttr mirror that behavior: keys are accepted from the
dictionary attr when the corresponding trait is present, and required exactly
when the format can't reconstruct the sizes itself (i.e. when
fmt.allOperands / fmt.allResultTypes is true). Otherwise the key remains
optional, since genParserVariadicSegmentResolution overwrites it with the
sizes inferred from the parsed operand/result groups.
The contents of the PR as well as description has been made using github-copilot. However I've reviewed the contents and tested locally the change.
For the PR description I've kept the AI suggestion as to me it seemed that the informations are needed to quickly understand the underlying issue.
---
Full diff: https://github.com/llvm/llvm-project/pull/212486.diff
5 Files Affected:
- (modified) mlir/test/IR/invalid-properties.mlir (+20)
- (modified) mlir/test/IR/properties.mlir (+11)
- (modified) mlir/test/lib/Dialect/Test/TestOps.td (+16)
- (modified) mlir/test/mlir-tblgen/op-format.td (+30)
- (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+40)
``````````diff
diff --git a/mlir/test/IR/invalid-properties.mlir b/mlir/test/IR/invalid-properties.mlir
index 2045db548c044..27c5adbf78e69 100644
--- a/mlir/test/IR/invalid-properties.mlir
+++ b/mlir/test/IR/invalid-properties.mlir
@@ -43,6 +43,26 @@ func.func @wrong_dense_i32_array_prop_type() {
// -----
+// `operandSegmentSizes` is inferable from the individually-spelled-out
+// variadic groups in this op's format, so the custom parser never reads this
+// key at all: presence of the key is rejected as an unrecognized key, just
+// like any other name the op doesn't know about.
+func.func @unknown_optional_operand_segment_sizes(%arg0: i64) {
+ // expected-error at +1 {{unknown key '"operandSegmentSizes"' when parsing properties dictionary}}
+ test.variadic_segment_prop %arg0, %arg0 : %arg0 : i64, i64 : i64 <{operandSegmentSizes = "bad"}> end
+ return
+}
+
+// -----
+
+func.func @unknown_optional_result_segment_sizes(%arg0: i64) {
+ // expected-error at +1 {{unknown key '"resultSegmentSizes"' when parsing properties dictionary}}
+ test.variadic_segment_prop %arg0, %arg0 : %arg0 : i64, i64 : i64 <{resultSegmentSizes = "bad"}> end
+ return
+}
+
+// -----
+
func.func @valid_all_properties() {
"test.with_properties"() <{a = 32 : i64, array = array<i64: 1, 2, 3, 4>, array32 = array<i32: 5, 6>, b = "foo", c = "bar", flag = true}> : () -> ()
return
diff --git a/mlir/test/IR/properties.mlir b/mlir/test/IR/properties.mlir
index 4dbb8ebfa5db3..4d83038f31cdd 100644
--- a/mlir/test/IR/properties.mlir
+++ b/mlir/test/IR/properties.mlir
@@ -46,6 +46,17 @@ test.using_property_ref_in_custom 1 + 4 = 5
%ci64 = arith.constant 0 : i64
test.variadic_segment_prop %ci64, %ci64 : %ci64 : i64, i64 : i64 end
+// Tests that the variadic segment size properties survive a round-trip
+// through the *custom* (non-generic) parser/printer when the assembly format
+// uses a bulk `functional-type(operands, results)` directive, which prevents
+// the printer from eliding `operandSegmentSizes` / `resultSegmentSizes` from
+// `<{...}>`. Without the parser-side fix, re-parsing the CHECK line below
+// (which is exactly what the printer emits) fails with "duplicate or unknown
+// 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>}>
+
// CHECK: test.with_default_valued_properties na{{$}}
// GENERIC: "test.with_default_valued_properties"()
// GENERIC-SAME: <{a = 0 : i32, b = "", c = -1 : i32, unit = false}> : () -> ()
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index c12e2abb3ab75..db55b90c294e1 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -3686,6 +3686,22 @@ def TestOpWithVariadicSegmentProperties : TEST_Op<"variadic_segment_prop",
}];
}
+// Same as `TestOpWithVariadicSegmentProperties`, but the assembly format uses
+// the bulk `type(operands)` / `type(results)` directives instead of spelling
+// out the type of each individual variadic group. This prevents the printer
+// from eliding `operandSegmentSizes` / `resultSegmentSizes` from the
+// `<{...}>` properties dictionary (since there is no other way to recover the
+// segment sizes on re-parse), so this op exercises the parser's ability to
+// read those trait-injected properties back out of `prop-dict`.
+def TestOpWithBulkTypesAndSegmentProperties : TEST_Op<"variadic_segment_prop_bulk_type",
+ [AttrSizedOperandSegments, AttrSizedResultSegments]> {
+ let arguments = (ins Variadic<I64>:$a1, Variadic<I64>:$a2);
+ let results = (outs Variadic<I64>:$b1, Variadic<I64>:$b2);
+ let assemblyFormat = [{
+ `(` operands `)` `:` functional-type(operands, results) prop-dict attr-dict
+ }];
+}
+
def TestOpUsingPropertyRefInCustom : TEST_Op<"using_property_ref_in_custom"> {
let assemblyFormat = "custom<IntProperty>($first) `+` custom<SumProperty>($second, ref($first)) attr-dict";
let arguments = (ins IntProp<"int64_t">:$first, IntProp<"int64_t">:$second);
diff --git a/mlir/test/mlir-tblgen/op-format.td b/mlir/test/mlir-tblgen/op-format.td
index 1790737a3a349..eea574888aee5 100644
--- a/mlir/test/mlir-tblgen/op-format.td
+++ b/mlir/test/mlir-tblgen/op-format.td
@@ -110,6 +110,36 @@ def OptionalGroupD : TestFormat_Op<[{
(custom<Custom>($a, $b)^)? attr-dict
}], [AttrSizedOperandSegments]>, Arguments<(ins Optional<I64>:$a, Optional<I64>:$b)>;
+//===----------------------------------------------------------------------===//
+// prop-dict + AttrSizedOperandSegments
+//===----------------------------------------------------------------------===//
+
+// When the format spells out each variadic operand group individually, the
+// segment sizes can be inferred from what was actually parsed, so the
+// `operandSegmentSizes` key is left completely untouched: it isn't read,
+// validated, or marked as a used key at all.
+// CHECK-LABEL: PropDictSegmentSizesInferred::setPropertiesFromParsedAttr
+// CHECK-NOT: operandSegmentSizes
+// CHECK: for (::mlir::NamedAttribute attr : dict) {
+def PropDictSegmentSizesInferred : TestFormat_Op<[{
+ $a1 `:` $a2 prop-dict attr-dict
+}], [AttrSizedOperandSegments]>,
+ Arguments<(ins Variadic<I64>:$a1, Variadic<I64>:$a2)>;
+
+// When the format uses a bulk `operands`/`type(operands)` directive, the
+// segment sizes can't be reconstructed from the parse, so the
+// `operandSegmentSizes` key is required and validated.
+// CHECK-LABEL: PropDictSegmentSizesRequired::setPropertiesFromParsedAttr
+// CHECK: auto operandSegmentSizesAttrName = ::mlir::StringAttr::get(ctx, "operandSegmentSizes");
+// CHECK-NEXT: usedKeys.insert(operandSegmentSizesAttrName);
+// CHECK-NEXT: auto attr = dict.get(operandSegmentSizesAttrName);
+// CHECK-NEXT: if (!attr) {
+// CHECK: if (::mlir::failed(::mlir::convertFromAttribute(prop.operandSegmentSizes, attr,
+def PropDictSegmentSizesRequired : TestFormat_Op<[{
+ `(` operands `)` `:` type(operands) prop-dict attr-dict
+}], [AttrSizedOperandSegments]>,
+ Arguments<(ins Variadic<I64>:$a1, Variadic<I64>:$a2)>;
+
// CHECK-LABEL: RegionRef::parse
// CHECK: auto odsResult = parseCustom(parser, *bodyRegion);
// CHECK-LABEL: RegionRef::print
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index cbcbc8e9bc102..4d5c31ae2ecae 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1313,6 +1313,46 @@ ::mlir::MLIRContext *ctx = dict.getContext();
(void)ctx;
)decl";
+ // `operandSegmentSizes`/`resultSegmentSizes` are trait-injected properties
+ // not enumerated by `op.getProperties()`, so they need to be special-cased
+ // here, mirroring `setPropertiesFromAttr` in OpDefinitionsGen.cpp. This is
+ // only necessary when the format can't infer the sizes itself (bulk
+ // `operands`/`type(results)` directives, or when there is no declarative
+ // assemblyFormat at all -- e.g. `hasCustomAssemblyFormat`, where this
+ // generated setter may still be reused by a hand-written parser that has
+ // no other way to recover the sizes); when the format spells out each
+ // variadic group individually, `genParserVariadicSegmentResolution` always
+ // overwrites the property from the parsed operand/result groups, so the key
+ // is left completely untouched here (same as any other property not
+ // handled by this format).
+ //
+ // {0}: segment sizes property name
+ const char *segmentSizesFromAttrFmt = R"decl(
+auto {0}AttrName = ::mlir::StringAttr::get(ctx, "{0}");
+usedKeys.insert({0}AttrName);
+auto attr = dict.get({0}AttrName);
+if (!attr) {{
+ emitError() << "expected key entry for {0} in DictionaryAttr to set "
+ "Properties.";
+ return ::mlir::failure();
+}
+if (::mlir::failed(::mlir::convertFromAttribute(prop.{0}, attr, [&]() {{
+ return emitError() << "for `{0}`: ";
+ })))
+ return ::mlir::failure();
+)decl";
+ bool hasNoDeclarativeFormat = !op.hasAssemblyFormat();
+ if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments") &&
+ (hasNoDeclarativeFormat || fmt.allOperands)) {
+ auto scope = body.scope("{\n", "}\n", /*indent=*/true);
+ body << formatv(segmentSizesFromAttrFmt, "operandSegmentSizes");
+ }
+ if (op.getTrait("::mlir::OpTrait::AttrSizedResultSegments") &&
+ (hasNoDeclarativeFormat || fmt.allResultTypes)) {
+ auto scope = body.scope("{\n", "}\n", /*indent=*/true);
+ body << formatv(segmentSizesFromAttrFmt, "resultSegmentSizes");
+ }
+
// {0}: fromAttribute call
// {1}: property name
// {2}: isRequired
``````````
</details>
https://github.com/llvm/llvm-project/pull/212486
More information about the Mlir-commits
mailing list