[MLIR] Make generated markdown doc more consistent

mlir/test/mlir-tblgen/gen-dialect-doc.td

@@ -36,7 +36,15 @@ def ACOp : Op<Test_Dialect, "c", [NoMemoryEffect, SingleBlockImplicitTerminator<
 def ABOp : Op<Test_Dialect, "b", [NoMemoryEffect, SingleBlockImplicitTerminator<"YieldOp">]>;
 }
 
-def AEOp : Op<Test_Dialect, "e", [NoMemoryEffect, SingleBlockImplicitTerminator<"YieldOp">]>;
+def AEOp : Op<Test_Dialect, "e", [NoMemoryEffect]> {
+  let summary = "Op with a summary";
+  let description = "Op with a description";
+  let arguments = (ins ConfinedType<AnyType, [CPred<"::llvm::isa<::mlir::TensorType>($_self)">]>:$tensor,
+                       I16Attr:$int_attr);
+  let results = (outs
+    ConfinedType<AnyType, [CPred<"::llvm::isa<::mlir::TensorType>($_self)">]>:$output
+  ); 
+}
 
 def TestAttr : DialectAttr<Test_Dialect, CPred<"true">> {
   let summary = "attribute summary";
@@ -81,11 +89,33 @@ def TestEnum :
 }
 
 // CHECK: Dialect without a [TOC] here.
-// CHECK: TOC added by tool.
-// CHECK: [TOC]
+// CHECK-NEXT:  TOC added by tool.
+// CHECK-EMPTY:
+// CHECK-NEXT:  [TOC]
+// CHECK-EMPTY:
 
 // CHECK-NOT: [TOC]
-// CHECK: test.e
+
+// CHECK:       test.e
+// CHECK-EMPTY:
+// CHECK-NEXT:  _Op with a summary_
+// CHECK-EMPTY:
+// CHECK-NEXT:  Op with a description
+// CHECK-EMPTY:
+
+// CHECK: Operands:
+// CHECK-EMPTY:
+// CHECK-NEXT:  | Operand | Description |
+// CHECK-NEXT:  | :-----: | ----------- |
+// CHECK-NEXT:  | `tensor` | |
+// CHECK-EMPTY:
+// CHECK-NEXT:  Results:
+// CHECK-EMPTY:
+// CHECK-NEXT:  | Result | Description |
+// CHECK-NEXT:  | :----: | ----------- |
+// CHECK-NEXT:  | `output` | |
+// CHECK-EMPTY:
+
 // CHECK: Group of ops
 // CHECK: test.a
 // CHECK: test.d
@@ -96,9 +126,11 @@ def TestEnum :
 // CHECK: Interfaces: `NoMemoryEffect (MemoryEffectOpInterface)`
 // CHECK: Effects: `MemoryEffects::Effect{}`
 
-// CHECK: ## Attribute constraints
-// CHECK: ### attribute summary
-// CHECK: attribute description
+// CHECK:       ## Attribute constraints
+// CHECK-EMPTY:
+// CHECK-NEXT:  ### attribute summary
+// CHECK-EMPTY:
+// CHECK:       attribute description
 
 // CHECK: TestAttrDefAttr
 // CHECK: Syntax:
@@ -120,15 +152,20 @@ def TestEnum :
 // CHECK: Syntax:
 // CHECK: !test.test_type_def_params
 
