[mlir][NFC] Make 'printOp' public in AsmPrinter

[dcaballe]

Summary

This patch moves the 'printOp' functionality to the public API of
AsmPrinter. No 'parseOp' seems to be needed at this time as existing
APIs are able to parse operations producing results where results are
omitted in the textual form (the LHS of an operation is redundant
when it comes to building the operation itself as it only contains
the result names).

[dcaballe]

mlir/lib/IR/AsmPrinter.cpp

2669

We have too many similar print methods for Operation *. We should rename them but I prefer to get your feedback first.

[rriddle]

Can you add a test in the TestDialect?

mlir/lib/IR/AsmPrinter.cpp

2669

Yeah, it'd be nice to rename them to be explicit about how they are printing. The current ones are really hard to reason about.

[dcaballe]

Renaming + testing

[bondhugula]

mlir/lib/IR/AsmPrinter.cpp

2669

What exactly is "etc." here?

2670

The names print and printFullOp look confusing now: the former is printing more than the latter now.

[dcaballe]

mlir/lib/IR/AsmPrinter.cpp

2669

I guess it refers to the location context but we only have indentation and location for now. I'll remove the etc. part.

2670

I kept print because I saw another print method for blocks but I now see it's the only one? I can rename it to printFullOpWithIndent or printFullOpWithIndentAndLoc but I'm not sure if this will scale well if we add more information to be printed. Perhaps something like printFullOpInContext or something like that?

[bondhugula]

mlir/lib/IR/AsmPrinter.cpp

2670

I don't have any more comments. Please feel free to go with what you think is better.

mlir/include/mlir/IR/OpImplementation.h

virtual void printOptionalAttrDict(ArrayRef<NamedAttribute> attrs,
                                      ArrayRef<StringRef> elidedAttrs = {}) = 0;
 
   /// If the specified operation has attributes, print out an attribute
   /// dictionary prefixed with 'attributes'.
   virtual void
   printOptionalAttrDictWithKeyword(ArrayRef<NamedAttribute> attrs,
                                    ArrayRef<StringRef> elidedAttrs = {}) = 0;
 
+  /// Prints the entire operation with the custom or generic assembly form.
+  virtual void printOp(Operation *op) = 0;
+
   /// Print the entire operation with the default generic assembly form.
   /// If `printOpName` is true, then the operation name is printed (the default)
   /// otherwise it is omitted and the print will start with the operand list.
   virtual void printGenericOp(Operation *op, bool printOpName = true) = 0;
 
   /// Prints a region.
   /// If 'printEntryBlockArgs' is false, the arguments of the
   /// block are not printed. If 'printBlockTerminator' is false, the terminator

