[Mlir-commits] [llvm] [mlir] [MLIR] Make generated markdown doc more consistent (PR #119926)

Kevin Gleason llvmlistbot at llvm.org
Fri Dec 13 13:19:45 PST 2024


https://github.com/GleasonK updated https://github.com/llvm/llvm-project/pull/119926

>From 2f027e4abe1fa783e9a81efc97dd95d7ea3b85e6 Mon Sep 17 00:00:00 2001
From: Kevin Gleason <gleasonk at google.com>
Date: Fri, 13 Dec 2024 18:59:45 +0000
Subject: [PATCH 1/3] [MLIR] Make generated markdown doc more consistent

---
 mlir/test/mlir-tblgen/gen-dialect-doc.td   |  2 +-
 mlir/tools/mlir-tblgen/OpDocGen.cpp        | 92 +++++++++++-----------
 mlir/tools/mlir-tblgen/OpInterfacesGen.cpp | 10 +--
 mlir/tools/mlir-tblgen/PassDocGen.cpp      |  6 +-
 4 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/mlir/test/mlir-tblgen/gen-dialect-doc.td b/mlir/test/mlir-tblgen/gen-dialect-doc.td
index 79d755111e8f67..eb462fc6510288 100644
--- a/mlir/test/mlir-tblgen/gen-dialect-doc.td
+++ b/mlir/test/mlir-tblgen/gen-dialect-doc.td
@@ -122,7 +122,7 @@ def TestEnum :
 
 // CHECK: ## Enums
 // CHECK: ### TestEnum
-// CHECK: enum summary
+// CHECK: _Enum summary_
 // CHECK: #### Cases:
 // CHECK: | Symbol | Value | String |
 // CHECK: | :----: | :---: | ------ |
diff --git a/mlir/tools/mlir-tblgen/OpDocGen.cpp b/mlir/tools/mlir-tblgen/OpDocGen.cpp
index d499c78a5cf44d..a6e92209c9c57c 100644
--- a/mlir/tools/mlir-tblgen/OpDocGen.cpp
+++ 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,6 +83,7 @@ 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());
@@ -87,22 +91,14 @@ void mlir::tblgen::emitDescriptionComment(StringRef description,
     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 << "«unnamed»";
-  os << " | " << it.constraint.getSummary() << "\n";
+    os << "| «unnamed»";
+  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,9 +297,9 @@ 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";
 }
 
 //===----------------------------------------------------------------------===//
@@ -309,9 +307,9 @@ static void emitAttrDoc(const Attribute &attr, raw_ostream &os) {
 //===----------------------------------------------------------------------===//
 
 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,11 +340,11 @@ 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())
@@ -354,7 +352,6 @@ static void emitAttrOrTypeDefDoc(const AttrOrTypeDef &def, raw_ostream &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,7 +443,7 @@ 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();
@@ -455,9 +451,9 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
           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);
