From 936e69fffdc404433fd95e3a6f9899b6e399579a Mon Sep 17 00:00:00 2001 From: Alex Rice Date: Thu, 22 Aug 2024 16:29:27 +0100 Subject: [PATCH 1/2] Optional attributes --- mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td | 5 +- mlir/lib/Dialect/IRDL/IR/IRDL.cpp | 42 ++++++++++++--- .../test/Dialect/IRDL/attributes-op.irdl.mlir | 11 ++-- .../Dialect/IRDL/variadics-error.irdl.mlir | 52 +++++++++++++++++- mlir/test/Dialect/IRDL/variadics.irdl.mlir | 53 ++++++++++++------- .../tools/tblgen-to-irdl/OpDefinitionsGen.cpp | 14 +++-- 6 files changed, 141 insertions(+), 36 deletions(-) diff --git a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td index f3bc3497500e7..32365e90607bc 100644 --- a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td +++ b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td @@ -328,9 +328,10 @@ def IRDL_AttributesOp : IRDL_Op<"attributes", [HasParent<"OperationOp">]> { }]; let arguments = (ins Variadic:$attributeValues, - StrArrayAttr:$attributeValueNames); + StrArrayAttr:$attributeValueNames, + VariadicityArrayAttr:$variadicity); let assemblyFormat = [{ - custom($attributeValues, $attributeValueNames) attr-dict + custom($attributeValues, $attributeValueNames, $variadicity) attr-dict }]; let hasVerifier = true; diff --git a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp index c0778d478619a..641f8b7eef285 100644 --- a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp +++ b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp @@ -186,11 +186,29 @@ LogicalResult ResultsOp::verify() { LogicalResult AttributesOp::verify() { size_t namesSize = getAttributeValueNames().size(); size_t valuesSize = getAttributeValues().size(); + auto variadicities = getVariadicity(); + size_t numVariadicities = variadicities.size(); + + if (numVariadicities != valuesSize) + return emitOpError() + << "the number of attributes and their variadicities must be " + "the same, but got " + << valuesSize << " and " << numVariadicities << " respectively"; + + for (size_t i = 0; i < numVariadicities; ++i) { + if (variadicities[i].getValue() == Variadicity::variadic) { + // auto test = + return emitOpError() << "requires attributes to have single or optional variadicity, but " << getAttributeValueNames()[i] + << " was specified as variadic."; + + + } + } if (namesSize != valuesSize) return emitOpError() << "the number of attribute names and their constraints must be " - "the same but got " + "the same, but got " << namesSize << " and " << valuesSize << " respectively"; return success(); @@ -383,13 +401,18 @@ static void printNamedValueListWithVariadicity( static ParseResult parseAttributesOp(OpAsmParser &p, SmallVectorImpl &attrOperands, - ArrayAttr &attrNamesAttr) { + ArrayAttr &attrNamesAttr, + VariadicityArrayAttr &variadicityAttr) { Builder &builder = p.getBuilder(); + MLIRContext *ctx = builder.getContext(); SmallVector attrNames; + SmallVector variadicities; + if (succeeded(p.parseOptionalLBrace())) { auto parseOperands = [&]() { if (p.parseAttribute(attrNames.emplace_back()) || p.parseEqual() || - p.parseOperand(attrOperands.emplace_back())) + parseValueWithVariadicity(p, attrOperands.emplace_back(), + variadicities.emplace_back())) return failure(); return success(); }; @@ -397,16 +420,23 @@ parseAttributesOp(OpAsmParser &p, return failure(); } attrNamesAttr = builder.getArrayAttr(attrNames); + variadicityAttr = VariadicityArrayAttr::get(ctx, variadicities); return success(); } static void printAttributesOp(OpAsmPrinter &p, AttributesOp op, - OperandRange attrArgs, ArrayAttr attrNames) { + OperandRange attrArgs, ArrayAttr attrNames, + VariadicityArrayAttr variadicityAttr) { if (attrNames.empty()) return; p << "{"; - interleaveComma(llvm::seq(0, attrNames.size()), p, - [&](int i) { p << attrNames[i] << " = " << attrArgs[i]; }); + interleaveComma(llvm::seq(0, attrNames.size()), p, [&](int i) { + Variadicity variadicity = variadicityAttr[i].getValue(); + p << attrNames[i] << " = "; + if (variadicity != Variadicity::single) + p << stringifyVariadicity(variadicity) << " "; + p << attrArgs[i]; + }); p << '}'; } diff --git a/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir b/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir index 89a059803a86f..4f38aa80e6e8d 100644 --- a/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir +++ b/mlir/test/Dialect/IRDL/attributes-op.irdl.mlir @@ -1,11 +1,12 @@ // RUN: mlir-opt %s -verify-diagnostics -split-input-file + irdl.dialect @errors { irdl.operation @attrs1 { %0 = irdl.is i32 %1 = irdl.is i64 - // expected-error@+1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same but got 1 and 2 respectively}} - "irdl.attributes"(%0, %1) <{attributeValueNames = ["attr1"]}> : (!irdl.attribute, !irdl.attribute) -> () + // expected-error@+1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same, but got 1 and 2 respectively}} + "irdl.attributes"(%0, %1) <{attributeValueNames = ["attr1"], variadicity = #irdl}> : (!irdl.attribute, !irdl.attribute) -> () } } @@ -15,8 +16,8 @@ irdl.dialect @errors { irdl.operation @attrs2 { %0 = irdl.is i32 %1 = irdl.is i64 - - // expected-error@+1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same but got 2 and 1 respectively}} - "irdl.attributes"(%0) <{attributeValueNames = ["attr1", "attr2"]}> : (!irdl.attribute) -> () + + // expected-error@+1 {{'irdl.attributes' op the number of attribute names and their constraints must be the same, but got 2 and 1 respectively}} + "irdl.attributes"(%0) <{attributeValueNames = ["attr1", "attr2"], variadicity = #irdl}> : (!irdl.attribute) -> () } } diff --git a/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir b/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir index 3e29dd837916b..86758fb97ac75 100644 --- a/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir +++ b/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir @@ -14,7 +14,7 @@ irdl.dialect @errors { irdl.dialect @errors { irdl.operation @operands2 { %0 = irdl.is i32 - + // expected-error@+1 {{'irdl.operands' op the number of operands and their variadicities must be the same, but got 1 and 2 respectively}} "irdl.operands"(%0) <{names = ["foo"], variadicity = #irdl}> : (!irdl.attribute) -> () } @@ -36,8 +36,56 @@ irdl.dialect @errors { irdl.dialect @errors { irdl.operation @results2 { %0 = irdl.is i32 - + // expected-error@+1 {{'irdl.results' op the number of results and their variadicities must be the same, but got 1 and 2 respectively}} "irdl.results"(%0) <{names = ["foo"], variadicity = #irdl}> : (!irdl.attribute) -> () } } + +// ----- + +irdl.dialect @errors { + irdl.operation @no_var { + %0 = irdl.is i32 + %1 = irdl.is i64 + + // expected-error@+1 {{'irdl.attributes' op requires attribute 'variadicity'}} + "irdl.attributes"(%0) <{attributeValueNames = ["attr1"]}> : (!irdl.attribute) -> () + } +} + +// ----- + +irdl.dialect @errors { + irdl.operation @attrs1 { + %0 = irdl.is i32 + %1 = irdl.is i64 + + // expected-error@+1 {{'irdl.attributes' op the number of attributes and their variadicities must be the same, but got 2 and 1 respectively}} + "irdl.attributes"(%0, %1) <{attributeValueNames = ["attr1", "attr2"], variadicity = #irdl}> : (!irdl.attribute, !irdl.attribute) -> () + } +} + +// ----- + +irdl.dialect @errors { + irdl.operation @attrs2 { + %0 = irdl.is i32 + %1 = irdl.is i64 + + // expected-error@+1 {{'irdl.attributes' op the number of attributes and their variadicities must be the same, but got 1 and 2 respectively}} + "irdl.attributes"(%0) <{attributeValueNames = ["attr1"], variadicity = #irdl}> : (!irdl.attribute) -> () + } +} + +// ----- + +irdl.dialect @errors { + irdl.operation @attrs_variadic { + %0 = irdl.is i32 + + // expected-error@+1 {{'irdl.attributes' op requires attributes to have single or optional variadicity, but "attr1" was specified as variadic}} + "irdl.attributes"(%0) <{attributeValueNames = ["attr1"], variadicity = #irdl}> : (!irdl.attribute) -> () + } +} + diff --git a/mlir/test/Dialect/IRDL/variadics.irdl.mlir b/mlir/test/Dialect/IRDL/variadics.irdl.mlir index 5118320f6d2e8..73d303941e99f 100644 --- a/mlir/test/Dialect/IRDL/variadics.irdl.mlir +++ b/mlir/test/Dialect/IRDL/variadics.irdl.mlir @@ -13,9 +13,9 @@ irdl.dialect @testvar { } // CHECK-LABEL: irdl.operation @var_operand { - // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 - // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 - // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 // CHECK-NEXT: irdl.operands(foo: %[[v0]], bar: variadic %[[v1]], baz: %[[v2]]) // CHECK-NEXT: } irdl.operation @var_operand { @@ -26,9 +26,9 @@ irdl.dialect @testvar { } // CHECK-LABEL: irdl.operation @opt_operand { - // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 - // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 - // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 // CHECK-NEXT: irdl.operands(foo: %[[v0]], bar: optional %[[v1]], baz: %[[v2]]) // CHECK-NEXT: } irdl.operation @opt_operand { @@ -39,9 +39,9 @@ irdl.dialect @testvar { } // CHECK-LABEL: irdl.operation @var_and_opt_operand { - // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 - // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 - // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 // CHECK-NEXT: irdl.operands(foo: variadic %[[v0]], bar: optional %[[v1]], baz: %[[v2]]) // CHECK-NEXT: } irdl.operation @var_and_opt_operand { @@ -62,9 +62,9 @@ irdl.dialect @testvar { } // CHECK-LABEL: irdl.operation @var_result { - // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 - // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 - // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 // CHECK-NEXT: irdl.results(foo: %[[v0]], bar: variadic %[[v1]], baz: %[[v2]]) // CHECK-NEXT: } irdl.operation @var_result { @@ -75,9 +75,9 @@ irdl.dialect @testvar { } // CHECK-LABEL: irdl.operation @opt_result { - // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 - // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 - // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 // CHECK-NEXT: irdl.results(foo: %[[v0]], bar: optional %[[v1]], baz: %[[v2]]) // CHECK-NEXT: } irdl.operation @opt_result { @@ -88,9 +88,9 @@ irdl.dialect @testvar { } // CHECK-LABEL: irdl.operation @var_and_opt_result { - // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 - // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 - // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 // CHECK-NEXT: irdl.results(foo: variadic %[[v0]], bar: optional %[[v1]], baz: %[[v2]]) // CHECK-NEXT: } irdl.operation @var_and_opt_result { @@ -99,4 +99,21 @@ irdl.dialect @testvar { %2 = irdl.is i64 irdl.results(foo: variadic %0, bar: optional %1, baz: %2) } + + // CHECK-LABEL: irdl.operation @var_attr { + // CHECK-NEXT: %[[v0:[^ ]*]] = irdl.is i16 + // CHECK-NEXT: %[[v1:[^ ]*]] = irdl.is i32 + // CHECK-NEXT: %[[v2:[^ ]*]] = irdl.is i64 + // CHECK-NEXT: irdl.attributes {"optional" = optional %[[v0]], "single" = %[[v1]], "single_no_word" = %[[v2]]} + // CHECK-NEXT: } + irdl.operation @var_attr { + %0 = irdl.is i16 + %1 = irdl.is i32 + %2 = irdl.is i64 + irdl.attributes { + "optional" = optional %0, + "single" = single %1, + "single_no_word" = %2 + } + } } diff --git a/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp b/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp index c2ad09ffaaed5..2cb603e27c417 100644 --- a/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp +++ b/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp @@ -454,11 +454,18 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder, SmallVector attributes; SmallVector attrNames; + SmallVector attrVariadicity; for (auto namedAttr : tblgenOp.getAttributes()) { + irdl::VariadicityAttr var; if (namedAttr.attr.isOptional()) - continue; + var = consBuilder.getAttr( + irdl::Variadicity::optional); + else + var = + consBuilder.getAttr(irdl::Variadicity::single); attributes.push_back(createAttrConstraint(consBuilder, namedAttr.attr)); attrNames.push_back(StringAttr::get(ctx, namedAttr.name)); + attrVariadicity.push_back(var); } SmallVector regions; @@ -479,8 +486,9 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder, ArrayAttr::get(ctx, resultNames), resultVariadicity); if (!attributes.empty()) - consBuilder.create(UnknownLoc::get(ctx), attributes, - ArrayAttr::get(ctx, attrNames)); + consBuilder.create( + UnknownLoc::get(ctx), attributes, ArrayAttr::get(ctx, attrNames), + irdl::VariadicityArrayAttr::get(ctx, attrVariadicity)); if (!regions.empty()) consBuilder.create(UnknownLoc::get(ctx), regions, ArrayAttr::get(ctx, regionNames)); From 8f664e87df3ddc38087826e351b6da286b0a1ccb Mon Sep 17 00:00:00 2001 From: Alex Rice Date: Thu, 20 Feb 2025 18:22:25 +0000 Subject: [PATCH 2/2] formatting --- mlir/lib/Dialect/IRDL/IR/IRDL.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp index 641f8b7eef285..2f2646a43d3b6 100644 --- a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp +++ b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp @@ -198,10 +198,10 @@ LogicalResult AttributesOp::verify() { for (size_t i = 0; i < numVariadicities; ++i) { if (variadicities[i].getValue() == Variadicity::variadic) { // auto test = - return emitOpError() << "requires attributes to have single or optional variadicity, but " << getAttributeValueNames()[i] + return emitOpError() << "requires attributes to have single or optional " + "variadicity, but " + << getAttributeValueNames()[i] << " was specified as variadic."; - - } }