[Mlir-commits] [mlir] [mlir][ods] Fix ODS bug for usePropertiesForAttributes = 0 (PR #173006)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Philip Lassen (philass)

<details>
<summary>Changes</summary>

This fixes invalid cpp generated in the `verifyInvariantsImpl` method for operations generated from ODS when `usePropertiesForAttributes = 0` is set on the Dialect.

Fixes the bug introduced in
- https://github.com/llvm/llvm-project/pull/153603

Closes
- https://github.com/llvm/llvm-project/issues/171217



---
Full diff: https://github.com/llvm/llvm-project/pull/173006.diff


2 Files Affected:

- (modified) mlir/test/mlir-tblgen/op-attribute.td (+46) 
- (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+1-1) 


``````````diff
diff --git a/mlir/test/mlir-tblgen/op-attribute.td b/mlir/test/mlir-tblgen/op-attribute.td
index a3cb9a41a5b7f..4107c1fcc26ce 100644
--- a/mlir/test/mlir-tblgen/op-attribute.td
+++ b/mlir/test/mlir-tblgen/op-attribute.td
@@ -469,6 +469,52 @@ def EOp : NS_Op<"e_op", []> {
 // DECL-LABEL: EOp declarations
 // DECL: static void build({{.*}}, uint32_t i32_attr, uint32_t dv_i32_attr, ::llvm::APFloat f64_attr, ::llvm::APFloat dv_f64_attr, ::llvm::StringRef str_attr, ::llvm::StringRef dv_str_attr, bool bool_attr, bool dv_bool_attr, ::SomeI32Enum enum_attr, ::SomeI32Enum dv_enum_attr = ::SomeI32Enum::case5)
 
+// Test verifyInvariantsImpl for op in dialect with `usePropertiesForAttributes = 0` set.
+// ---
+
+def Test3_Dialect : Dialect {
+  let name = "test3";
+  let cppNamespace = "foobar3";
+  let usePropertiesForAttributes = 0;
+}
+def FOp : Op<Test3_Dialect, "f_op", []> {
+  let arguments = (ins
+      SomeAttr:$aAttr,
+      DefaultValuedOptionalAttr<SomeAttr, "4.2">:$bAttr,
+      OptionalAttr<SomeAttr>:$cAttr
+  );
+}
+
+// DEF-LABEL: FOp definitions
+// DEF: ::llvm::LogicalResult FOp::verifyInvariantsImpl() {
+// DEF:   auto namedAttrRange = (*this)->getAttrs();
+// DEF:   auto namedAttrIt = namedAttrRange.begin();
+// DEF:   ::mlir::Attribute tblgen_aAttr;
+
+// DEF:   while (true) {
+// DEF:     if (namedAttrIt == namedAttrRange.end())
+// DEF:       return emitOpError("requires attribute 'aAttr'");
+// DEF:     if (namedAttrIt->getName() == getAAttrAttrName()) {
+// DEF:       tblgen_aAttr = namedAttrIt->getValue();
+// DEF:       break;
+// DEF:     }
+// DEF:     ++namedAttrIt;
+// DEF:   }
+
+// DEF:   ::mlir::Attribute tblgen_bAttr;
+// DEF:   ::mlir::Attribute tblgen_cAttr;
+// DEF:   while (true) {
+// DEF:     if (namedAttrIt == namedAttrRange.end()) {
+// DEF:       break;
+// DEF:     }
+// DEF:     else if (namedAttrIt->getName() == getBAttrAttrName()) {
+// DEF:       tblgen_bAttr = namedAttrIt->getValue();
+// DEF:     }
+// DEF:     else if (namedAttrIt->getName() == getCAttrAttrName()) {
+// DEF:       tblgen_cAttr = namedAttrIt->getValue();
+// DEF:     }
+// DEF:     ++namedAttrIt;
+// DEF:   }
 
 // Test proper namespacing for AttrDef
 // ---
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index dbae5d9258d04..d1f1e85371133 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -970,7 +970,7 @@ genAttributeVerifier(const OpOrAdaptorHelper &emitHelper, FmtContext &ctx,
   const char *const findRequiredAttr = R"(
 while (true) {{
   if (namedAttrIt == namedAttrRange.end())
-    return {1}"requires attribute '{2}'");
+    return {1}requires attribute '{2}'");
   if (namedAttrIt->getName() == {0}) {{
     tblgen_{2} = namedAttrIt->getValue();
     break;

``````````

</details>


https://github.com/llvm/llvm-project/pull/173006