[mlir] [irdl] Optional attributes in irdl #110981
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir-irdl @llvm/pr-subscribers-mlir-core Author: Alex Rice (alexarice) ChangesAdds a variadicity array property to @math-fehr Full diff: https://github.com/llvm/llvm-project/pull/110981.diff 6 Files Affected:
diff --git a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
index c7fcb55120c827..f5b98ff5f1371e 100644
--- a/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
+++ b/mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
@@ -321,9 +321,10 @@ def IRDL_AttributesOp : IRDL_Op<"attributes", [HasParent<"OperationOp">]> {
}];
let arguments = (ins Variadic<IRDL_AttributeType>:$attributeValues,
- StrArrayAttr:$attributeValueNames);
+ StrArrayAttr:$attributeValueNames,
+ VariadicityArrayAttr:$variadicity);
let assemblyFormat = [{
- custom<AttributesOp>($attributeValues, $attributeValueNames) attr-dict
+ custom<AttributesOp>($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 c5c44d97ce0911..2c37a5bb82d74a 100644
--- a/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
+++ b/mlir/lib/Dialect/IRDL/IR/IRDL.cpp
@@ -107,11 +107,18 @@ LogicalResult ResultsOp::verify() {
LogicalResult AttributesOp::verify() {
size_t namesSize = getAttributeValueNames().size();
size_t valuesSize = getAttributeValues().size();
+ size_t numVariadicities = getVariadicity().size();
+
+ if (numVariadicities != valuesSize)
+ return emitOpError()
+ << "the number of attributes and their variadicities must be "
+ "the same, but got "
+ << valuesSize << " and " << numVariadicities << " respectively";
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();
@@ -249,13 +256,18 @@ static void printValuesWithVariadicity(OpAsmPrinter &p, Operation *op,
static ParseResult
parseAttributesOp(OpAsmParser &p,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &attrOperands,
- ArrayAttr &attrNamesAttr) {
+ ArrayAttr &attrNamesAttr,
+ VariadicityArrayAttr &variadicityAttr) {
Builder &builder = p.getBuilder();
+ MLIRContext *ctx = builder.getContext();
SmallVector<Attribute> attrNames;
+ SmallVector<VariadicityAttr> 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();
};
@@ -263,16 +275,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<int>(0, attrNames.size()), p,
- [&](int i) { p << attrNames[i] << " = " << attrArgs[i]; });
+ interleaveComma(llvm::seq<int>(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 89a059803a86fd..4f38aa80e6e8d0 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<variadicity_array[ single, single]>}> : (!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<variadicity_array[ single]>}> : (!irdl.attribute) -> ()
}
}
diff --git a/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir b/mlir/test/Dialect/IRDL/variadics-error.irdl.mlir
index 67fa94ea8b7427..51d9e5aeec2319 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) <{variadicity = #irdl<variadicity_array[single, single]>}> : (!irdl.attribute) -> ()
}
@@ -36,8 +36,45 @@ irdl.dialect @errors {
irdl.dialect @errors {
irdl.operation @results2 {
%0 = irdl.is i32
-
+
// expected-error@+1 {{'irdl.results' op the number of operands and their variadicities must be the same, but got 1 and 2 respectively}}
"irdl.results"(%0) <{variadicity = #irdl<variadicity_array[single, single]>}> : (!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<variadicity_array[ single]>}> : (!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<variadicity_array[ single, single]>}> : (!irdl.attribute) -> ()
+ }
+}
+
diff --git a/mlir/test/Dialect/IRDL/variadics.irdl.mlir b/mlir/test/Dialect/IRDL/variadics.irdl.mlir
index 64c5e1878f08eb..c61e764c2e118d 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(%[[v0]], variadic %[[v1]], %[[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(%[[v0]], optional %[[v1]], %[[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(variadic %[[v0]], optional %[[v1]], %[[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(%[[v0]], variadic %[[v1]], %[[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(%[[v0]], optional %[[v1]], %[[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(variadic %[[v0]], optional %[[v1]], %[[v2]])
// CHECK-NEXT: }
irdl.operation @var_and_opt_result {
@@ -99,4 +99,21 @@ irdl.dialect @testvar {
%2 = irdl.is i64
irdl.results(variadic %0, optional %1, %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 ace1029b4e7ff0..637598e2d43bac 100644
--- a/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp
+++ b/mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp
@@ -422,11 +422,18 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder,
SmallVector<Value> attributes;
SmallVector<Attribute> attrNames;
+ SmallVector<irdl::VariadicityAttr> attrVariadicity;
for (auto namedAttr : tblgenOp.getAttributes()) {
+ irdl::VariadicityAttr var;
if (namedAttr.attr.isOptional())
- continue;
+ var = consBuilder.getAttr<irdl::VariadicityAttr>(
+ irdl::Variadicity::optional);
+ else
+ var =
+ consBuilder.getAttr<irdl::VariadicityAttr>(irdl::Variadicity::single);
attributes.push_back(createAttrConstraint(consBuilder, namedAttr.attr));
attrNames.push_back(StringAttr::get(ctx, namedAttr.name));
+ attrVariadicity.push_back(var);
}
SmallVector<Value> regions;
@@ -443,8 +450,9 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder,
consBuilder.create<irdl::ResultsOp>(UnknownLoc::get(ctx), results,
resultVariadicity);
if (!attributes.empty())
- consBuilder.create<irdl::AttributesOp>(UnknownLoc::get(ctx), attributes,
- ArrayAttr::get(ctx, attrNames));
+ consBuilder.create<irdl::AttributesOp>(
+ UnknownLoc::get(ctx), attributes, ArrayAttr::get(ctx, attrNames),
+ irdl::VariadicityArrayAttr::get(ctx, attrVariadicity));
if (!regions.empty())
consBuilder.create<irdl::RegionsOp>(UnknownLoc::get(ctx), regions);
|
math-fehr
reviewed
Nov 22, 2024
|
|
||
| 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"; | ||
|
|
There was a problem hiding this comment.
Can you add a check that no variadicity of attribute is variadic?
Optional attributes make sense, but not variadic ones (they should be put in an array instead).
|
@Moxinilian could you confirm that you already implemented this in a different PR? Edit: I might have been getting confused, looks like named operands got added but not this. |
alexarice
force-pushed
the
alexarice/opt-attrs
branch
from
ab9fab0 to
936e69f
Compare
|
@math-fehr I've rebased onto main and added the verification/test you requested. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Ah, I didn't see the comment about the verifier, I'll take a look when I get the chance |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a variadicity array property to
irdl.attributes, allowing optional attributes(/properties) to be defined. Also updates the tblgen-to-irdl script to support this.@math-fehr