-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[MLIR][TableGen] Add gen-attrdef-list #126332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This adds a new mlir-tblgen option (-gen-attrdef-list) to generate a list of AttrDefs from a .td file in a format suitable for use in patterns where a macro is defined to expand various repeated code snippets for each item in the list. Specifically, the file will contain a list in this format ATTRDEF(MyAttr) This will be used in ClangIR to create an attribute visitor.
@llvm/pr-subscribers-mlir-core Author: Andy Kaylor (andykaylor) ChangesThis adds a new mlir-tblgen option (-gen-attrdef-list) to generate a list of AttrDefs from a .td file in a format suitable for use in patterns where a macro is defined to expand various repeated code snippets for each item in the list. Specifically, the file will contain a list in this format ATTRDEF(MyAttr) This will be used in ClangIR to create an attribute visitor. Full diff: https://github.com/llvm/llvm-project/pull/126332.diff 2 Files Affected:
diff --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td
index 35d2c49619ee69c..ecb1c93108597ee 100644
--- a/mlir/test/mlir-tblgen/attrdefs.td
+++ b/mlir/test/mlir-tblgen/attrdefs.td
@@ -1,5 +1,6 @@
// RUN: mlir-tblgen -gen-attrdef-decls -I %S/../../include %s | FileCheck %s --check-prefix=DECL
// RUN: mlir-tblgen -gen-attrdef-defs -I %S/../../include %s | FileCheck %s --check-prefix=DEF
+// RUN: mlir-tblgen -gen-attrdef-list -I %S/../../include %s | FileCheck %s --check-prefix=LIST
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/OpBase.td"
@@ -19,6 +20,12 @@ include "mlir/IR/OpBase.td"
// DEF: ::test::CompoundAAttr,
// DEF: ::test::SingleParameterAttr
+// LIST: ATTRDEF(IndexAttr)
+// LIST: ATTRDEF(SimpleAAttr)
+// LIST: ATTRDEF(CompoundAAttr)
+// LIST: ATTRDEF(SingleParameterAttr)
+
+// LIST: #undef ATTRDEF
// DEF-LABEL: ::mlir::OptionalParseResult generatedAttributeParser(
// DEF-SAME: ::mlir::AsmParser &parser,
// DEF-SAME: ::llvm::StringRef *mnemonic, ::mlir::Type type,
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 6a39424bd463fd7..a5a9a5c55908788 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -690,6 +690,7 @@ class DefGenerator {
public:
bool emitDecls(StringRef selectedDialect);
bool emitDefs(StringRef selectedDialect);
+ bool emitList(StringRef selectedDialect);
protected:
DefGenerator(ArrayRef<const Record *> defs, raw_ostream &os,
@@ -1025,6 +1026,23 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
return false;
}
+bool DefGenerator::emitList(StringRef selectedDialect) {
+ emitSourceFileHeader(("List of " + defType + "Def Definitions").str(), os);
+
+ SmallVector<AttrOrTypeDef, 16> defs;
+ collectAllDefs(selectedDialect, defRecords, defs);
+ if (defs.empty())
+ return false;
+
+ auto interleaveFn = [&](const AttrOrTypeDef &def) {
+ os << defType.upper() << "DEF(" << def.getCppClassName() << ")";
+ };
+ llvm::interleave(defs, os, interleaveFn, "\n");
+ os << "\n\n";
+ os << "#undef " << defType.upper() << "DEF" << "\n";
+ return false;
+}
+
//===----------------------------------------------------------------------===//
// Type Constraints
//===----------------------------------------------------------------------===//
@@ -1099,7 +1117,12 @@ static mlir::GenRegistration
AttrDefGenerator generator(records, os);
return generator.emitDecls(attrDialect);
});
-
+static mlir::GenRegistration
+ genAttrList("gen-attrdef-list", "Generate an AttrDef list",
+ [](const RecordKeeper &records, raw_ostream &os) {
+ AttrDefGenerator generator(records, os);
+ return generator.emitList(attrDialect);
+ });
//===----------------------------------------------------------------------===//
// TypeDef
|
@llvm/pr-subscribers-mlir Author: Andy Kaylor (andykaylor) ChangesThis adds a new mlir-tblgen option (-gen-attrdef-list) to generate a list of AttrDefs from a .td file in a format suitable for use in patterns where a macro is defined to expand various repeated code snippets for each item in the list. Specifically, the file will contain a list in this format ATTRDEF(MyAttr) This will be used in ClangIR to create an attribute visitor. Full diff: https://github.com/llvm/llvm-project/pull/126332.diff 2 Files Affected:
diff --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td
index 35d2c49619ee69c..ecb1c93108597ee 100644
--- a/mlir/test/mlir-tblgen/attrdefs.td
+++ b/mlir/test/mlir-tblgen/attrdefs.td
@@ -1,5 +1,6 @@
// RUN: mlir-tblgen -gen-attrdef-decls -I %S/../../include %s | FileCheck %s --check-prefix=DECL
// RUN: mlir-tblgen -gen-attrdef-defs -I %S/../../include %s | FileCheck %s --check-prefix=DEF
+// RUN: mlir-tblgen -gen-attrdef-list -I %S/../../include %s | FileCheck %s --check-prefix=LIST
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/OpBase.td"
@@ -19,6 +20,12 @@ include "mlir/IR/OpBase.td"
// DEF: ::test::CompoundAAttr,
// DEF: ::test::SingleParameterAttr
+// LIST: ATTRDEF(IndexAttr)
+// LIST: ATTRDEF(SimpleAAttr)
+// LIST: ATTRDEF(CompoundAAttr)
+// LIST: ATTRDEF(SingleParameterAttr)
+
+// LIST: #undef ATTRDEF
// DEF-LABEL: ::mlir::OptionalParseResult generatedAttributeParser(
// DEF-SAME: ::mlir::AsmParser &parser,
// DEF-SAME: ::llvm::StringRef *mnemonic, ::mlir::Type type,
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 6a39424bd463fd7..a5a9a5c55908788 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -690,6 +690,7 @@ class DefGenerator {
public:
bool emitDecls(StringRef selectedDialect);
bool emitDefs(StringRef selectedDialect);
+ bool emitList(StringRef selectedDialect);
protected:
DefGenerator(ArrayRef<const Record *> defs, raw_ostream &os,
@@ -1025,6 +1026,23 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
return false;
}
+bool DefGenerator::emitList(StringRef selectedDialect) {
+ emitSourceFileHeader(("List of " + defType + "Def Definitions").str(), os);
+
+ SmallVector<AttrOrTypeDef, 16> defs;
+ collectAllDefs(selectedDialect, defRecords, defs);
+ if (defs.empty())
+ return false;
+
+ auto interleaveFn = [&](const AttrOrTypeDef &def) {
+ os << defType.upper() << "DEF(" << def.getCppClassName() << ")";
+ };
+ llvm::interleave(defs, os, interleaveFn, "\n");
+ os << "\n\n";
+ os << "#undef " << defType.upper() << "DEF" << "\n";
+ return false;
+}
+
//===----------------------------------------------------------------------===//
// Type Constraints
//===----------------------------------------------------------------------===//
@@ -1099,7 +1117,12 @@ static mlir::GenRegistration
AttrDefGenerator generator(records, os);
return generator.emitDecls(attrDialect);
});
-
+static mlir::GenRegistration
+ genAttrList("gen-attrdef-list", "Generate an AttrDef list",
+ [](const RecordKeeper &records, raw_ostream &os) {
+ AttrDefGenerator generator(records, os);
+ return generator.emitList(attrDialect);
+ });
//===----------------------------------------------------------------------===//
// TypeDef
|
This change (llvm/clangir#1318) in the clangir incubator repo shows the intended use of the generated files. |
It's not clear to me from reading the clangir pr that the existing attrdef list generation can't be used in combination with TypeSwitch and templates to get what you want. I dont think we need anything additional to be generated. Have you tried to use the existing GET_ATTRDEF_LIST logic for this? You would need to have overloaded 'visit' methods (i.e. can't have the name of the attribute in the visit method), but this should all just work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking RC
@River707 Thanks! I had tried to figure out a way to do this with the existing GET_ATTRDEF_LIST but I couldn't find my way to a solution. It looks like TypeSwitch is the piece I was missing. I did still run into a problem trying to adapt my visitor base class, because (at least with the compiler I'm using) TypeSwitch doesn't like taking a template parameter for the return type. That is, when I try this:
I get a bunch of errors like this:
But if I switch to this:
It works. That feels like a compiler bug, but I could be missing something. However, I don't think I need it to work. I updated my clangir patch to just use TypeSwitch in my visitor implementation class, and I don't think I will even need to use GET_ATTRDEF_LIST. If you're interested, the updated clangir change is here: llvm/clangir#1330 I'll close this PR if the new clangir implementation is acceptable to the reviewers there. |
Nice, I'm glad that worked. This use case is one of the reasons I wrote TypeSwitch initially. The template error seems a bit weird, I feel like I've written that exact code before. |
Withdrawn in favor of TypeSwitch-based implementation |
We previously discussed having an mlir-tblgen utility to complete the CIRAttrVisitor implementation with all support attribute types, but when I proposed an implementation to do this, a reviewer suggested using TypeSwitch instead, and I have done that in the incubator. See llvm#126332 This change brings the TypeSwitch implementation into the upstream repo to replace the visitor class.
The previously upstreamed lowering from ClangIR to LLVM IR diverged from the incubator implementation, but when the incubator was updated to incorporate these changes some issues arose which require the upstream implementation to be modified to re-align with the incubator. First, in the earlier upstream implementation a CIRAttrVisitor class was introduced with the intention that an mlir-tblgen based extension would be created to automatically add all CIR attributes to the visitor. When I proposed this in mlir-tblgen a reviewer suggested that what I wanted could be better accomplished with TypeSwitch. See #126332 This was done in the incubator, and here I am bringing that implementation upstream. The other issue was that the global op initialization in the incubator had more cases than I had accounted for in my previous upstream refactoring. I did still refactor the incubator code, but not in quite the same way as the upstream code. This change re-aligns the two.
…(#129293) The previously upstreamed lowering from ClangIR to LLVM IR diverged from the incubator implementation, but when the incubator was updated to incorporate these changes some issues arose which require the upstream implementation to be modified to re-align with the incubator. First, in the earlier upstream implementation a CIRAttrVisitor class was introduced with the intention that an mlir-tblgen based extension would be created to automatically add all CIR attributes to the visitor. When I proposed this in mlir-tblgen a reviewer suggested that what I wanted could be better accomplished with TypeSwitch. See llvm/llvm-project#126332 This was done in the incubator, and here I am bringing that implementation upstream. The other issue was that the global op initialization in the incubator had more cases than I had accounted for in my previous upstream refactoring. I did still refactor the incubator code, but not in quite the same way as the upstream code. This change re-aligns the two.
The previously upstreamed lowering from ClangIR to LLVM IR diverged from the incubator implementation, but when the incubator was updated to incorporate these changes some issues arose which require the upstream implementation to be modified to re-align with the incubator. First, in the earlier upstream implementation a CIRAttrVisitor class was introduced with the intention that an mlir-tblgen based extension would be created to automatically add all CIR attributes to the visitor. When I proposed this in mlir-tblgen a reviewer suggested that what I wanted could be better accomplished with TypeSwitch. See llvm#126332 This was done in the incubator, and here I am bringing that implementation upstream. The other issue was that the global op initialization in the incubator had more cases than I had accounted for in my previous upstream refactoring. I did still refactor the incubator code, but not in quite the same way as the upstream code. This change re-aligns the two.
This adds a new mlir-tblgen option (-gen-attrdef-list) to generate a list of AttrDefs from a .td file in a format suitable for use in patterns where a macro is defined to expand various repeated code snippets for each item in the list. Specifically, the file will contain a list in this format
ATTRDEF(MyAttr)
This will be used in ClangIR to create an attribute visitor.