[MLIR] Make `print-op-generic` print generic DialectSymbol.

[bollu]

Summary

This permits external consumers of MLIR to easily parse attributes/types,
since we guarantee to print the generic form of the attribute/type
as !dialect<"name<args>"> / #dialect<"name<args>">.

[rriddle]

You can already easily parse attributes/types (they have a limited syntax, see what the MLIR parser does for this), and we are actively trying to remove the string form.

[bollu]

Rebase on latest build, fix tests

[bollu]

@rriddle as an external consumer of MLIR, it is quite annoying to have to parse both foo<"..."> as well as foo.bar<balanced-parens>.

It would be much more pleasant to have one or the other, *especially* since it seems like foo<"..."> isn't going anywhere anytime soon, since the syntax opaque syntax of a raw string is more expressive than the pretty syntax. [For example, as you know, the opaque syntax allows the string to have imbalanced parentheses such as foo<">">.]

[rriddle]

In D126551#3542967, @bollu wrote:

@rriddle as an external consumer of MLIR, it is quite annoying to have to parse both foo<"..."> as well as foo.bar<balanced-parens>.

It would be much more pleasant to have one or the other, *especially* since it seems like foo<"..."> isn't going anywhere anytime soon, since the syntax opaque syntax of a raw string is more expressive than the pretty syntax. [For example, as you know, the opaque syntax allows the string to have imbalanced parentheses such as foo<">">.]

I understand the frustration, but making things easier for external users to parse is not a motivating driver for developments/stability/etc. of the MLIR parser. This is especially true when they desires are counter to the direction that we want to drive the project.

[mehdi_amini]

In D126551#3542967, @bollu wrote:

since it seems like foo<"..."> isn't going anywhere anytime soon, since the syntax opaque syntax of a raw string is more expressive than the pretty syntax. [For example, as you know, the opaque syntax allows the string to have imbalanced parentheses such as foo<">">.]

@rriddle maybe you can expand on this? In particular since you mentioned

we are actively trying to remove the string form.

mlir/lib/IR/AsmPrinter.cpp

private:
   unsigned nextConflictID = 0;
 
   /// These are the printing flags.  They control, eg., whether to print in
   /// generic form.
   OpPrintingFlags printerFlags;
 };
 } // namespace
 