-// CHECK: ## Enums
-// CHECK: ### TestEnum
-// CHECK: enum summary
-// CHECK: #### Cases:
-// CHECK: | Symbol | Value | String |
-// CHECK: | :----: | :---: | ------ |
-// CHECK: | First | `0` | first |
-// CHECK: | Second | `1` | second |
-// CHECK: | Third | `2` | third |
+// CHECK:       ## Enums
+// CHECK-EMPTY:
+// CHECK-NEXT:  ### TestEnum
+// CHECK-EMPTY:
+// CHECK-NEXT:  _Enum summary_
+// CHECK-EMPTY:
+// CHECK-NEXT:  #### Cases:
+// CHECK-EMPTY:
+// CHECK-NEXT:  | Symbol | Value | String |
+// CHECK-NEXT:  | :----: | :---: | ------ |
+// CHECK-NEXT:  | First | `0` | first |
+// CHECK-NEXT:  | Second | `1` | second |
+// CHECK-NEXT:  | Third | `2` | third |
+// CHECK-EMPTY:
 
 def Toc_Dialect : Dialect {
   let name = "test_toc";

mlir/test/mlir-tblgen/gen-pass-doc.td

@@ -0,0 +1,33 @@
+// RUN: mlir-tblgen -gen-pass-doc -I %S/../../include -dialect=test %s | FileCheck %s
+
+include "mlir/Pass/PassBase.td"
+
+def TestPassDocA : Pass<"test-pass-doc-a"> {
+  let summary = "pass summary";
+  let description = [{
+    Pass description
+  }];
+
+  let options = [
+    ListOption<"option", "option", "std::string", "pass option">
+  ];
+}
+
+def TestPassDocB : Pass<"test-pass-doc-b"> {
+}
+
+// Ensure there are empty lines between individual pass docs.
+
+// CHECK:      `-test-pass-doc-a`
+// CHECK-EMPTY:
+// CHECK-NEXT:  _Pass summary_
+// CHECK-EMPTY:
+// CHECK-NEXT:  Pass description
+// CHECK-EMPTY:
+// CHECK-NEXT:  Options
+// CHECK-EMPTY:
+// CHECK-NEXT:  ```
+// CHECK-NEXT:  -option : pass option
+// CHECK-NEXT:  ```
+// CHECK-EMPTY:
+// CHECK-NEXT:  `-test-pass-doc-b`

mlir/tools/mlir-tblgen/OpDocGen.cpp

@@ -54,12 +54,12 @@ cl::opt<bool> allowHugoSpecificFeatures(
     cl::cat(docCat));
 
 void mlir::tblgen::emitSummary(StringRef summary, raw_ostream &os) {
-  if (!summary.empty()) {
-    StringRef trimmed = summary.trim();
-    char first = std::toupper(trimmed.front());
-    StringRef rest = trimmed.drop_front();
-    os << "\n_" << first << rest << "_\n\n";
-  }
+  if (summary.empty())
+    return;
+  StringRef trimmed = summary.trim();
+  char first = std::toupper(trimmed.front());
+  StringRef rest = trimmed.drop_front();
+  os << "\n_" << first << rest << "_\n";
 }
 
 // Emit the description by aligning the text to the left per line (e.g.,
@@ -69,6 +69,9 @@ void mlir::tblgen::emitSummary(StringRef summary, raw_ostream &os) {
 // in a way the user wanted but has some additional indenting due to being
 // nested in the op definition.
 void mlir::tblgen::emitDescription(StringRef description, raw_ostream &os) {
+  if (description.empty())
+    return;
+  os << "\n";
   raw_indented_ostream ros(os);
   StringRef trimmed = description.rtrim(" \t");
   ros.printReindented(trimmed);
@@ -80,29 +83,22 @@ void mlir::tblgen::emitDescriptionComment(StringRef description,
                                           raw_ostream &os, StringRef prefix) {
   if (description.empty())
     return;
+  os << "\n";
   raw_indented_ostream ros(os);
   StringRef trimmed = description.rtrim(" \t");
   ros.printReindented(trimmed, (Twine(prefix) + "/// ").str());
   if (!trimmed.ends_with("\n"))
     ros << "\n";
 }
 
-// Emits `str` with trailing newline if not empty.
-static void emitIfNotEmpty(StringRef str, raw_ostream &os) {
-  if (!str.empty()) {
-    emitDescription(str, os);
-    os << "\n";
-  }
-}
-
 /// Emit the given named constraint.
 template <typename T>
 static void emitNamedConstraint(const T &it, raw_ostream &os) {
   if (!it.name.empty())
     os << "| `" << it.name << "`";
   else
-    os << "&laquo;unnamed&raquo;";
-  os << " | " << it.constraint.getSummary() << "\n";
+    os << "| &laquo;unnamed&raquo;";
+  os << " | " << it.constraint.getSummary() << " |\n";
 }
 
 //===----------------------------------------------------------------------===//
@@ -112,6 +108,8 @@ static void emitNamedConstraint(const T &it, raw_ostream &os) {
 /// Emit the assembly format of an operation.
 static void emitAssemblyFormat(StringRef opName, StringRef format,
                                raw_ostream &os) {
+  if (format.empty())
+    return;
   os << "\nSyntax:\n\n```\noperation ::= `" << opName << "` ";
 
   // Print the assembly format aligned.
@@ -124,7 +122,7 @@ static void emitAssemblyFormat(StringRef opName, StringRef format,
     if (!formatChunk.empty())
       os.indent(indent) << formatChunk << "\n";
   } while (!split.second.empty());
-  os << "```\n\n";
+  os << "```\n";
 }
 
 /// Place `text` between backticks so that the Markdown processor renders it as
@@ -199,7 +197,7 @@ static void emitOpDoc(const Operator &op, raw_ostream &os) {
   std::string classNameStr = op.getQualCppClassName();
   StringRef className = classNameStr;
   (void)className.consume_front(stripPrefix);
-  os << formatv("### `{0}` ({1})\n", op.getOperationName(), className);
+  os << formatv("\n### `{0}` ({1})\n", op.getOperationName(), className);
 
   // Emit the summary, syntax, and description if present.
   if (op.hasSummary())
@@ -281,8 +279,8 @@ static void emitSourceLink(StringRef inputFilename, raw_ostream &os) {
 
   StringRef inputFromMlirInclude = inputFilename.substr(pathBegin);
 
-  os << "[source](https://github.com/llvm/llvm-project/blob/main/"
-     << inputFromMlirInclude << ")\n\n";
+  os << "\n[source](https://github.com/llvm/llvm-project/blob/main/"
+     << inputFromMlirInclude << ")\n";
 }
 
 static void emitOpDoc(const RecordKeeper &records, raw_ostream &os) {
@@ -299,19 +297,19 @@ static void emitOpDoc(const RecordKeeper &records, raw_ostream &os) {
 //===----------------------------------------------------------------------===//
 
 static void emitAttrDoc(const Attribute &attr, raw_ostream &os) {
-  os << "### " << attr.getSummary() << "\n\n";
+  os << "\n### " << attr.getSummary() << "\n";
   emitDescription(attr.getDescription(), os);
-  os << "\n\n";
+  os << "\n";
 }
 
 //===----------------------------------------------------------------------===//
 // Type Documentation
 //===----------------------------------------------------------------------===//
 
 static void emitTypeDoc(const Type &type, raw_ostream &os) {
-  os << "### " << type.getSummary() << "\n\n";
+  os << "\n### " << type.getSummary() << "\n";
   emitDescription(type.getDescription(), os);
-  os << "\n\n";
+  os << "\n";
 }
 
 //===----------------------------------------------------------------------===//
@@ -342,19 +340,18 @@ static void emitAttrOrTypeDefAssemblyFormat(const AttrOrTypeDef &def,
 }
 
 static void emitAttrOrTypeDefDoc(const AttrOrTypeDef &def, raw_ostream &os) {
-  os << formatv("### {0}\n", def.getCppClassName());
+  os << formatv("\n### {0}\n", def.getCppClassName());
 
   // Emit the summary if present.
   if (def.hasSummary())
-    os << "\n" << def.getSummary() << "\n";
+    emitSummary(def.getSummary(), os);
 
   // Emit the syntax if present.
   if (def.getMnemonic() && !def.hasCustomAssemblyFormat())
     emitAttrOrTypeDefAssemblyFormat(def, os);
 
   // Emit the description if present.
   if (def.hasDescription()) {
-    os << "\n";
     mlir::tblgen::emitDescription(def.getDescription(), os);
   }
 
@@ -363,11 +360,11 @@ static void emitAttrOrTypeDefDoc(const AttrOrTypeDef &def, raw_ostream &os) {
   if (!parameters.empty()) {
     os << "\n#### Parameters:\n\n";
     os << "| Parameter | C++ type | Description |\n"
-       << "| :-------: | :-------: | ----------- |\n";
+       << "| :-------: | :-------: | ----------- |";
     for (const auto &it : parameters) {
       auto desc = it.getSummary();
-      os << "| " << it.getName() << " | `" << it.getCppType() << "` | "
-         << (desc ? *desc : "") << " |\n";
+      os << "\n| " << it.getName() << " | `" << it.getCppType() << "` | "
+         << (desc ? *desc : "") << " |";
     }
   }
 
@@ -388,20 +385,19 @@ static void emitAttrOrTypeDefDoc(const RecordKeeper &records, raw_ostream &os,
 //===----------------------------------------------------------------------===//
 
 static void emitEnumDoc(const EnumAttr &def, raw_ostream &os) {
-  os << formatv("### {0}\n", def.getEnumClassName());
+  os << formatv("\n### {0}\n", def.getEnumClassName());
 
   // Emit the summary if present.
-  if (!def.getSummary().empty())
-    os << "\n" << def.getSummary() << "\n";
+  emitSummary(def.getSummary(), os);
 
   // Emit case documentation.
   std::vector<EnumAttrCase> cases = def.getAllCases();
   os << "\n#### Cases:\n\n";
   os << "| Symbol | Value | String |\n"
-     << "| :----: | :---: | ------ |\n";
+     << "| :----: | :---: | ------ |";
   for (const auto &it : cases) {
-    os << "| " << it.getSymbol() << " | `" << it.getValue() << "` | "
-       << it.getStr() << " |\n";
+    os << "\n| " << it.getSymbol() << " | `" << it.getValue() << "` | "
+       << it.getStr() << " |";
   }
 
   os << "\n";
@@ -447,17 +443,17 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
                       ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
                       ArrayRef<EnumAttr> enums, raw_ostream &os) {
   if (!ops.empty()) {
-    os << "## Operations\n\n";
+    os << "\n## Operations\n";
     emitSourceLink(inputFilename, os);
     for (const OpDocGroup &grouping : ops) {
       bool nested = !grouping.summary.empty();
       maybeNest(
           nested,
           [&](raw_ostream &os) {
             if (nested) {
-              os << "## " << StringRef(grouping.summary).trim() << "\n\n";
+              os << "\n## " << StringRef(grouping.summary).trim() << "\n";
               emitDescription(grouping.description, os);
-              os << "\n\n";
+              os << "\n";
             }
             for (const Operator &op : grouping.ops) {
               emitOpDoc(op, os);
@@ -468,32 +464,32 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
   }
 
   if (!attributes.empty()) {
-    os << "## Attribute constraints\n\n";
+    os << "\n## Attribute constraints\n";
     for (const Attribute &attr : attributes)
       emitAttrDoc(attr, os);
   }
 
   if (!attrDefs.empty()) {
-    os << "## Attributes\n\n";
+    os << "\n## Attributes\n";
     for (const AttrDef &def : attrDefs)
       emitAttrOrTypeDefDoc(def, os);
   }
 
   // TODO: Add link between use and def for types
   if (!types.empty()) {
-    os << "## Type constraints\n\n";
+    os << "\n## Type constraints\n";
     for (const Type &type : types)
       emitTypeDoc(type, os);
   }
 
   if (!typeDefs.empty()) {
-    os << "## Types\n\n";
+    os << "\n## Types\n";
     for (const TypeDef &def : typeDefs)
       emitAttrOrTypeDefDoc(def, os);
   }
 
   if (!enums.empty()) {
-    os << "## Enums\n\n";
+    os << "\n## Enums\n";
     for (const EnumAttr &def : enums)
       emitEnumDoc(def, os);
   }
@@ -504,14 +500,14 @@ static void emitDialectDoc(const Dialect &dialect, StringRef inputFilename,
                            ArrayRef<AttrDef> attrDefs, ArrayRef<OpDocGroup> ops,
                            ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
                            ArrayRef<EnumAttr> enums, raw_ostream &os) {
-  os << "# '" << dialect.getName() << "' Dialect\n\n";
-  emitIfNotEmpty(dialect.getSummary(), os);
-  emitIfNotEmpty(dialect.getDescription(), os);
+  os << "\n# '" << dialect.getName() << "' Dialect\n";
+  emitSummary(dialect.getSummary(), os);
+  emitDescription(dialect.getDescription(), os);
 
   // Generate a TOC marker except if description already contains one.
   Regex r("^[[:space:]]*\\[TOC\\]$", Regex::RegexFlags::Newline);
   if (!r.match(dialect.getDescription()))
-    os << "[TOC]\n\n";
+    os << "\n[TOC]\n";
 
   emitBlock(attributes, inputFilename, attrDefs, ops, types, typeDefs, enums,
             os);