diff --git a/mlir/tools/mlir-tblgen/OpInterfacesGen.cpp b/mlir/tools/mlir-tblgen/OpInterfacesGen.cpp
index 1f1b1d9a340391..dcd68e6c2d6366 100644
--- a/mlir/tools/mlir-tblgen/OpInterfacesGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpInterfacesGen.cpp
@@ -627,8 +627,8 @@ static void emitInterfaceDoc(const Record &interfaceDef, raw_ostream &os) {
   Interface interface(&interfaceDef);
 
   // Emit the interface name followed by the description.
-  os << "## " << interface.getName() << " (`" << interfaceDef.getName()
-     << "`)\n\n";
+  os << "\n## " << interface.getName() << " (`" << interfaceDef.getName()
+     << "`)\n";
   if (auto description = interface.getDescription())
     mlir::tblgen::emitDescription(*description, os);
 
@@ -636,7 +636,7 @@ static void emitInterfaceDoc(const Record &interfaceDef, raw_ostream &os) {
   os << "\n### Methods:\n";
   for (const auto &method : interface.getMethods()) {
     // Emit the method name.
-    os << "#### `" << method.getName() << "`\n\n```c++\n";
+    os << "\n#### `" << method.getName() << "`\n\n```c++\n";
 
     // Emit the method signature.
     if (method.isStatic())
@@ -656,13 +656,13 @@ static void emitInterfaceDoc(const Record &interfaceDef, raw_ostream &os) {
     if (!method.getBody())
       os << "\nNOTE: This method *must* be implemented by the user.";
 
-    os << "\n\n";
+    os << "\n";
   }
 }
 
 bool InterfaceGenerator::emitInterfaceDocs() {
   os << "<!-- Autogenerated by mlir-tblgen; don't manually edit -->\n";
-  os << "# " << interfaceBaseType << " definitions\n";
+  os << "\n# " << interfaceBaseType << " definitions\n";
 
   for (const auto *def : defs)
     emitInterfaceDoc(*def, os);
diff --git a/mlir/tools/mlir-tblgen/PassDocGen.cpp b/mlir/tools/mlir-tblgen/PassDocGen.cpp
index a2cb514ece3eba..456f9ceffeb9b2 100644
--- a/mlir/tools/mlir-tblgen/PassDocGen.cpp
+++ b/mlir/tools/mlir-tblgen/PassDocGen.cpp
@@ -22,14 +22,14 @@ using llvm::RecordKeeper;
 
 /// Emit the documentation for the given pass.
 static void emitDoc(const Pass &pass, raw_ostream &os) {
-  os << llvm::formatv("### `-{0}`\n", pass.getArgument());
+  os << llvm::formatv("\n### `-{0}`\n", pass.getArgument());
   emitSummary(pass.getSummary(), os);
   emitDescription(pass.getDescription(), os);
 
   // Handle the options of the pass.
   ArrayRef<PassOption> options = pass.getOptions();
   if (!options.empty()) {
-    os << "\n#### Options\n```\n";
+    os << "\n#### Options\n\n```\n";
     size_t longestOption = 0;
     for (const PassOption &option : options)
       longestOption = std::max(option.getArgument().size(), longestOption);
@@ -44,7 +44,7 @@ static void emitDoc(const Pass &pass, raw_ostream &os) {
   // Handle the statistics of the pass.
   ArrayRef<PassStatistic> stats = pass.getStatistics();
   if (!stats.empty()) {
-    os << "\n#### Statistics\n```\n";
+    os << "\n#### Statistics\n\n```\n";
     size_t longestStat = 0;
     for (const PassStatistic &stat : stats)
       longestStat = std::max(stat.getName().size(), longestStat);

>From f04be095f42290ec9c6d2bda8e71b6f152332818 Mon Sep 17 00:00:00 2001
From: Kevin Gleason <gleasonk at google.com>
Date: Fri, 13 Dec 2024 20:35:33 +0000
Subject: [PATCH 2/3] Add tests

---
 mlir/test/mlir-tblgen/gen-dialect-doc.td | 22 +++++++++++++++++++++-
 mlir/test/mlir-tblgen/gen-pass-doc.td    | 20 ++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 mlir/test/mlir-tblgen/gen-pass-doc.td

diff --git a/mlir/test/mlir-tblgen/gen-dialect-doc.td b/mlir/test/mlir-tblgen/gen-dialect-doc.td
index eb462fc6510288..06c90b702278eb 100644
--- a/mlir/test/mlir-tblgen/gen-dialect-doc.td
+++ b/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";
@@ -85,7 +93,19 @@ def TestEnum :
 // CHECK: [TOC]
 
 // CHECK-NOT: [TOC]
+
 // CHECK: test.e
+// CHECK: _Op with a summary_
+// CHECK: Op with a description
+// CHECK: Operands:
+// CHECK: | Operand | Description |
+// CHECK: | :-----: | ----------- |
+// CHECK: | `tensor` | |
+// CHECK: Results:
+// CHECK: | Result | Description |
+// CHECK: | :----: | ----------- |
+// CHECK: | `output` | |
+
 // CHECK: Group of ops
 // CHECK: test.a
 // CHECK: test.d
diff --git a/mlir/test/mlir-tblgen/gen-pass-doc.td b/mlir/test/mlir-tblgen/gen-pass-doc.td
new file mode 100644
index 00000000000000..6c3aeabc508457
--- /dev/null
+++ b/mlir/test/mlir-tblgen/gen-pass-doc.td
@@ -0,0 +1,20 @@
+// RUN: mlir-tblgen -gen-pass-doc -I %S/../../include -dialect=test %s | FileCheck %s
+
+include "mlir/Pass/PassBase.td"
+
+def TestPassDoc : Pass<"test-pass-doc"> {
+  let summary = "pass summary";
+  let description = [{
+    Pass description
+  }];
+
+  let options = [
+    ListOption<"option", "option", "std::string", "pass option">
+  ];
+}
+
+// CHECK: `-test-pass-doc`
+// CHECK: _Pass summary_
+// CHECK: Pass description
+// CHECK: Options
+// CHECK: -option : pass option

>From 298588014be602df6718876c46922c774161923a Mon Sep 17 00:00:00 2001
From: Kevin Gleason <gleasonk at google.com>
Date: Fri, 13 Dec 2024 21:19:30 +0000
Subject: [PATCH 3/3] Add Pass.td dep to bazel overlay

---
 .../bazel/llvm-project-overlay/mlir/test/mlir-tblgen/BUILD.bazel | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utils/bazel/llvm-project-overlay/mlir/test/mlir-tblgen/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/test/mlir-tblgen/BUILD.bazel
index 7fe1fdc8c6936d..1dd418c75984e7 100644
--- a/utils/bazel/llvm-project-overlay/mlir/test/mlir-tblgen/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/test/mlir-tblgen/BUILD.bazel
@@ -34,6 +34,7 @@ package(default_visibility = ["//visibility:public"])
             "//mlir:include/mlir/IR/OpBase.td",
             "//mlir:include/mlir/Interfaces/InferTypeOpInterface.td",
             "//mlir:include/mlir/Interfaces/SideEffectInterfaces.td",
+            "//mlir:include/mlir/Pass/PassBase.td",
             "//mlir:mlir-opt",
             "//mlir:mlir-tblgen",
             "//mlir/test:lit_data",



More information about the Mlir-commits mailing list