-SSANameState::SSANameState(
-    Operation *op, const OpPrintingFlags &printerFlags)
+SSANameState::SSANameState(Operation *op, const OpPrintingFlags &printerFlags)
     : printerFlags(printerFlags) {
   llvm::SaveAndRestore<unsigned> valueIDSaver(nextValueID);
   llvm::SaveAndRestore<unsigned> argumentIDSaver(nextArgumentID);
   llvm::SaveAndRestore<unsigned> conflictIDSaver(nextConflictID);
 
   // The naming context includes `nextValueID`, `nextArgumentID`,
   // `nextConflictID` and `usedNames` scoped HashTable. This information is
   // carried from the parent region.
static bool isDialectSymbolSimpleEnoughForPrettyForm(StringRef symName) {
   } while (!nestedPunctuation.empty());
 
   // If there were extra characters, then we failed.
   return symName.empty();
 }
 
 /// Print the given dialect symbol to the stream.
 static void printDialectSymbol(raw_ostream &os, StringRef symPrefix,
-                               StringRef dialectName, StringRef symString) {
+                               StringRef dialectName, StringRef symString,
+                               bool shouldPrintGenericOp) {
   os << symPrefix << dialectName;
 
   // If this symbol name is simple enough, print it directly in pretty form,
   // otherwise, we print it as an escaped string.
-  if (isDialectSymbolSimpleEnoughForPrettyForm(symString)) {
+  if (!shouldPrintGenericOp &&
+      isDialectSymbolSimpleEnoughForPrettyForm(symString)) {
     os << '.' << symString;
     return;
   }
 
   os << "<\"";
   llvm::printEscapedString(symString, os);
   os << "\">";
 }
void AsmPrinter::Impl::printAttribute(Attribute attr,
   if (succeeded(printAlias(attr)))
     return;
 
   auto attrType = attr.getType();
   if (!isa<BuiltinDialect>(attr.getDialect())) {
     printDialectAttribute(attr);
   } else if (auto opaqueAttr = attr.dyn_cast<OpaqueAttr>()) {
     printDialectSymbol(os, "#", opaqueAttr.getDialectNamespace(),
-                       opaqueAttr.getAttrData());
+                       opaqueAttr.getAttrData(),
+                       printerFlags.shouldPrintGenericOpForm());
   } else if (attr.isa<UnitAttr>()) {
     os << "unit";
     return;
   } else if (auto dictAttr = attr.dyn_cast<DictionaryAttr>()) {
     os << '{';
     interleaveComma(dictAttr.getValue(),
                     [&](NamedAttribute attr) { printNamedAttribute(attr); });
     os << '}';
void AsmPrinter::Impl::printType(Type type) {
 
   // Try to print an alias for this type.
   if (state && succeeded(state->getAliasState().getAlias(type, os)))
     return;
 
   TypeSwitch<Type>(type)
       .Case<OpaqueType>([&](OpaqueType opaqueTy) {
         printDialectSymbol(os, "!", opaqueTy.getDialectNamespace(),
-                           opaqueTy.getTypeData());
+                           opaqueTy.getTypeData(),
+                           printerFlags.shouldPrintGenericOpForm());
       })
       .Case<IndexType>([&](Type) { os << "index"; })
       .Case<BFloat16Type>([&](Type) { os << "bf16"; })
       .Case<Float16Type>([&](Type) { os << "f16"; })
       .Case<Float32Type>([&](Type) { os << "f32"; })
       .Case<Float64Type>([&](Type) { os << "f64"; })
       .Case<Float80Type>([&](Type) { os << "f80"; })
       .Case<Float128Type>([&](Type) { os << "f128"; })
void AsmPrinter::Impl::printDialectAttribute(Attribute attr) {
   // Ask the dialect to serialize the attribute to a string.
   std::string attrName;
   {
     llvm::raw_string_ostream attrNameStr(attrName);
     Impl subPrinter(attrNameStr, printerFlags, state);
     DialectAsmPrinter printer(subPrinter);
     dialect.printAttribute(attr, printer);
   }
-  printDialectSymbol(os, "#", dialect.getNamespace(), attrName);
+  printDialectSymbol(os, "#", dialect.getNamespace(), attrName,
+                     printerFlags.shouldPrintGenericOpForm());
 }
 
 void AsmPrinter::Impl::printDialectType(Type type) {
   auto &dialect = type.getDialect();
 
   // Ask the dialect to serialize the type to a string.
   std::string typeName;
   {
     llvm::raw_string_ostream typeNameStr(typeName);
     Impl subPrinter(typeNameStr, printerFlags, state);
     DialectAsmPrinter printer(subPrinter);
     dialect.printType(type, printer);
   }
-  printDialectSymbol(os, "!", dialect.getNamespace(), typeName);
+  printDialectSymbol(os, "!", dialect.getNamespace(), typeName,
+                     printerFlags.shouldPrintGenericOpForm());
 }
 
 void AsmPrinter::Impl::printEscapedString(StringRef str) {
   os << "\"";
   llvm::printEscapedString(str, os);
   os << "\"";
 }
 

mlir/test/Dialect/LLVMIR/func.mlir

 // RUN: mlir-opt -split-input-file -verify-diagnostics %s | mlir-opt | FileCheck %s
 // RUN: mlir-opt -split-input-file -verify-diagnostics -mlir-print-op-generic %s | FileCheck %s --check-prefix=GENERIC
 // RUN: mlir-opt -split-input-file -verify-diagnostics %s -mlir-print-debuginfo | mlir-opt -mlir-print-debuginfo | FileCheck %s --check-prefix=LOCINFO
 
 module {
   // GENERIC: "llvm.func"
-  // GENERIC: function_type = !llvm.func<void ()>
+  // GENERIC: function_type = !llvm<"func<void ()>">
   // GENERIC-SAME: sym_name = "foo"
   // GENERIC-SAME: () -> ()
   // CHECK: llvm.func @foo()
   "llvm.func"() ({
   }) {sym_name = "foo", function_type = !llvm.func<void ()>} : () -> ()
 
   // GENERIC: "llvm.func"
-  // GENERIC: function_type = !llvm.func<i64 (i64, i64)>
+  // GENERIC: function_type = !llvm<"func<i64 (i64, i64)>">
   // GENERIC-SAME: sym_name = "bar"
   // GENERIC-SAME: () -> ()
   // CHECK: llvm.func @bar(i64, i64) -> i64
   "llvm.func"() ({
   }) {sym_name = "bar", function_type = !llvm.func<i64 (i64, i64)>} : () -> ()
 
   // GENERIC: "llvm.func"
   // CHECK: llvm.func @baz(%{{.*}}: i64) -> i64
   "llvm.func"() ({
   // GENERIC: ^bb0
   ^bb0(%arg0: i64):
     // GENERIC: llvm.return
     llvm.return %arg0 : i64
 
-  // GENERIC: function_type = !llvm.func<i64 (i64)>
+  // GENERIC: function_type = !llvm<"func<i64 (i64)>">
   // GENERIC-SAME: sym_name = "baz"
   // GENERIC-SAME: () -> ()
   }) {sym_name = "baz", function_type = !llvm.func<i64 (i64)>} : () -> ()
 
   // CHECK: llvm.func @qux(!llvm.ptr<i64> {llvm.noalias}, i64)
   // CHECK: attributes {xxx = {yyy = 42 : i64}}
   "llvm.func"() ({
   }) {sym_name = "qux", function_type = !llvm.func<void (ptr<i64>, i64)>,
module {
   llvm.func weak @weak_linkage() {
     llvm.return
   }
 
   // Omit the `external` linkage, which is the default, in the custom format.
   // Check that it is present in the generic format using its numeric value.
   //
   // CHECK: llvm.func @external_func
-  // GENERIC: linkage = #llvm.linkage<external>
+  // GENERIC: linkage = #llvm<"linkage<external>">
   llvm.func external @external_func()
 
   // CHECK-LABEL: llvm.func @arg_struct_attr(
   // CHECK-SAME: %{{.*}}: !llvm.struct<(i32)> {llvm.struct_attrs = [{llvm.noalias}]}) {
   llvm.func @arg_struct_attr(
       %arg0 : !llvm.struct<(i32)> {llvm.struct_attrs = [{llvm.noalias}]}) {
     llvm.return
   }