[Mlir-commits] [mlir] [mlir] Fix TableGen emission for ops without properties (PR #112851)

[lorenzo chelini]

https://github.com/chelini created https://github.com/llvm/llvm-project/pull/112851

The current condition to emit `setPropertiesFromParsedAttr` is not strict enough since we emit the method even when we do not have properties at all. Similarly we should not emit `parseProperties` and `printProperties` if we don't have properties.

>From 7e5fbe83ea9558bf8cb329cd08800d589536df44 Mon Sep 17 00:00:00 2001
From: lorenzo chelini <lchelini at nvidia.com>
Date: Fri, 18 Oct 2024 10:16:24 +0200
Subject: [PATCH] [mlir] Fix TableGen emission for ops without properties

The current condition to emit `setPropertiesFromParsedAttr` is not strict enough
since we emit the method even when we do not have properties at all. Similarly
we should not emit `parseProperties` and `printProperties` if we don't have
properties.
---
 mlir/test/IR/properties.mlir           |  4 ++++
 mlir/test/lib/Dialect/Test/TestOps.td  |  5 +++++
 mlir/tools/mlir-tblgen/OpFormatGen.cpp | 19 ++++++++++++-------
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/mlir/test/IR/properties.mlir b/mlir/test/IR/properties.mlir
index 418b81dcbb034f..9a1c49cb7dabf3 100644
--- a/mlir/test/IR/properties.mlir
+++ b/mlir/test/IR/properties.mlir
@@ -19,6 +19,10 @@ test.with_nice_properties "foo bar" is -3
 // GENERIC-SAME:  <{prop = "content for properties"}> : () -> ()
 test.with_wrapped_properties <{prop = "content for properties"}>
 
+// CHECK: test.empty_properties
+// GENERIC: "test.empty_properties"()
+test.empty_properties
+
 // CHECK: test.using_property_in_custom
 // CHECK-SAME: [1, 4, 20]{{$}}
 // GENERIC: "test.using_property_in_custom"()
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 9e19966414d1d7..bc6c6cf213ea4b 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -3006,6 +3006,11 @@ def TestOpWithWrappedProperties : TEST_Op<"with_wrapped_properties"> {
   );
 }
 
+def TestOpWithEmptyProperties : TEST_Op<"empty_properties"> {
+  let assemblyFormat = "prop-dict attr-dict";
+  let arguments = (ins);
+}
+
 def TestOpUsingPropertyInCustom : TEST_Op<"using_property_in_custom"> {
   let assemblyFormat = "custom<UsingPropertyInCustom>($prop) attr-dict";
   let arguments = (ins IntArrayProperty<"int64_t">:$prop);
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index c99c71572bec23..4703de67025ded 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1275,8 +1275,9 @@ static void genAttrParser(AttributeVariable *attr, MethodBody &body,
 // 'prop-dict' dictionary attr.
 static void genParsedAttrPropertiesSetter(OperationFormat &fmt, Operator &op,
                                           OpClass &opClass) {
-  // Not required unless 'prop-dict' is present.
-  if (!fmt.hasPropDict)
+  // Not required unless 'prop-dict' is present or we
+  // are not using properties.
+  if (!fmt.hasPropDict || !fmt.useProperties)
     return;
 
   SmallVector<MethodParameter> paramList;
@@ -1621,8 +1622,10 @@ void OperationFormat::genElementParser(FormatElement *element, MethodBody &body,
     body.unindent() << "}\n";
     body.unindent();
   } else if (isa<PropDictDirective>(element)) {
-    body << "  if (parseProperties(parser, result))\n"
-         << "    return ::mlir::failure();\n";
+    if (useProperties) {
+      body << "  if (parseProperties(parser, result))\n"
+           << "    return ::mlir::failure();\n";
+    }
   } else if (auto *customDir = dyn_cast<CustomDirective>(element)) {
     genCustomDirectiveParser(customDir, body, useProperties, opCppClassName);
   } else if (isa<OperandsDirective>(element)) {
@@ -2047,9 +2050,11 @@ static void genPropDictPrinter(OperationFormat &fmt, Operator &op,
     }
   }
 
-  body << "  _odsPrinter << \" \";\n"
-       << "  printProperties(this->getContext(), _odsPrinter, "
-          "getProperties(), elidedProps);\n";
+  if (fmt.useProperties) {
+    body << "  _odsPrinter << \" \";\n"
+         << "  printProperties(this->getContext(), _odsPrinter, "
+            "getProperties(), elidedProps);\n";
+  }
 }
 
 /// Generate the printer for the 'attr-dict' directive.