[Mlir-commits] [mlir] 70074c1 - [mlir][AsmPrinter] Fallback to using qualified printer if unqualified output is empty

[Markus Böck]

Author: Markus Böck
Date: 2023-03-27T15:48:48+02:00
New Revision: 70074c113432e4065724b5aba79f610d9d7281de

URL: https://github.com/llvm/llvm-project/commit/70074c113432e4065724b5aba79f610d9d7281de
DIFF: https://github.com/llvm/llvm-project/commit/70074c113432e4065724b5aba79f610d9d7281de.diff

LOG: [mlir][AsmPrinter] Fallback to using qualified printer if unqualified output is empty

The current behaviour of always writing the unqualified form of an attribute or type is problematic for any type or attribute that might output an empty string, making it impossible to parse and therefore roundtrip. This is commonly the case for any types or attributes with optional parameters. One would have to currently woarkaround the issue by either changing ones syntax to not be completetly empty or by explicitly using `qualified` in ALL ops using that type or attribute.

This patch fixes that issue by simply checking whether anything was written to the output. In the case there wasn't, it simply falls back to using the normal printer with the dialect prefix. This also makes the default of unqualified printing always correct and safe. The implementation could theoretically still be tricked if the user were to print just a space or similar. I'd argue this'd be user error and not worth handling.

Fixes https://github.com/llvm/llvm-project/issues/61701

Differential Revision: https://reviews.llvm.org/D146944

Added: 
    mlir/test/IR/print-empty-attr-or-type.mlir

Modified: 
    mlir/include/mlir/IR/OpImplementation.h
    mlir/test/lib/Dialect/Test/TestOps.td
    mlir/test/lib/Dialect/Test/TestTypeDefs.td

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index e2cd8dc358e72..02abca33f9460 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -141,7 +141,16 @@ class AsmPrinter {
   void printStrippedAttrOrType(AttrOrType attrOrType) {
     if (succeeded(printAlias(attrOrType)))
       return;
+
+    raw_ostream &os = getStream();
+    uint64_t posPrior = os.tell();
     attrOrType.print(*this);
+    if (posPrior != os.tell())
+      return;
+
+    // Fallback to printing with prefix if the above failed to write anything
+    // to the output stream.
+    *this << attrOrType;
   }
 
   /// Print the provided array of attributes or types in the context of an

diff  --git a/mlir/test/IR/print-empty-attr-or-type.mlir b/mlir/test/IR/print-empty-attr-or-type.mlir
new file mode 100644
index 0000000000000..836467d6786a5
--- /dev/null
+++ b/mlir/test/IR/print-empty-attr-or-type.mlir
@@ -0,0 +1,9 @@
+// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+
+func.func @test(%arg0 : !test.optional_value_type, %arg1 : !test.optional_value_type<3>) {
+  // CHECK: test.format_maybe_empty_type %{{.*}} : !test.optional_value_type
+  test.format_maybe_empty_type %arg0 : !test.optional_value_type
+  // CHECK: test.format_maybe_empty_type %{{.*}} : <3>
+  test.format_maybe_empty_type %arg1 : !test.optional_value_type<3>
+  return
+}

diff  --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index e747d4bddfd7a..ff24ac94bdfb3 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -2294,6 +2294,11 @@ def FormatNestedCompoundAttr : TEST_Op<"format_cpmd_nested_attr"> {
   let assemblyFormat = "`nested` $nested attr-dict-with-keyword";
 }
 
+def FormatMaybeEmptyType : TEST_Op<"format_maybe_empty_type"> {
+  let arguments = (ins TestTypeOptionalValueType:$in);
+  let assemblyFormat = "$in `:` type($in) attr-dict";
+}
+
 def FormatQualifiedCompoundAttr : TEST_Op<"format_qual_cpmd_nested_attr"> {
   let arguments = (ins CompoundNestedOuter:$nested);
   let assemblyFormat = "`nested` qualified($nested) attr-dict-with-keyword";

diff  --git a/mlir/test/lib/Dialect/Test/TestTypeDefs.td b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
index 72e857c122224..68588cd232658 100644
--- a/mlir/test/lib/Dialect/Test/TestTypeDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
@@ -312,6 +312,14 @@ def TestTypeAPFloat : Test_Type<"TestTypeAPFloat"> {
   let assemblyFormat = "`<` $a `>`";
 }
 
+def TestTypeOptionalValueType : Test_Type<"TestTypeOptionalValueType"> {
+  let parameters = (ins
+    OptionalParameter<"std::optional<int>">:$value
+  );
+  let mnemonic = "optional_value_type";
+  let assemblyFormat = "(`<` $value^ `>`)?";
+}
+
 def TestTypeDefaultValuedType : Test_Type<"TestTypeDefaultValuedType"> {
   let parameters = (ins
     DefaultValuedParameter<"mlir::IntegerType",