[mlir][docgen] Emit OpInterface for Operations within Dialect Doc #104693
Conversation
This only emits the OpInterfaces which are used Operations defined in a specific Dialect.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-mlir-core Author: Ammar (ammar2199) ChangesThis only emits the OpInterfaces which are used Operations defined Full diff: https://github.com/llvm/llvm-project/pull/104693.diff 1 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/OpDocGen.cpp b/mlir/tools/mlir-tblgen/OpDocGen.cpp
index 71df80cd110f15..9639345ad815ae 100644
--- a/mlir/tools/mlir-tblgen/OpDocGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDocGen.cpp
@@ -18,6 +18,7 @@
#include "mlir/TableGen/AttrOrTypeDef.h"
#include "mlir/TableGen/Attribute.h"
#include "mlir/TableGen/GenInfo.h"
+#include "mlir/TableGen/Interfaces.h"
#include "mlir/TableGen/Operator.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SetVector.h"
@@ -229,7 +230,8 @@ static void emitOpDoc(const Operator &op, raw_ostream &os) {
// Expandable description.
// This appears as just the summary, but when clicked shows the full
// description.
- os << "<details>" << "<summary>" << it.attr.getSummary() << "</summary>"
+ os << "<details>"
+ << "<summary>" << it.attr.getSummary() << "</summary>"
<< "{{% markdown %}}" << description << "{{% /markdown %}}"
<< "</details>";
} else {
@@ -381,6 +383,51 @@ static void emitAttrOrTypeDefDoc(const RecordKeeper &recordKeeper,
emitAttrOrTypeDefDoc(AttrOrTypeDef(def), os);
}
+//===----------------------------------------------------------------------===//
+// OpInterface Documentation
+//===----------------------------------------------------------------------===//
+
+static void emitOpInterfaceDoc(const Interface &opIf, raw_ostream &os) {
+ os << llvm::formatv("### {0}\n", opIf.getName());
+
+ ArrayRef<std::pair<Record *, SMRange>> superclasses =
+ opIf.getDef().getSuperClasses();
+ if (!superclasses.empty()) {
+ llvm::interleaveComma(superclasses, os << "Base Classes: ",
+ [&](const std::pair<Record *, SMRange> &p) {
+ os << p.first->getName();
+ });
+ os << "\n\n";
+ }
+
+ auto descriptionOpt = opIf.getDescription();
+ if (descriptionOpt) {
+ os << "#### Description:\n\n";
+ emitDescription(descriptionOpt.value(), os);
+ }
+
+ ArrayRef<InterfaceMethod> methods = opIf.getMethods();
+ if (!methods.empty()) {
+ os << "#### Methods:\n\n";
+ for (auto &method : methods) {
+ os << llvm::formatv("**{0}**\n\n", method.getName());
+ descriptionOpt = method.getDescription();
+ if (descriptionOpt) {
+ os << "Description: \n\n";
+ emitDescription(descriptionOpt.value(), os);
+ os << "\n";
+ }
+ os << llvm::formatv("Return Type: {0}\n\n", method.getReturnType());
+ if (!method.getArguments().empty()) {
+ llvm::interleaveComma(
+ method.getArguments(), os << "Arguments:\n",
+ [&](const InterfaceMethod::Argument &arg) { os << arg.type; });
+ }
+ os << "\n\n";
+ }
+ }
+}
+
//===----------------------------------------------------------------------===//
// Enum Documentation
//===----------------------------------------------------------------------===//
@@ -446,7 +493,9 @@ static void maybeNest(bool nest, llvm::function_ref<void(raw_ostream &os)> fn,
static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
ArrayRef<AttrDef> attrDefs, ArrayRef<OpDocGroup> ops,
ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
- ArrayRef<EnumAttr> enums, raw_ostream &os) {
+ ArrayRef<EnumAttr> enums,
+ ArrayRef<Interface> relevantOpInterfaces,
+ raw_ostream &os) {
if (!ops.empty()) {
os << "## Operations\n\n";
emitSourceLink(inputFilename, os);
@@ -498,13 +547,24 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
for (const EnumAttr &def : enums)
emitEnumDoc(def, os);
}
+
+ if (!relevantOpInterfaces.empty()) {
+ os << "## Op Interfaces\n\n";
+ os << "**The following Op Interfaces are used by the Operations in this "
+ "Dialect**\n";
+ for (const Interface &interface : relevantOpInterfaces) {
+ emitOpInterfaceDoc(interface, os);
+ }
+ }
}
static void emitDialectDoc(const Dialect &dialect, StringRef inputFilename,
ArrayRef<Attribute> attributes,
ArrayRef<AttrDef> attrDefs, ArrayRef<OpDocGroup> ops,
ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
- ArrayRef<EnumAttr> enums, raw_ostream &os) {
+ ArrayRef<EnumAttr> enums,
+ ArrayRef<Interface> relevantOpInterfaces,
+ raw_ostream &os) {
os << "# '" << dialect.getName() << "' Dialect\n\n";
emitIfNotEmpty(dialect.getSummary(), os);
emitIfNotEmpty(dialect.getDescription(), os);
@@ -515,7 +575,7 @@ static void emitDialectDoc(const Dialect &dialect, StringRef inputFilename,
os << "[TOC]\n\n";
emitBlock(attributes, inputFilename, attrDefs, ops, types, typeDefs, enums,
- os);
+ relevantOpInterfaces, os);
}
static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
@@ -544,16 +604,19 @@ static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
std::vector<Type> dialectTypes;
std::vector<TypeDef> dialectTypeDefs;
std::vector<EnumAttr> dialectEnums;
+ std::vector<Interface> relevantOpInterfaces;
- llvm::SmallDenseSet<Record *> seen;
- auto addIfNotSeen = [&](llvm::Record *record, const auto &def, auto &vec) {
+ llvm::SmallDenseSet<const Record *> seen;
+ auto addIfNotSeen = [&](const llvm::Record *record, const auto &def,
+ auto &vec) {
if (seen.insert(record).second) {
vec.push_back(def);
return true;
}
return false;
};
- auto addIfInDialect = [&](llvm::Record *record, const auto &def, auto &vec) {
+ auto addIfInDialect = [&](const llvm::Record *record, const auto &def,
+ auto &vec) {
return def.getDialect() == *dialect && addIfNotSeen(record, def, vec);
};
@@ -589,6 +652,19 @@ static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
for (Record *def : enumDefs)
addIfNotSeen(def, EnumAttr(def), dialectEnums);
+ for (Record *def : opDefs) {
+ Operator o(def);
+ for (auto &trait : o.getTraits()) {
+ if (const InterfaceTrait *ifTrait = dyn_cast<InterfaceTrait>(&trait)) {
+ if (trait.getDef().isSubClassOf("SideEffectsTraitBase")) {
+ continue;
+ }
+ addIfNotSeen(&ifTrait->getDef(), ifTrait->getInterface(),
+ relevantOpInterfaces);
+ }
+ }
+ }
+
// Sort alphabetically ignorning dialect for ops and section name for
// sections.
// TODO: The sorting order could be revised, currently attempting to sort of
@@ -606,7 +682,7 @@ static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
os << "<!-- Autogenerated by mlir-tblgen; don't manually edit -->\n";
emitDialectDoc(*dialect, recordKeeper.getInputFilename(), dialectAttrs,
dialectAttrDefs, dialectOps, dialectTypes, dialectTypeDefs,
- dialectEnums, os);
+ dialectEnums, relevantOpInterfaces, os);
return false;
}
|
|
@llvm/pr-subscribers-mlir Author: Ammar (ammar2199) ChangesThis only emits the OpInterfaces which are used Operations defined Full diff: https://github.com/llvm/llvm-project/pull/104693.diff 1 Files Affected:
diff --git a/mlir/tools/mlir-tblgen/OpDocGen.cpp b/mlir/tools/mlir-tblgen/OpDocGen.cpp
index 71df80cd110f15..9639345ad815ae 100644
--- a/mlir/tools/mlir-tblgen/OpDocGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDocGen.cpp
@@ -18,6 +18,7 @@
#include "mlir/TableGen/AttrOrTypeDef.h"
#include "mlir/TableGen/Attribute.h"
#include "mlir/TableGen/GenInfo.h"
+#include "mlir/TableGen/Interfaces.h"
#include "mlir/TableGen/Operator.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SetVector.h"
@@ -229,7 +230,8 @@ static void emitOpDoc(const Operator &op, raw_ostream &os) {
// Expandable description.
// This appears as just the summary, but when clicked shows the full
// description.
- os << "<details>" << "<summary>" << it.attr.getSummary() << "</summary>"
+ os << "<details>"
+ << "<summary>" << it.attr.getSummary() << "</summary>"
<< "{{% markdown %}}" << description << "{{% /markdown %}}"
<< "</details>";
} else {
@@ -381,6 +383,51 @@ static void emitAttrOrTypeDefDoc(const RecordKeeper &recordKeeper,
emitAttrOrTypeDefDoc(AttrOrTypeDef(def), os);
}
+//===----------------------------------------------------------------------===//
+// OpInterface Documentation
+//===----------------------------------------------------------------------===//
+
+static void emitOpInterfaceDoc(const Interface &opIf, raw_ostream &os) {
+ os << llvm::formatv("### {0}\n", opIf.getName());
+
+ ArrayRef<std::pair<Record *, SMRange>> superclasses =
+ opIf.getDef().getSuperClasses();
+ if (!superclasses.empty()) {
+ llvm::interleaveComma(superclasses, os << "Base Classes: ",
+ [&](const std::pair<Record *, SMRange> &p) {
+ os << p.first->getName();
+ });
+ os << "\n\n";
+ }
+
+ auto descriptionOpt = opIf.getDescription();
+ if (descriptionOpt) {
+ os << "#### Description:\n\n";
+ emitDescription(descriptionOpt.value(), os);
+ }
+
+ ArrayRef<InterfaceMethod> methods = opIf.getMethods();
+ if (!methods.empty()) {
+ os << "#### Methods:\n\n";
+ for (auto &method : methods) {
+ os << llvm::formatv("**{0}**\n\n", method.getName());
+ descriptionOpt = method.getDescription();
+ if (descriptionOpt) {
+ os << "Description: \n\n";
+ emitDescription(descriptionOpt.value(), os);
+ os << "\n";
+ }
+ os << llvm::formatv("Return Type: {0}\n\n", method.getReturnType());
+ if (!method.getArguments().empty()) {
+ llvm::interleaveComma(
+ method.getArguments(), os << "Arguments:\n",
+ [&](const InterfaceMethod::Argument &arg) { os << arg.type; });
+ }
+ os << "\n\n";
+ }
+ }
+}
+
//===----------------------------------------------------------------------===//
// Enum Documentation
//===----------------------------------------------------------------------===//
@@ -446,7 +493,9 @@ static void maybeNest(bool nest, llvm::function_ref<void(raw_ostream &os)> fn,
static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
ArrayRef<AttrDef> attrDefs, ArrayRef<OpDocGroup> ops,
ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
- ArrayRef<EnumAttr> enums, raw_ostream &os) {
+ ArrayRef<EnumAttr> enums,
+ ArrayRef<Interface> relevantOpInterfaces,
+ raw_ostream &os) {
if (!ops.empty()) {
os << "## Operations\n\n";
emitSourceLink(inputFilename, os);
@@ -498,13 +547,24 @@ static void emitBlock(ArrayRef<Attribute> attributes, StringRef inputFilename,
for (const EnumAttr &def : enums)
emitEnumDoc(def, os);
}
+
+ if (!relevantOpInterfaces.empty()) {
+ os << "## Op Interfaces\n\n";
+ os << "**The following Op Interfaces are used by the Operations in this "
+ "Dialect**\n";
+ for (const Interface &interface : relevantOpInterfaces) {
+ emitOpInterfaceDoc(interface, os);
+ }
+ }
}
static void emitDialectDoc(const Dialect &dialect, StringRef inputFilename,
ArrayRef<Attribute> attributes,
ArrayRef<AttrDef> attrDefs, ArrayRef<OpDocGroup> ops,
ArrayRef<Type> types, ArrayRef<TypeDef> typeDefs,
- ArrayRef<EnumAttr> enums, raw_ostream &os) {
+ ArrayRef<EnumAttr> enums,
+ ArrayRef<Interface> relevantOpInterfaces,
+ raw_ostream &os) {
os << "# '" << dialect.getName() << "' Dialect\n\n";
emitIfNotEmpty(dialect.getSummary(), os);
emitIfNotEmpty(dialect.getDescription(), os);
@@ -515,7 +575,7 @@ static void emitDialectDoc(const Dialect &dialect, StringRef inputFilename,
os << "[TOC]\n\n";
emitBlock(attributes, inputFilename, attrDefs, ops, types, typeDefs, enums,
- os);
+ relevantOpInterfaces, os);
}
static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
@@ -544,16 +604,19 @@ static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
std::vector<Type> dialectTypes;
std::vector<TypeDef> dialectTypeDefs;
std::vector<EnumAttr> dialectEnums;
+ std::vector<Interface> relevantOpInterfaces;
- llvm::SmallDenseSet<Record *> seen;
- auto addIfNotSeen = [&](llvm::Record *record, const auto &def, auto &vec) {
+ llvm::SmallDenseSet<const Record *> seen;
+ auto addIfNotSeen = [&](const llvm::Record *record, const auto &def,
+ auto &vec) {
if (seen.insert(record).second) {
vec.push_back(def);
return true;
}
return false;
};
- auto addIfInDialect = [&](llvm::Record *record, const auto &def, auto &vec) {
+ auto addIfInDialect = [&](const llvm::Record *record, const auto &def,
+ auto &vec) {
return def.getDialect() == *dialect && addIfNotSeen(record, def, vec);
};
@@ -589,6 +652,19 @@ static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
for (Record *def : enumDefs)
addIfNotSeen(def, EnumAttr(def), dialectEnums);
+ for (Record *def : opDefs) {
+ Operator o(def);
+ for (auto &trait : o.getTraits()) {
+ if (const InterfaceTrait *ifTrait = dyn_cast<InterfaceTrait>(&trait)) {
+ if (trait.getDef().isSubClassOf("SideEffectsTraitBase")) {
+ continue;
+ }
+ addIfNotSeen(&ifTrait->getDef(), ifTrait->getInterface(),
+ relevantOpInterfaces);
+ }
+ }
+ }
+
// Sort alphabetically ignorning dialect for ops and section name for
// sections.
// TODO: The sorting order could be revised, currently attempting to sort of
@@ -606,7 +682,7 @@ static bool emitDialectDoc(const RecordKeeper &recordKeeper, raw_ostream &os) {
os << "<!-- Autogenerated by mlir-tblgen; don't manually edit -->\n";
emitDialectDoc(*dialect, recordKeeper.getInputFilename(), dialectAttrs,
dialectAttrDefs, dialectOps, dialectTypes, dialectTypeDefs,
- dialectEnums, os);
+ dialectEnums, relevantOpInterfaces, os);
return false;
}
|
|
@tomnatan30 Let me know what you think. I only emitted code for OpInterfaces which were used by the Operations within the .td file, so that the dialect-doc served as a more complete reference. I did this since it seemed like the -gen-op-interface-docs generator could already produce all the documentation for op interfaces necessary. If you had something else in mind thought, please let me know! :) |
|
Thanks @ammar2199, I actually wasn't aware of -gen-op-interface-docs, sorry about that! I thought it would be nice to have the dialect doc also include all op interfaces defined in the dialect, maybe it's simply a matter of adding what is generated by -gen-op-interface-docs in the dialect doc. I personally don't think we should inlcude information on interfaces that aren't part of the dialect, because this doc serves as documentation for the IR of the dialect itself, and the user can always read the documentation of the relevant dialect. @jpienaar wdyt? Also, worth updating the test file in mlir/test/mlir-tblgen/gen-dialect-doc.td with an op interface so we see the result of this change. See #98885 for reference (added enum support) |
|
Note that ops/types/attrs/enums can have their own separate doc generated, but the dialect doc includes all of them. |
|
@tomnatan30 Ah ok, I see what you're saying. Thank you for the feedback. Question: Is an OpInterface tied to a dialect though? For instance, I'm looking at the LinAlg Dialect and can see that there are Ops, EnumAttrs, and OpInterfaces defined underneath the LinAlg directory. However there's no dialect field defined for any of the OpInterfaces like there is for Ops and EnumAttrs (the dialect fields are defined in class Op and class DialectAttr, respectively - both within include/mlir/IR/) If there were no Dialect field for OpInterfaces, then my thought is that it may not be easily possible to differentiate between if an OpInterface field is part of a dialect and if it's not - since many Ops seem to use the OpInterfaces used in include/mlir/IR. If certain OpInterfaces are indeed meant to be used by only a single Dialect, then one solution could be to define a class specifies this field and make Dialect-Specific OpInterfaces inherit from it. Would also like to hear from @jpienaar too. I myself am not too familar on all of the concepts yet. Thanks! :) |
|
@tomnatan30 Hmm, ok, maybe it's not worth adding this. It seems Interfaces in general are supposed to be agnostic of Dialects, therefore it might not make sense to place it within the Dialect Doc. In the MLIR docs own words: |
|
I see you point! I thought of op interfaces as a way to have a dialect expose an API for ops from other dialects to implement, so that passes in this dialect can operate on the interface without being aware of the ops themselves and their dialects. @jpienaar wdyt? |
This only emits the OpInterfaces which are used Operations defined
in a specific Dialect. Issue: #104593