[Mlir-commits] [mlir] [mlir]Add resultSegmentSizes/operandSegmentSizes to prop-dict. (PR #211222)
Natanael Cintean
llvmlistbot at llvm.org
Tue Jul 28 05:46:48 PDT 2026
https://github.com/natanael-cintean updated https://github.com/llvm/llvm-project/pull/211222
>From 41835df006b9b5146dda44d7e2824b5e0a2baaa9 Mon Sep 17 00:00:00 2001
From: Natanael Cintean <natanael.cintean at intel.com>
Date: Tue, 21 Jul 2026 14:56:22 +0000
Subject: [PATCH 1/4] Add resultSegmentSizes/operandSegmentSizes to prop-dict.
Add test to check for these in prop-dict.
---
mlir/test/IR/properties.mlir | 11 +++++++++
mlir/test/lib/Dialect/Test/TestOps.td | 16 +++++++++++++
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 32 ++++++++++++++++++++++++++
3 files changed, 59 insertions(+)
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/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index cbcbc8e9bc102..f9d07f6b3ba0f 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1313,6 +1313,38 @@ ::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 the handling in `setPropertiesFromAttr` in
+ // OpDefinitionsGen.cpp.
+ //
+ // {0}: segment sizes property name
+ // {1}: isRequired
+ 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 && {1}) {{
+ emitError() << "expected key entry for {0} in DictionaryAttr to set "
+ "Properties.";
+ return ::mlir::failure();
+}
+if (attr && ::mlir::failed(::mlir::convertFromAttribute(prop.{0}, attr, [&]() {{
+ return emitError() << "for `{0}`: ";
+ })))
+ return ::mlir::failure();
+)decl";
+ if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments")) {
+ auto scope = body.scope("{\n", "}\n", /*indent=*/true);
+ body << formatv(segmentSizesFromAttrFmt, "operandSegmentSizes",
+ fmt.allOperands);
+ }
+ if (op.getTrait("::mlir::OpTrait::AttrSizedResultSegments")) {
+ auto scope = body.scope("{\n", "}\n", /*indent=*/true);
+ body << formatv(segmentSizesFromAttrFmt, "resultSegmentSizes",
+ fmt.allResultTypes);
+ }
+
// {0}: fromAttribute call
// {1}: property name
// {2}: isRequired
>From 446038c810667481033bb27862af18d61141d6be Mon Sep 17 00:00:00 2001
From: Natanael Cintean <natanael.cintean at intel.com>
Date: Tue, 28 Jul 2026 11:07:29 +0000
Subject: [PATCH 2/4] Add tests for invalid operandSegmentSizes as well as
required and infered cases.
---
mlir/test/IR/invalid-properties.mlir | 20 +++++++++++++++++
mlir/test/mlir-tblgen/op-format.td | 32 ++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/mlir/test/IR/invalid-properties.mlir b/mlir/test/IR/invalid-properties.mlir
index 2045db548c044..3678c98db43d1 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 not required in `<{...}>` for this op, since its
+// format spells out each variadic group individually
+// If it is present anyway, it
+// must still be well-formed: it should not be silently ignored.
+func.func @malformed_optional_operand_segment_sizes(%arg0: i64) {
+ // expected-error at +1 {{for `operandSegmentSizes`: expected DenseI32ArrayAttr}}
+ test.variadic_segment_prop %arg0, %arg0 : %arg0 : i64, i64 : i64 <{operandSegmentSizes = "bad"}> end
+ return
+}
+
+// -----
+
+func.func @malformed_optional_result_segment_sizes(%arg0: i64) {
+ // expected-error at +1 {{for `resultSegmentSizes`: expected DenseI32ArrayAttr}}
+ 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/mlir-tblgen/op-format.td b/mlir/test/mlir-tblgen/op-format.td
index 1790737a3a349..7f388fd796f53 100644
--- a/mlir/test/mlir-tblgen/op-format.td
+++ b/mlir/test/mlir-tblgen/op-format.td
@@ -110,6 +110,38 @@ 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 optional (but still validated if present).
+// CHECK-LABEL: PropDictSegmentSizesInferred::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 && false) {
+// CHECK: if (attr && ::mlir::failed(::mlir::convertFromAttribute(prop.operandSegmentSizes, attr,
+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.
+// 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 && true) {
+// CHECK: if (attr && ::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
>From 1f940458423cbea69d4f443aaf34e49c95fe415d Mon Sep 17 00:00:00 2001
From: Natanael Cintean <natanael.cintean at intel.com>
Date: Tue, 28 Jul 2026 11:59:56 +0000
Subject: [PATCH 3/4] Only insert the key in the bulk operand/results formater
case, i.e. when they can't be inferred.
---
mlir/test/IR/invalid-properties.mlir | 16 ++++++++--------
mlir/test/mlir-tblgen/op-format.td | 16 +++++++---------
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 26 +++++++++++++++-----------
3 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/mlir/test/IR/invalid-properties.mlir b/mlir/test/IR/invalid-properties.mlir
index 3678c98db43d1..27c5adbf78e69 100644
--- a/mlir/test/IR/invalid-properties.mlir
+++ b/mlir/test/IR/invalid-properties.mlir
@@ -43,20 +43,20 @@ func.func @wrong_dense_i32_array_prop_type() {
// -----
-// `operandSegmentSizes` is not required in `<{...}>` for this op, since its
-// format spells out each variadic group individually
-// If it is present anyway, it
-// must still be well-formed: it should not be silently ignored.
-func.func @malformed_optional_operand_segment_sizes(%arg0: i64) {
- // expected-error at +1 {{for `operandSegmentSizes`: expected DenseI32ArrayAttr}}
+// `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 @malformed_optional_result_segment_sizes(%arg0: i64) {
- // expected-error at +1 {{for `resultSegmentSizes`: expected DenseI32ArrayAttr}}
+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
}
diff --git a/mlir/test/mlir-tblgen/op-format.td b/mlir/test/mlir-tblgen/op-format.td
index 7f388fd796f53..eea574888aee5 100644
--- a/mlir/test/mlir-tblgen/op-format.td
+++ b/mlir/test/mlir-tblgen/op-format.td
@@ -116,13 +116,11 @@ def OptionalGroupD : TestFormat_Op<[{
// 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 optional (but still validated if present).
+// `operandSegmentSizes` key is left completely untouched: it isn't read,
+// validated, or marked as a used key at all.
// CHECK-LABEL: PropDictSegmentSizesInferred::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 && false) {
-// CHECK: if (attr && ::mlir::failed(::mlir::convertFromAttribute(prop.operandSegmentSizes, attr,
+// CHECK-NOT: operandSegmentSizes
+// CHECK: for (::mlir::NamedAttribute attr : dict) {
def PropDictSegmentSizesInferred : TestFormat_Op<[{
$a1 `:` $a2 prop-dict attr-dict
}], [AttrSizedOperandSegments]>,
@@ -130,13 +128,13 @@ def PropDictSegmentSizesInferred : TestFormat_Op<[{
// 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.
+// `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 && true) {
-// CHECK: if (attr && ::mlir::failed(::mlir::convertFromAttribute(prop.operandSegmentSizes, attr,
+// 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]>,
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index f9d07f6b3ba0f..d1de4baa02e83 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1315,34 +1315,38 @@ ::mlir::MLIRContext *ctx = dict.getContext();
// `operandSegmentSizes`/`resultSegmentSizes` are trait-injected properties
// not enumerated by `op.getProperties()`, so they need to be special-cased
- // here, mirroring the handling in `setPropertiesFromAttr` in
- // OpDefinitionsGen.cpp.
+ // here, mirroring `setPropertiesFromAttr` in OpDefinitionsGen.cpp. This is
+ // only necessary when the format can't infer the sizes itself (bulk
+ // `operands`/`type(results)` directives); 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
- // {1}: isRequired
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 && {1}) {{
+if (!attr) {{
emitError() << "expected key entry for {0} in DictionaryAttr to set "
"Properties.";
return ::mlir::failure();
}
-if (attr && ::mlir::failed(::mlir::convertFromAttribute(prop.{0}, attr, [&]() {{
+if (::mlir::failed(::mlir::convertFromAttribute(prop.{0}, attr, [&]() {{
return emitError() << "for `{0}`: ";
})))
return ::mlir::failure();
)decl";
- if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments")) {
+ if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments") &&
+ fmt.allOperands) {
auto scope = body.scope("{\n", "}\n", /*indent=*/true);
- body << formatv(segmentSizesFromAttrFmt, "operandSegmentSizes",
- fmt.allOperands);
+ body << formatv(segmentSizesFromAttrFmt, "operandSegmentSizes");
}
- if (op.getTrait("::mlir::OpTrait::AttrSizedResultSegments")) {
+ if (op.getTrait("::mlir::OpTrait::AttrSizedResultSegments") &&
+ fmt.allResultTypes) {
auto scope = body.scope("{\n", "}\n", /*indent=*/true);
- body << formatv(segmentSizesFromAttrFmt, "resultSegmentSizes",
- fmt.allResultTypes);
+ body << formatv(segmentSizesFromAttrFmt, "resultSegmentSizes");
}
// {0}: fromAttribute call
>From 1a0497ccb15b9efbe7a8a4aa94b11c49accf7f5d Mon Sep 17 00:00:00 2001
From: Natanael Cintean <natanael.cintean at intel.com>
Date: Tue, 28 Jul 2026 12:45:31 +0000
Subject: [PATCH 4/4] Handle no declarative format corner case.
---
mlir/tools/mlir-tblgen/OpFormatGen.cpp | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index d1de4baa02e83..4d5c31ae2ecae 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1317,7 +1317,10 @@ ::mlir::MLIRContext *ctx = dict.getContext();
// 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); when the format spells out each
+ // `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
@@ -1338,13 +1341,14 @@ if (::mlir::failed(::mlir::convertFromAttribute(prop.{0}, attr, [&]() {{
})))
return ::mlir::failure();
)decl";
+ bool hasNoDeclarativeFormat = !op.hasAssemblyFormat();
if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments") &&
- fmt.allOperands) {
+ (hasNoDeclarativeFormat || fmt.allOperands)) {
auto scope = body.scope("{\n", "}\n", /*indent=*/true);
body << formatv(segmentSizesFromAttrFmt, "operandSegmentSizes");
}
if (op.getTrait("::mlir::OpTrait::AttrSizedResultSegments") &&
- fmt.allResultTypes) {
+ (hasNoDeclarativeFormat || fmt.allResultTypes)) {
auto scope = body.scope("{\n", "}\n", /*indent=*/true);
body << formatv(segmentSizesFromAttrFmt, "resultSegmentSizes");
}
More information about the Mlir-commits
mailing list