[Mlir-commits] [mlir] eb16603 - [mlir] Expand default arg builders to wrapped attributes.
Jacques Pienaar
llvmlistbot at llvm.org
Fri Nov 3 15:30:17 PDT 2023
Author: Jacques Pienaar
Date: 2023-11-03T15:30:10-07:00
New Revision: eb166030a1b40567b7a83658a692f0eab99dde13
URL: https://github.com/llvm/llvm-project/commit/eb166030a1b40567b7a83658a692f0eab99dde13
DIFF: https://github.com/llvm/llvm-project/commit/eb166030a1b40567b7a83658a692f0eab99dde13.diff
LOG: [mlir] Expand default arg builders to wrapped attributes.
Previously only unwrapped values were considered for default values in
builders, expand to Attributes given the change to populate defaults. To avoid
overlap between wrapped and unwrapped, the starting index is incremented
depending on if the smallest starting index for default valued args can be
materialized in unwrapped form or not.
Given the above limitation this is pretty small change.
`optional` is unfortunately meant to also represent conditionally set which
means we cannot allow nullptr for all Attributes marked optional at the moment.
Reviewed By: Mogball
Differential Revision: https://reviews.llvm.org/D140705
Added:
mlir/test/mlir-tblgen/op-default-builder.td
Modified:
mlir/test/mlir-tblgen/op-attribute.td
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
Removed:
################################################################################
diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index cb72e99fde01b50..f81fc3df857ec26 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -412,7 +412,7 @@ def DefaultDictAttrOp : NS_Op<"default_dict_attr_op", []> {
// DEF: attributes.append(attrNames[1], odsBuilder.getDictionaryAttr(getDefaultDictAttrs(odsBuilder)));
// DECL-LABEL: DefaultDictAttrOp declarations
-// DECL: build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::DictionaryAttr empty, ::mlir::DictionaryAttr non_empty)
+// DECL: build(::mlir::OpBuilder &odsBuilder, ::mlir::OperationState &odsState, ::mlir::DictionaryAttr empty = nullptr, ::mlir::DictionaryAttr non_empty = nullptr)
// Test derived type attr.
diff --git a/mlir/test/mlir-tblgen/op-default-builder.td b/mlir/test/mlir-tblgen/op-default-builder.td
new file mode 100644
index 000000000000000..82386f245bc5dc1
--- /dev/null
+++ b/mlir/test/mlir-tblgen/op-default-builder.td
@@ -0,0 +1,71 @@
+// RUN: mlir-tblgen -gen-op-decls -I %S/../../include %s | FileCheck %s
+
+include "mlir/IR/AttrTypeBase.td"
+include "mlir/IR/EnumAttr.td"
+include "mlir/IR/OpBase.td"
+
+def Test_Dialect : Dialect {
+ let name = "test";
+ let cppNamespace = "foobar";
+}
+class NS_Op<string mnemonic, list<Trait> traits> :
+ Op<Test_Dialect, mnemonic, traits>;
+
+def SomeAttr : Attr<CPred<"some-condition">, "some attribute kind"> {
+ let storageType = "some-attr-kind";
+ let returnType = "some-return-type";
+ let convertFromStorage = "$_self.some-convert-from-storage()";
+ let constBuilderCall = "some-const-builder-call($_builder, $0)";
+}
+
+def AOp : NS_Op<"a_op", []> {
+ let arguments = (ins
+ FloatLike:$lhs,
+ SomeAttr:$aAttr,
+ DefaultValuedAttr<SomeAttr, "4.2">:$bAttr,
+ OptionalAttr<SomeAttr>:$cAttr,
+ DefaultValuedOptionalAttr<SomeAttr, "7.2">:$dAttr
+ );
+}
+
+// CHECK-LABEL: AOp declarations
+// Note: `cAttr` below could be conditionally optional and so the generation is
+// currently conservative.
+// CHECK-DAG: ::mlir::Value lhs, some-attr-kind aAttr, some-attr-kind bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-attr-kind dAttr);
+// CHECK-DAG: ::mlir::Value lhs, some-return-type aAttr, some-return-type bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-return-type dAttr = 7.2);
+// CHECK-DAG: ::mlir::TypeRange resultTypes, ::mlir::Value lhs, some-attr-kind aAttr, some-attr-kind bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-attr-kind dAttr);
+// CHECK-DAG: ::mlir::TypeRange resultTypes, ::mlir::Value lhs, some-return-type aAttr, some-return-type bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-return-type dAttr = 7.2);
+
+def BOp : NS_Op<"b_op", []> {
+ let arguments = (ins
+ DefaultValuedAttr<SomeAttr, "6.2">:$aAttr,
+ DefaultValuedAttr<SomeAttr, "4.2">:$bAttr
+ );
+}
+
+// Verify that non-overlapping builders created where all could be elided.
+// CHECK-LABEL: BOp declarations
+// CHECK-DAG: some-attr-kind aAttr, some-attr-kind bAttr = nullptr);
+// CHECK-DAG: some-return-type aAttr = 6.2, some-return-type bAttr = 4.2);
+// CHECK-DAG: ::mlir::TypeRange resultTypes, some-attr-kind aAttr, some-attr-kind bAttr = nullptr);
+// CHECK-DAG: ::mlir::TypeRange resultTypes, some-return-type aAttr = 6.2, some-return-type bAttr = 4.2);
+
+def COp : NS_Op<"c_op", []> {
+ let arguments = (ins
+ FloatLike:$value,
+ OptionalAttr<SymbolRefArrayAttr>:$ag,
+ OptionalAttr<SymbolRefArrayAttr>:$as,
+ OptionalAttr<SymbolRefArrayAttr>:$nos,
+ OptionalAttr<I64Attr>:$al,
+ UnitAttr:$vo,
+ UnitAttr:$non
+ );
+}
+
+// CHECK-LABEL: COp declarations
+// Note: `al` below could be conditionally optional and so the generation is
+// currently conservative.
+// CHECK-DAG: ::mlir::Value value, /*optional*/::mlir::ArrayAttr ag, /*optional*/::mlir::ArrayAttr as, /*optional*/::mlir::ArrayAttr nos, /*optional*/::mlir::IntegerAttr al, /*optional*/::mlir::UnitAttr vo, /*optional*/::mlir::UnitAttr non = nullptr);
+// CHECK-DAG: ::mlir::Value value, /*optional*/::mlir::ArrayAttr ag, /*optional*/::mlir::ArrayAttr as, /*optional*/::mlir::ArrayAttr nos, /*optional*/::mlir::IntegerAttr al, /*optional*/bool vo = false, /*optional*/bool non = false);
+// CHECK-DAG: ::mlir::TypeRange resultTypes, ::mlir::Value value, /*optional*/::mlir::ArrayAttr ag, /*optional*/::mlir::ArrayAttr as, /*optional*/::mlir::ArrayAttr nos, /*optional*/::mlir::IntegerAttr al, /*optional*/::mlir::UnitAttr vo, /*optional*/::mlir::UnitAttr non = nullptr);
+// CHECK-DAG: ::mlir::TypeRange resultTypes, ::mlir::Value value, /*optional*/::mlir::ArrayAttr ag, /*optional*/::mlir::ArrayAttr as, /*optional*/::mlir::ArrayAttr nos, /*optional*/::mlir::IntegerAttr al, /*optional*/bool vo = false, /*optional*/bool non = false);
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index a620265b3dd8809..842964b853d084d 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -2883,16 +2883,20 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> ¶mList,
// Successors and variadic regions go at the end of the parameter list, so no
// default arguments are possible.
bool hasTrailingParams = op.getNumSuccessors() || op.getNumVariadicRegions();
- if (attrParamKind == AttrParamKind::UnwrappedValue && !hasTrailingParams) {
+ if (!hasTrailingParams) {
// Calculate the start index from which we can attach default values in the
// builder declaration.
for (int i = op.getNumArgs() - 1; i >= 0; --i) {
auto *namedAttr =
llvm::dyn_cast_if_present<tblgen::NamedAttribute *>(op.getArg(i));
- if (!namedAttr || !namedAttr->attr.hasDefaultValue())
+ if (!namedAttr)
break;
- if (!canUseUnwrappedRawValue(namedAttr->attr))
+ Attribute attr = namedAttr->attr;
+ // TODO: Currently we can't
diff erentiate between optional meaning do not
+ // verify/not always error if missing or optional meaning need not be
+ // specified in builder. Expand isOptional once we can
diff erentiate.
+ if (!attr.hasDefaultValue() && !attr.isDerivedAttr())
break;
// Creating an APInt requires us to provide bitwidth, value, and
@@ -2907,6 +2911,21 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> ¶mList,
defaultValuedAttrStartIndex = i;
}
}
+ // Avoid generating build methods that are ambiguous due to default values by
+ // requiring at least one attribute.
+ if (defaultValuedAttrStartIndex < op.getNumArgs()) {
+ // TODO: This should have been possible as a cast<NamedAttribute> but
+ // required template instantiations is not yet defined for the tblgen helper
+ // classes.
+ auto *namedAttr =
+ cast<NamedAttribute *>(op.getArg(defaultValuedAttrStartIndex));
+ Attribute attr = namedAttr->attr;
+ if ((attrParamKind == AttrParamKind::WrappedAttr &&
+ canUseUnwrappedRawValue(attr)) ||
+ (attrParamKind == AttrParamKind::UnwrappedValue &&
+ !canUseUnwrappedRawValue(attr)))
+ ++defaultValuedAttrStartIndex;
+ }
/// Collect any inferred attributes.
for (const NamedTypeConstraint &operand : op.getOperands()) {
@@ -2959,9 +2978,12 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> ¶mList,
// Attach default value if requested and possible.
std::string defaultValue;
- if (attrParamKind == AttrParamKind::UnwrappedValue &&
- i >= defaultValuedAttrStartIndex) {
- defaultValue += attr.getDefaultValue();
+ if (i >= defaultValuedAttrStartIndex) {
+ if (attrParamKind == AttrParamKind::UnwrappedValue &&
+ canUseUnwrappedRawValue(attr))
+ defaultValue += attr.getDefaultValue();
+ else
+ defaultValue += "nullptr";
}
paramList.emplace_back(type, namedAttr.name, StringRef(defaultValue),
attr.isOptional());
More information about the Mlir-commits
mailing list