Skip to content

[mlir] [irdl] Optional attributes in irdl #110981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mlir/include/mlir/Dialect/IRDL/IR/IRDLOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,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;
Expand Down
42 changes: 36 additions & 6 deletions mlir/lib/Dialect/IRDL/IR/IRDL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

return success();
Expand Down Expand Up @@ -383,30 +401,42 @@ static void printNamedValueListWithVariadicity(
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();
};
if (p.parseCommaSeparatedList(parseOperands) || p.parseRBrace())
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 << '}';
}

Expand Down
11 changes: 6 additions & 5 deletions mlir/test/Dialect/IRDL/attributes-op.irdl.mlir
Original file line number Diff line number Diff line change
@@ -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) -> ()
}
}

Expand All @@ -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) -> ()
}
}
52 changes: 50 additions & 2 deletions mlir/test/Dialect/IRDL/variadics-error.irdl.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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<variadicity_array[single, single]>}> : (!irdl.attribute) -> ()
}
Expand All @@ -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<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) -> ()
}
}

// -----

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<variadicity_array[ variadic]>}> : (!irdl.attribute) -> ()
}
}

53 changes: 35 additions & 18 deletions mlir/test/Dialect/IRDL/variadics.irdl.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
}
}
14 changes: 11 additions & 3 deletions mlir/tools/tblgen-to-irdl/OpDefinitionsGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,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;
Expand All @@ -479,8 +486,9 @@ irdl::OperationOp createIRDLOperation(OpBuilder &builder,
ArrayAttr::get(ctx, resultNames),
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,
ArrayAttr::get(ctx, regionNames));
Expand Down