[Mlir-commits] [mlir] e432cb1 - [mlir]Add resultSegmentSizes/operandSegmentSizes to prop-dict. (#211222)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Jul 28 06:11:39 PDT 2026
Author: Natanael Cintean
Date: 2026-07-28T13:11:34Z
New Revision: e432cb12962f1619bc73c50a2dc4d2ad5ac6b44c
URL: https://github.com/llvm/llvm-project/commit/e432cb12962f1619bc73c50a2dc4d2ad5ac6b44c
DIFF: https://github.com/llvm/llvm-project/commit/e432cb12962f1619bc73c50a2dc4d2ad5ac6b44c.diff
LOG: [mlir]Add resultSegmentSizes/operandSegmentSizes to prop-dict. (#211222)
`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 #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.
Fixes #211220
Added:
Modified:
mlir/test/IR/invalid-properties.mlir
mlir/test/IR/properties.mlir
mlir/test/lib/Dialect/Test/TestOps.td
mlir/test/mlir-tblgen/op-format.td
mlir/tools/mlir-tblgen/OpFormatGen.cpp
Removed:
################################################################################
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
More information about the Mlir-commits
mailing list