[mlir-tblgen] Relax builder ambiguity check (#118310)

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.

Author: RoboTux

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

@@ -1556,15 +1556,6 @@ def Arith_CmpFOp : Arith_CompareOp<"cmpf",
     static arith::CmpFPredicate getPredicateByName(StringRef name);
   }];
 
-  let builders = [
-    OpBuilder<(ins "::mlir::arith::CmpFPredicateAttr":$predicate,
-                   "Value":$lhs, "Value":$rhs), [{
-      build($_builder, $_state, predicate, lhs, rhs,
-          mlir::arith::FastMathFlagsAttr::get($_builder.getContext(),
-              mlir::arith::FastMathFlags::none));
-    }]>
-  ];
-
   let hasFolder = 1;
   let hasCanonicalizer = 1;
   let assemblyFormat = [{ $predicate `,` $lhs `,` $rhs (`fastmath` `` $fastmath^)?

mlir/include/mlir/Dialect/Tosa/IR/TosaOps.td

@@ -1905,24 +1905,6 @@ def Tosa_RescaleOp: Tosa_Op<"rescale", [Pure,
     Tosa_Tensor:$output
   );
 
-  // Custom builder that does not require optional input_unsigned and
-  // output_unsigned.
-  let builders = [
-    OpBuilder<(ins "::mlir::Type":$output,
-                   "::mlir::Value":$input,
-                   "::mlir::IntegerAttr":$input_zp,
-                   "::mlir::IntegerAttr":$output_zp,
-                   "::mlir::DenseI32ArrayAttr":$multiplier,
-                   "::mlir::DenseI8ArrayAttr":$shift,
-                   "::mlir::BoolAttr":$scale32,
-                   "::mlir::BoolAttr":$double_round,
-                   "::mlir::BoolAttr":$per_channel), [{
-      auto FalseAttr = BoolAttr::get($_builder.getContext(), false);
-      build($_builder, $_state, output, input, input_zp, output_zp, multiplier,
-            shift, scale32, double_round, per_channel, FalseAttr, FalseAttr);
-    }]>
-  ];
-
   let assemblyFormat = "operands attr-dict `:` functional-type(operands, results)";
 }
 

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

@@ -31,9 +31,9 @@ def AOp : NS_Op<"a_op", []> {
 // 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-attr-kind aAttr, some-attr-kind bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-attr-kind dAttr = nullptr);
 // 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-attr-kind aAttr, some-attr-kind bAttr, /*optional*/some-attr-kind cAttr, /*optional*/some-attr-kind dAttr = nullptr);
 // 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", []> {

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

@@ -3088,6 +3088,26 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> &paramList,
       defaultValuedAttrStartIndex = i;
     }
   }
+
+  // Check if parameters besides default valued one are enough to distinguish
+  // between builders with wrapped and unwrapped arguments.
+  bool hasBuilderAmbiguity = true;
+  for (const auto &arg : op.getArgs()) {
+    auto *namedAttr = dyn_cast<NamedAttribute *>(arg);
+    if (!namedAttr)
+      continue;
+    Attribute attr = namedAttr->attr;
+    if (attr.hasDefaultValue() || attr.isDerivedAttr())
+      continue;
+
+    if (attrParamKind != AttrParamKind::WrappedAttr ||
+        !canUseUnwrappedRawValue(attr))
+      continue;
+
+    hasBuilderAmbiguity = false;
+    break;
+  }
+
   // Avoid generating build methods that are ambiguous due to default values by
   // requiring at least one attribute.
   if (defaultValuedAttrStartIndex < op.getNumArgs()) {
@@ -3098,9 +3118,9 @@ void OpEmitter::buildParamList(SmallVectorImpl<MethodParameter> &paramList,
         cast<NamedAttribute *>(op.getArg(defaultValuedAttrStartIndex));
     Attribute attr = namedAttr->attr;
     if ((attrParamKind == AttrParamKind::WrappedAttr &&
-         canUseUnwrappedRawValue(attr)) ||
+         canUseUnwrappedRawValue(attr) && hasBuilderAmbiguity) ||
         (attrParamKind == AttrParamKind::UnwrappedValue &&
-         !canUseUnwrappedRawValue(attr))) {
+         !canUseUnwrappedRawValue(attr) && hasBuilderAmbiguity)) {
       ++defaultValuedAttrStartIndex;
       defaultValuedAttrLikeStartIndex = defaultValuedAttrStartIndex;
     }