Skip to content

Commit 148d84a

Browse files
committed
[mlir-tblgen] Relax builder ambiguity check
The mlir-tblgen tool prevents the parameter of the build() constructor for the first default-valued attribute of an operation from having a default value to avoid ambiguity with the corresponding build() constructor taking unwrapped value. However it does so even when earlier wrapped unwrappable attribute would lift the ambiguity. This commit relax the logic accordingly, which allows to remove a manual constructor in Arith dialect.
1 parent 37832d5 commit 148d84a

File tree

3 files changed

+24
-13
lines changed

3 files changed

+24
-13
lines changed

mlir/include/mlir/Dialect/Arith/IR/ArithOps.td

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,15 +1550,6 @@ def Arith_CmpFOp : Arith_CompareOp<"cmpf",
15501550
static arith::CmpFPredicate getPredicateByName(StringRef name);
15511551
}];
15521552

1553-
let builders = [
1554-
OpBuilder<(ins "::mlir::arith::CmpFPredicateAttr":$predicate,
1555-
"Value":$lhs, "Value":$rhs), [{
1556-
build($_builder, $_state, predicate, lhs, rhs,
1557-
mlir::arith::FastMathFlagsAttr::get($_builder.getContext(),
1558-
mlir::arith::FastMathFlags::none));
1559-
}]>
1560-
];
1561-
15621553
let hasFolder = 1;
15631554
let hasCanonicalizer = 1;
15641555
let assemblyFormat = [{ $predicate `,` $lhs `,` $rhs (`fastmath` `` $fastmath^)?

mlir/test/mlir-tblgen/op-default-builder.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ def AOp : NS_Op<"a_op", []> {
3131
// CHECK-LABEL: AOp declarations
3232
// Note: `cAttr` below could be conditionally optional and so the generation is
3333
// currently conservative.
34-
// CHECK-DAG: ::mlir::Value lhs, some-attr-kind aAttr, some-attr-kind bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-attr-kind dAttr);
34+
// CHECK-DAG: ::mlir::Value lhs, some-attr-kind aAttr, some-attr-kind bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-attr-kind dAttr = nullptr);
3535
// 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);
36-
// 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);
36+
// 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 = nullptr);
3737
// 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);
3838

3939
def BOp : NS_Op<"b_op", []> {

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3088,6 +3088,26 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> &paramList,
30883088
defaultValuedAttrStartIndex = i;
30893089
}
30903090
}
3091+
3092+
// Check if parameters besides default valued one are enough to distinguish
3093+
// between builders with wrapped and unwrapped arguments.
3094+
bool hasBuilderAmbiguity = true;
3095+
for (int i = 0; i < op.getNumArgs(); ++i) {
3096+
auto *namedAttr = dyn_cast<NamedAttribute *>(op.getArg(i));
3097+
if (!namedAttr)
3098+
continue;
3099+
Attribute attr = namedAttr->attr;
3100+
if (attr.hasDefaultValue() || attr.isDerivedAttr())
3101+
continue;
3102+
3103+
if (attrParamKind != AttrParamKind::WrappedAttr ||
3104+
!canUseUnwrappedRawValue(attr))
3105+
continue;
3106+
3107+
hasBuilderAmbiguity = false;
3108+
break;
3109+
}
3110+
30913111
// Avoid generating build methods that are ambiguous due to default values by
30923112
// requiring at least one attribute.
30933113
if (defaultValuedAttrStartIndex < op.getNumArgs()) {
@@ -3098,9 +3118,9 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> &paramList,
30983118
cast<NamedAttribute *>(op.getArg(defaultValuedAttrStartIndex));
30993119
Attribute attr = namedAttr->attr;
31003120
if ((attrParamKind == AttrParamKind::WrappedAttr &&
3101-
canUseUnwrappedRawValue(attr)) ||
3121+
canUseUnwrappedRawValue(attr) && hasBuilderAmbiguity) ||
31023122
(attrParamKind == AttrParamKind::UnwrappedValue &&
3103-
!canUseUnwrappedRawValue(attr))) {
3123+
!canUseUnwrappedRawValue(attr) && hasBuilderAmbiguity)) {
31043124
++defaultValuedAttrStartIndex;
31053125
defaultValuedAttrLikeStartIndex = defaultValuedAttrStartIndex;
31063126
}

0 commit comments

Comments
 (0)