[Mlir-commits] [mlir] [mlir][tblgen] Fix bug around parsing optional prop-dict keys (PR #120045)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Krzysztof Drewniak (krzysz00)

<details>
<summary>Changes</summary>

The printer for prop-dict would elide properties that had their default value, such as optional properties that were not present. The parser would similarly not raise an error if such a key was missing. However, after not raising an error, the parser would attempt to convert the null attribute to a property anyway, causing failures.

This commit fixes the issue and adds tests.

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


3 Files Affected:

- (modified) mlir/test/lib/Dialect/Test/TestOpsSyntax.td (+7) 
- (modified) mlir/test/mlir-tblgen/op-format.mlir (+23) 
- (modified) mlir/tools/mlir-tblgen/OpFormatGen.cpp (+1-1) 


``````````diff
diff --git a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
index 795b9da955632c..c988d1f0ec871a 100644
--- a/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
+++ b/mlir/test/lib/Dialect/Test/TestOpsSyntax.td
@@ -431,6 +431,13 @@ def FormatOptionalWithElse : TEST_Op<"format_optional_else"> {
   let assemblyFormat = "(`then` $isFirstBranchPresent^):(`else`)? attr-dict";
 }
 
+def FormatOptionalPropDict : TEST_Op<"format_optional_prop_dict"> {
+  let arguments = (ins
+    OptionalProperty<StringProperty>:$a,
+    DefaultValuedProperty<I32Property, "1">:$b);
+  let assemblyFormat = "prop-dict attr-dict";
+}
+
 def FormatCompoundAttr : TEST_Op<"format_compound_attr"> {
   let arguments = (ins CompoundAttrA:$compound);
   let assemblyFormat = "$compound attr-dict-with-keyword";
diff --git a/mlir/test/mlir-tblgen/op-format.mlir b/mlir/test/mlir-tblgen/op-format.mlir
index 03288ae8bd3d77..08b0c52413a757 100644
--- a/mlir/test/mlir-tblgen/op-format.mlir
+++ b/mlir/test/mlir-tblgen/op-format.mlir
@@ -276,6 +276,29 @@ test.format_optional_else then
 // CHECK: test.format_optional_else else
 test.format_optional_else else
 
+//===----------------------------------------------------------------------===//
+// Default-valued properties (ex. optional) elided in property dictionary
+// TODO: elisions generate extra spaces
+//===----------------------------------------------------------------------===//
+
+// CHECK: test.format_optional_prop_dict {{$}}
+test.format_optional_prop_dict
+
+// CHECK: test.format_optional_prop_dict {{$}}
+test.format_optional_prop_dict <{a = [], b = 1 : i32}>
+
+// CHECK: test.format_optional_prop_dict {{$}}
+test.format_optional_prop_dict <{}>
+
+// CHECK: test.format_optional_prop_dict < {a = ["foo"]}>
+test.format_optional_prop_dict <{a = ["foo"]}>
+
+// CHECK: test.format_optional_prop_dict < {b = 2 : i32}>
+test.format_optional_prop_dict <{b = 2 : i32}>
+
+// CHECK: test.format_optional_prop_dict <{a = ["foo"], b = 2 : i32}>
+test.format_optional_prop_dict <{a = ["foo"], b = 2 : i32}>
+
 //===----------------------------------------------------------------------===//
 // Format a custom attribute
 //===----------------------------------------------------------------------===//
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index 097a578cb2025e..1f8d8992f898ab 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -1309,7 +1309,7 @@ if (!attr && {2}) {{
              "Properties.";
   return ::mlir::failure();
 }
-if (::mlir::failed(setFromAttr(prop.{1}, attr, emitError)))
+if (attr && ::mlir::failed(setFromAttr(prop.{1}, attr, emitError)))
   return ::mlir::failure();
 )decl";
 

``````````

</details>


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