mlir/lib/IR/AsmPrinter.cpp

 /// in the output, and trims down unnecessary output.
 class DummyAliasOperationPrinter : private OpAsmPrinter {
 public:
   explicit DummyAliasOperationPrinter(const OpPrintingFlags &printerFlags,
                                       AliasInitializer &initializer)
       : printerFlags(printerFlags), initializer(initializer) {}
 
   /// Print the given operation.
-  void print(Operation *op) {
+  void printOp(Operation *op) override {
     // Visit the operation location.
     if (printerFlags.shouldPrintDebugInfo())
       initializer.visit(op->getLoc(), /*canBeDeferred=*/true);
 
     // If requested, always print the generic form.
     if (!printerFlags.shouldPrintGenericOpForm()) {
       // Check to see if this is a known operation.  If so, use the registered
       // custom printer hook.
void print(Block *block, bool printBlockArgs = true,
     // requested.
     bool hasTerminator =
         !block->empty() && block->back().hasTrait<OpTrait::IsTerminator>();
     auto range = llvm::make_range(
         block->begin(),
         std::prev(block->end(),
                   (!hasTerminator || printBlockTerminator) ? 0 : 1));
     for (Operation &op : range)
-      print(&op);
+      printOp(&op);
   }
 
   /// Print the given region.
   void printRegion(Region &region, bool printEntryBlockArgs,
                    bool printBlockTerminators,
                    bool printEmptyBlock = false) override {
     if (region.empty())
       return;

 void AliasInitializer::initialize(
     Operation *op, const OpPrintingFlags &printerFlags,
     llvm::MapVector<Attribute, SymbolAlias> &attrToAlias,
     llvm::MapVector<Type, SymbolAlias> &typeToAlias) {
   // Use a dummy printer when walking the IR so that we can collect the
   // attributes/types that will actually be used during printing when
   // considering aliases.
   DummyAliasOperationPrinter aliasPrinter(printerFlags, *this);
-  aliasPrinter.print(op);
+  aliasPrinter.printOp(op);
 
   // Initialize the aliases sorted by name.
   initializeAliases(aliasToAttr, attrToAlias, &deferrableAttributes);
   initializeAliases(aliasToType, typeToAlias);
 }
 
 void AliasInitializer::visit(Attribute attr, bool canBeDeferred) {
   if (!visitedAttributes.insert(attr).second) {
public:
 
   /// Print the given top-level operation.
   void printTopLevelOperation(Operation *op);
 
   /// Print the given operation with its indent and location.
   void print(Operation *op);
   /// Print the bare location, not including indentation/location/etc.
   void printOperation(Operation *op);
+  /// Print the given operation in the custom or generic form.
+  void printOp(Operation *op) override;
   /// Print the given operation in the generic form.
   void printGenericOp(Operation *op, bool printOpName) override;
 
   /// Print the name of the given block.
   void printBlockName(Block *block);
 
   /// Print the given block. If 'printBlockArgs' is false, the arguments of the
   /// block are not printed. If 'printBlockTerminator' is false, the terminator
   /// operation of the block is not printed.
   void print(Block *block, bool printBlockArgs = true,
              bool printBlockTerminator = true);
if (size_t numResults = op->getNumResults()) {
 
     } else {
       printResultGroup(/*resultNo=*/0, /*resultCount=*/numResults);
     }
 
     os << " = ";
   }
 
-  // If requested, always print the generic form.
-  if (!printerFlags.shouldPrintGenericOpForm()) {
-    // Check to see if this is a known operation. If so, use the registered
-    // custom printer hook.
-    if (auto opInfo = op->getRegisteredInfo()) {
-      opInfo->printAssembly(op, *this, defaultDialectStack.back());
-      return;
-    }
-    // Otherwise try to dispatch to the dialect, if available.
-    if (Dialect *dialect = op->getDialect()) {
-      if (auto opPrinter = dialect->getOperationPrinter(op)) {
-        // Print the op name first.
-        StringRef name = op->getName().getStringRef();
-        // Only drop the default dialect prefix when it cannot lead to
-        // ambiguities.
-        if (name.count('.') == 1)
-          name.consume_front((defaultDialectStack.back() + ".").str());
-        os << name;
-
-        // Print the rest of the op now.
-        opPrinter(op, *this);
-        return;
-      }
-    }
-  }
-
-  // Otherwise print with the generic assembly form.
-  printGenericOp(op, /*printOpName=*/true);
+  printOp(op);
 }
 
 void OperationPrinter::printUsersComment(Operation *op) {
   unsigned numResults = op->getNumResults();
   if (!numResults && op->getNumOperands()) {
     os << " // id: ";
     printOperationID(op);
   } else if (numResults && op->use_empty()) {
void OperationPrinter::printUserIDs(Operation *user, bool prefixComma) {
   if (!user->getNumResults()) {
     printOperationID(user);
   } else {
     interleaveComma(user->getResults(),
                     [this](Value result) { printValueID(result); });
   }
 }
 
+void OperationPrinter::printOp(Operation *op) {
+  // If requested, always print the generic form.
+  if (!printerFlags.shouldPrintGenericOpForm()) {
+    // Check to see if this is a known operation. If so, use the registered
+    // custom printer hook.
+    if (auto opInfo = op->getRegisteredInfo()) {
+      opInfo->printAssembly(op, *this, defaultDialectStack.back());
+      return;
+    }
+    // Otherwise try to dispatch to the dialect, if available.
+    if (Dialect *dialect = op->getDialect()) {
+      if (auto opPrinter = dialect->getOperationPrinter(op)) {
+        // Print the op name first.
+        StringRef name = op->getName().getStringRef();
+        // Only drop the default dialect prefix when it cannot lead to
+        // ambiguities.
+        if (name.count('.') == 1)
+          name.consume_front((defaultDialectStack.back() + ".").str());
+        os << name;
+
+        // Print the rest of the op now.
+        opPrinter(op, *this);
+        return;
+      }
+    }
+  }
+
+  // Otherwise print with the generic assembly form.
+  printGenericOp(op, /*printOpName=*/true);
+}
+
 void OperationPrinter::printGenericOp(Operation *op, bool printOpName) {
   if (printOpName)
     printEscapedString(op->getName().getStringRef());
   os << '(';
   interleaveComma(op->getOperands(), [&](Value value) { printValueID(value); });
   os << ')';
 
   // For terminators, print the list of successors and their operands.