Skip to content

Commit 2cf6663

Browse files
authored
[MLIR] Make generated markdown doc more consistent (#119926)
A few changes to doc generation: - All summaries are in italics. - In general each optional block starts and ends with a newline. - All table elements are enclosed in `|`'s - Overall reduce the number of >2newlines in a row Rationale for this change is that our markdown to docs generator requires a newline before all headers, otherwise it gets inlined into the line before it, see `### sdy-op-priority-propagate` in the image below. <img width="883" alt="image" src="https://github.com/user-attachments/assets/b795c424-cecb-48df-abbe-aee2030f4491" /> That said overall I feel this formatting is more consistent now, here's a before and after: - Dialect documentation diff: https://www.diffchecker.com/OVMHoXeL/ - Pass documentation diff: https://www.diffchecker.com/XEJRmW3k/
1 parent c2fea0d commit 2cf6663

File tree

6 files changed

+139
-72
lines changed

6 files changed

+139
-72
lines changed

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

+53-16
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,15 @@ def ACOp : Op<Test_Dialect, "c", [NoMemoryEffect, SingleBlockImplicitTerminator<
3636
def ABOp : Op<Test_Dialect, "b", [NoMemoryEffect, SingleBlockImplicitTerminator<"YieldOp">]>;
3737
}
3838

39-
def AEOp : Op<Test_Dialect, "e", [NoMemoryEffect, SingleBlockImplicitTerminator<"YieldOp">]>;
39+
def AEOp : Op<Test_Dialect, "e", [NoMemoryEffect]> {
40+
let summary = "Op with a summary";
41+
let description = "Op with a description";
42+
let arguments = (ins ConfinedType<AnyType, [CPred<"::llvm::isa<::mlir::TensorType>($_self)">]>:$tensor,
43+
I16Attr:$int_attr);
44+
let results = (outs
45+
ConfinedType<AnyType, [CPred<"::llvm::isa<::mlir::TensorType>($_self)">]>:$output
46+
);
47+
}
4048

4149
def TestAttr : DialectAttr<Test_Dialect, CPred<"true">> {
4250
let summary = "attribute summary";
@@ -81,11 +89,33 @@ def TestEnum :
8189
}
8290

8391
// CHECK: Dialect without a [TOC] here.
84-
// CHECK: TOC added by tool.
85-
// CHECK: [TOC]
92+
// CHECK-NEXT: TOC added by tool.
93+
// CHECK-EMPTY:
94+
// CHECK-NEXT: [TOC]
95+
// CHECK-EMPTY:
8696

8797
// CHECK-NOT: [TOC]
88-
// CHECK: test.e
98+
99+
// CHECK: test.e
100+
// CHECK-EMPTY:
101+
// CHECK-NEXT: _Op with a summary_
102+
// CHECK-EMPTY:
103+
// CHECK-NEXT: Op with a description
104+
// CHECK-EMPTY:
105+
106+
// CHECK: Operands:
107+
// CHECK-EMPTY:
108+
// CHECK-NEXT: | Operand | Description |
109+
// CHECK-NEXT: | :-----: | ----------- |
110+
// CHECK-NEXT: | `tensor` | |
111+
// CHECK-EMPTY:
112+
// CHECK-NEXT: Results:
113+
// CHECK-EMPTY:
114+
// CHECK-NEXT: | Result | Description |
115+
// CHECK-NEXT: | :----: | ----------- |
116+
// CHECK-NEXT: | `output` | |
117+
// CHECK-EMPTY:
118+
89119
// CHECK: Group of ops
90120
// CHECK: test.a
91121
// CHECK: test.d
@@ -96,9 +126,11 @@ def TestEnum :
96126
// CHECK: Interfaces: `NoMemoryEffect (MemoryEffectOpInterface)`
97127
// CHECK: Effects: `MemoryEffects::Effect{}`
98128

99-
// CHECK: ## Attribute constraints
100-
// CHECK: ### attribute summary
101-
// CHECK: attribute description
129+
// CHECK: ## Attribute constraints
130+
// CHECK-EMPTY:
131+
// CHECK-NEXT: ### attribute summary
132+
// CHECK-EMPTY:
133+
// CHECK: attribute description
102134

103135
// CHECK: TestAttrDefAttr
104136
// CHECK: Syntax:
@@ -120,15 +152,20 @@ def TestEnum :
120152
// CHECK: Syntax:
121153
// CHECK: !test.test_type_def_params
122154

123-
// CHECK: ## Enums
124-
// CHECK: ### TestEnum
125-
// CHECK: enum summary
126-
// CHECK: #### Cases:
127-
// CHECK: | Symbol | Value | String |
128-
// CHECK: | :----: | :---: | ------ |
129-
// CHECK: | First | `0` | first |
130-
// CHECK: | Second | `1` | second |
131-
// CHECK: | Third | `2` | third |
155+
// CHECK: ## Enums
156+
// CHECK-EMPTY:
157+
// CHECK-NEXT: ### TestEnum
158+
// CHECK-EMPTY:
159+
// CHECK-NEXT: _Enum summary_
160+
// CHECK-EMPTY:
161+
// CHECK-NEXT: #### Cases:
162+
// CHECK-EMPTY:
163+
// CHECK-NEXT: | Symbol | Value | String |
164+
// CHECK-NEXT: | :----: | :---: | ------ |
165+
// CHECK-NEXT: | First | `0` | first |
166+
// CHECK-NEXT: | Second | `1` | second |
167+
// CHECK-NEXT: | Third | `2` | third |
168+
// CHECK-EMPTY:
132169

133170
def Toc_Dialect : Dialect {
134171
let name = "test_toc";

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

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: mlir-tblgen -gen-pass-doc -I %S/../../include -dialect=test %s | FileCheck %s
2+
3+
include "mlir/Pass/PassBase.td"
4+
5+
def TestPassDocA : Pass<"test-pass-doc-a"> {
6+
let summary = "pass summary";
7+
let description = [{
8+
Pass description
9+
}];
10+
11+
let options = [
12+
ListOption<"option", "option", "std::string", "pass option">
13+
];
14+
}
15+
16+
def TestPassDocB : Pass<"test-pass-doc-b"> {
17+
}
18+
19+
// Ensure there are empty lines between individual pass docs.
20+
21+
// CHECK: `-test-pass-doc-a`
22+
// CHECK-EMPTY:
23+
// CHECK-NEXT: _Pass summary_
24+
// CHECK-EMPTY:
25+
// CHECK-NEXT: Pass description
26+
// CHECK-EMPTY:
27+
// CHECK-NEXT: Options
28+
// CHECK-EMPTY:
29+
// CHECK-NEXT: ```
30+
// CHECK-NEXT: -option : pass option
31+
// CHECK-NEXT: ```
32+
// CHECK-EMPTY:
33+
// CHECK-NEXT: `-test-pass-doc-b`

mlir/tools/mlir-tblgen/OpDocGen.cpp

+44-48
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ cl::opt<bool> allowHugoSpecificFeatures(
5454
cl::cat(docCat));
5555

5656
void mlir::tblgen::emitSummary(StringRef summary, raw_ostream &os) {
57-
if (!summary.empty()) {
58-
StringRef trimmed = summary.trim();
59-
char first = std::toupper(trimmed.front());
60-
StringRef rest = trimmed.drop_front();
61-
os << "\n_" << first << rest << "_\n\n";
62-
}
57+
if (summary.empty())
58+
return;
59+
StringRef trimmed = summary.trim();
60+
char first = std::toupper(trimmed.front());
61+
StringRef rest = trimmed.drop_front();
62+
os << "\n_" << first << rest << "_\n";
6363
}
6464

6565
// 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) {
6969
// in a way the user wanted but has some additional indenting due to being
7070
// nested in the op definition.
7171
void mlir::tblgen::emitDescription(StringRef description, raw_ostream &os) {
72+
if (description.empty())
73+
return;
74+
os << "\n";
7275
raw_indented_ostream ros(os);
7376
StringRef trimmed = description.rtrim(" \t");
7477
ros.printReindented(trimmed);
@@ -80,29 +83,22 @@ void mlir::tblgen::emitDescriptionComment(StringRef description,
8083
raw_ostream &os, StringRef prefix) {
8184
if (description.empty())
8285
return;
86+
os << "\n";
8387
raw_indented_ostream ros(os);
8488
StringRef trimmed = description.rtrim(" \t");
8589
ros.printReindented(trimmed, (Twine(prefix) + "/// ").str());
8690
if (!trimmed.ends_with("\n"))
8791
ros << "\n";
8892
}
8993

90-
// Emits `str` with trailing newline if not empty.
91-
static void emitIfNotEmpty(StringRef str, raw_ostream &os) {
92-
if (!str.empty()) {
93-
emitDescription(str, os);
94-
os << "\n";
95-
}
96-
}
97-
9894
/// Emit the given named constraint.
9995
template <typename T>
10096
static void emitNamedConstraint(const T &it, raw_ostream &os) {
10197
if (!it.name.empty())
10298
os << "| `" << it.name << "`";
10399
else
104-
os << "&laquo;unnamed&raquo;";
105-
os << " | " << it.constraint.getSummary() << "\n";
100+
os << "| &laquo;unnamed&raquo;";
101+
os << " | " << it.constraint.getSummary() << " |\n";
106102
}
107103

108104
//===----------------------------------------------------------------------===//
@@ -112,6 +108,8 @@ static void emitNamedConstraint(const T &it, raw_ostream &os) {
112108
/// Emit the assembly format of an operation.
113109
static void emitAssemblyFormat(StringRef opName, StringRef format,
114110
raw_ostream &os) {
111+
if (format.empty())
112+
return;
115113
os << "\nSyntax:\n\n```\noperation ::= `" << opName << "` ";
116114

117115
// Print the assembly format aligned.
@@ -124,7 +122,7 @@ static void emitAssemblyFormat(StringRef opName, StringRef format,
124122
if (!formatChunk.empty())
125123
os.indent(indent) << formatChunk << "\n";
126124
} while (!split.second.empty());
127-
os << "```\n\n";
125+
os << "```\n";
128126
}
129127

130128
/// 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) {
199197
std::string classNameStr = op.getQualCppClassName();
200198
StringRef className = classNameStr;
201199
(void)className.consume_front(stripPrefix);
202-
os << formatv("### `{0}` ({1})\n", op.getOperationName(), className);
200+
os << formatv("\n### `{0}` ({1})\n", op.getOperationName(), className);
203201

204202
// Emit the summary, syntax, and description if present.
205203
if (op.hasSummary())
@@ -281,8 +279,8 @@ static void emitSourceLink(StringRef inputFilename, raw_ostream &os) {
281279

282280
StringRef inputFromMlirInclude = inputFilename.substr(pathBegin);
283281

284-
os << "[source](https://github.com/llvm/llvm-project/blob/main/"
285-
<< inputFromMlirInclude << ")\n\n";
282+
os << "\n[source](https://github.com/llvm/llvm-project/blob/main/"
283+
<< inputFromMlirInclude << ")\n";
286284
}
287285

288286
static void emitOpDoc(const RecordKeeper &records, raw_ostream &os) {
@@ -299,19 +297,19 @@ static void emitOpDoc(const RecordKeeper &records, raw_ostream &os) {
299297
//===----------------------------------------------------------------------===//
300298

301299
static void emitAttrDoc(const Attribute &attr, raw_ostream &os) {
302-
os << "### " << attr.getSummary() << "\n\n";
300+
os << "\n### " << attr.getSummary() << "\n";
303301
emitDescription(attr.getDescription(), os);
304-
os << "\n\n";
302+
os << "\n";
305303
}
306304

307305
//===----------------------------------------------------------------------===//
308306
// Type Documentation
309307
//===----------------------------------------------------------------------===//
310308

311309
static void emitTypeDoc(const Type &type, raw_ostream &os) {
312-
os << "### " << type.getSummary() << "\n\n";
310+
os << "\n### " << type.getSummary() << "\n";
313311
emitDescription(type.getDescription(), os);
314-
os << "\n\n";
312+
os << "\n";
315313
}
316314

317315
//===----------------------------------------------------------------------===//
@@ -342,19 +340,18 @@ static void emitAttrOrTypeDefAssemblyFormat(const AttrOrTypeDef &def,
342340
}
343341

344342
static void emitAttrOrTypeDefDoc(const AttrOrTypeDef &def, raw_ostream &os) {
345-
os << formatv("### {0}\n", def.getCppClassName());
343+
os << formatv("\n### {0}\n", def.getCppClassName());
346344

347345
// Emit the summary if present.
348346
if (def.hasSummary())
349-
os << "\n" << def.getSummary() << "\n";
347+
emitSummary(def.getSummary(), os);
350348

351349
// Emit the syntax if present.
352350
if (def.getMnemonic() && !def.hasCustomAssemblyFormat())
353351
emitAttrOrTypeDefAssemblyFormat(def, os);
354352

355353
// Emit the description if present.
356354
if (def.hasDescription()) {
357-
os << "\n";
358355
mlir::tblgen::emitDescription(def.getDescription(), os);
359356
}
360357

@@ -363,11 +360,11 @@ static void emitAttrOrTypeDefDoc(const AttrOrTypeDef &def, raw_ostream &os) {
363360
if (!parameters.empty()) {
364361
os << "\n#### Parameters:\n\n";
365362
os << "| Parameter | C++ type | Description |\n"
366-
<< "| :-------: | :-------: | ----------- |\n";
363+
<< "| :-------: | :-------: | ----------- |";
367364
for (const auto &it : parameters) {
368365
auto desc = it.getSummary();
369-
os << "| " << it.getName() << " | `" << it.getCppType() << "` | "
370-
<< (desc ? *desc : "") << " |\n";
366+
os << "\n| " << it.getName() << " | `" << it.getCppType() << "` | "
367+
<< (desc ? *desc : "") << " |";
371368
}
372369
}
373370

@@ -388,20 +385,19 @@ static void emitAttrOrTypeDefDoc(const RecordKeeper &records, raw_ostream &os,
388385
//===----------------------------------------------------------------------===//
389386

390387
static void emitEnumDoc(const EnumAttr &def, raw_ostream &os) {
391-
os << formatv("### {0}\n", def.getEnumClassName());
388+
os << formatv("\n### {0}\n", def.getEnumClassName());
392389

393390
// Emit the summary if present.
394-
if (!def.getSummary().empty())
395-
os << "\n" << def.getSummary() << "\n";
391+
emitSummary(def.getSummary(), os);
396392

397393
// Emit case documentation.
398394
std::vector<EnumAttrCase> cases = def.getAllCases();
399395
os << "\n#### Cases:\n\n";
400396
os << "| Symbol | Value | String |\n"
401-
<< "| :----: | :---: | ------ |\n";
397+
<< "| :----: | :---: | ------ |";
402398
for (const auto &it : cases) {
403-
os << "| " << it.getSymbol() << " | `" << it.getValue() << "` | "
404-
<< it.getStr() << " |\n";
399+
os << "\n| " << it.getSymbol() << " | `" << it.getValue() << "` | "
400+
<< it.getStr() << " |";
405401
}
406402

407403
os << "\n";
@@ -447,17 +443,17 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
447443
ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
448444
ArrayRef<EnumAttr> enums, raw_ostream &os) {
449445
if (!ops.empty()) {
450-
os << "## Operations\n\n";
446+
os << "\n## Operations\n";
451447
emitSourceLink(inputFilename, os);
452448
for (const OpDocGroup &grouping : ops) {
453449
bool nested = !grouping.summary.empty();
454450
maybeNest(
455451
nested,
456452
[&](raw_ostream &os) {
457453
if (nested) {
458-
os << "## " << StringRef(grouping.summary).trim() << "\n\n";
454+
os << "\n## " << StringRef(grouping.summary).trim() << "\n";
459455
emitDescription(grouping.description, os);
460-
os << "\n\n";
456+
os << "\n";
461457
}
462458
for (const Operator &op : grouping.ops) {
463459
emitOpDoc(op, os);
@@ -468,32 +464,32 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
468464
}
469465

470466
if (!attributes.empty()) {
471-
os << "## Attribute constraints\n\n";
467+
os << "\n## Attribute constraints\n";
472468
for (const Attribute &attr : attributes)
473469
emitAttrDoc(attr, os);
474470
}
475471

476472
if (!attrDefs.empty()) {
477-
os << "## Attributes\n\n";
473+
os << "\n## Attributes\n";
478474
for (const AttrDef &def : attrDefs)
479475
emitAttrOrTypeDefDoc(def, os);
480476
}
481477

482478
// TODO: Add link between use and def for types
483479
if (!types.empty()) {
484-
os << "## Type constraints\n\n";
480+
os << "\n## Type constraints\n";
485481
for (const Type &type : types)
486482
emitTypeDoc(type, os);
487483
}
488484

489485
if (!typeDefs.empty()) {
490-
os << "## Types\n\n";
486+
os << "\n## Types\n";
491487
for (const TypeDef &def : typeDefs)
492488
emitAttrOrTypeDefDoc(def, os);
493489
}
494490

495491
if (!enums.empty()) {
496-
os << "## Enums\n\n";
492+
os << "\n## Enums\n";
497493
for (const EnumAttr &def : enums)
498494
emitEnumDoc(def, os);
499495
}
@@ -504,14 +500,14 @@ static void emitDialectDoc(const Dialect &dialect, StringRef inputFilename,
504500
ArrayRef<AttrDef> attrDefs, ArrayRef<OpDocGroup> ops,
505501
ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
506502
ArrayRef<EnumAttr> enums, raw_ostream &os) {
507-
os << "# '" << dialect.getName() << "' Dialect\n\n";
508-
emitIfNotEmpty(dialect.getSummary(), os);
509-
emitIfNotEmpty(dialect.getDescription(), os);
503+
os << "\n# '" << dialect.getName() << "' Dialect\n";
504+
emitSummary(dialect.getSummary(), os);
505+
emitDescription(dialect.getDescription(), os);
510506

511507
// Generate a TOC marker except if description already contains one.
512508
Regex r("^[[:space:]]*\\[TOC\\]$", Regex::RegexFlags::Newline);
513509
if (!r.match(dialect.getDescription()))
514-
os << "[TOC]\n\n";
510+
os << "\n[TOC]\n";
515511

516512
emitBlock(attributes, inputFilename, attrDefs, ops, types, typeDefs, enums,
517513
os);

0 commit comments

Comments
 (0)