[MLIR][TableGen] Add gen-attrdef-list #126332

andykaylor · 2025-02-08T01:46:33Z

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.

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.

llvmbot · 2025-02-08T01:47:05Z

@llvm/pr-subscribers-mlir-core

Author: Andy Kaylor (andykaylor)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/126332.diff

2 Files Affected:

(modified) mlir/test/mlir-tblgen/attrdefs.td (+7)
(modified) mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp (+24-1)

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

llvmbot · 2025-02-08T01:47:05Z

@llvm/pr-subscribers-mlir

Author: Andy Kaylor (andykaylor)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/126332.diff

2 Files Affected:

(modified) mlir/test/mlir-tblgen/attrdefs.td (+7)
(modified) mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp (+24-1)

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

andykaylor · 2025-02-08T01:47:41Z

This change (llvm/clangir#1318) in the clangir incubator repo shows the intended use of the generated files.

River707 · 2025-02-08T02:06:59Z

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.

River707

Marking RC

andykaylor · 2025-02-10T22:55:15Z

@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:

  RetTy visit(mlir::Attribute attr) {
    #define GET_ATTRDEF_LIST
    return llvm::TypeSwitch<mlir::Attribute, RetTy>(attr)
    .Case<
#include "clang/CIR/Dialect/IR/CIROpsAttributes.cpp.inc"
    >([&](auto attrT) { return getImpl()->visitCirAttr(attrT); })
      .Default([&](auto attrT) { return getImpl()->visitNonCirAttr(attrT); });
  }

I get a bunch of errors like this:

llvm-project/build/tools/clang/include/clang/CIR/Dialect/IR/CIROpsAttributes.cpp.inc: In member function ‘RetTy cir::CirAttrVisitor<ImplClass, RetTy>::visit(mlir::Attribute)’:
llvm-project/build/tools/clang/include/clang/CIR/Dialect/IR/CIROpsAttributes.cpp.inc:12:16: error: expected primary-expression before ‘,’ token
   12 | ::cir::LangAttr,
      |                ^

But if I switch to this:

    return llvm::TypeSwitch<mlir::Attribute, mlir::Value>(attr)

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.

River707 · 2025-02-10T23:18:06Z

@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:
  RetTy visit(mlir::Attribute attr) {
    #define GET_ATTRDEF_LIST
    return llvm::TypeSwitch<mlir::Attribute, RetTy>(attr)
    .Case<
#include "clang/CIR/Dialect/IR/CIROpsAttributes.cpp.inc"
    >([&](auto attrT) { return getImpl()->visitCirAttr(attrT); })
      .Default([&](auto attrT) { return getImpl()->visitNonCirAttr(attrT); });
  }
I get a bunch of errors like this:
llvm-project/build/tools/clang/include/clang/CIR/Dialect/IR/CIROpsAttributes.cpp.inc: In member function ‘RetTy cir::CirAttrVisitor<ImplClass, RetTy>::visit(mlir::Attribute)’:
llvm-project/build/tools/clang/include/clang/CIR/Dialect/IR/CIROpsAttributes.cpp.inc:12:16: error: expected primary-expression before ‘,’ token
   12 | ::cir::LangAttr,
      |                ^
But if I switch to this:
    return llvm::TypeSwitch<mlir::Attribute, mlir::Value>(attr)
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.

andykaylor · 2025-02-11T16:55:48Z

Withdrawn in favor of TypeSwitch-based implementation

andykaylor requested review from joker-eph, bcardosolopes and jurahul February 8, 2025 01:46

llvmbot added mlir:core MLIR Core Infrastructure mlir labels Feb 8, 2025

River707 requested changes Feb 8, 2025

View reviewed changes

andykaylor mentioned this pull request Feb 10, 2025

[CIR][NFC] Replace CIR attr visitor base class with a type switch llvm/clangir#1330

Merged

andykaylor closed this Feb 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MLIR][TableGen] Add gen-attrdef-list #126332

[MLIR][TableGen] Add gen-attrdef-list #126332

andykaylor commented Feb 8, 2025

llvmbot commented Feb 8, 2025

llvmbot commented Feb 8, 2025

andykaylor commented Feb 8, 2025

River707 commented Feb 8, 2025 •

edited

Loading

River707 left a comment

andykaylor commented Feb 10, 2025

River707 commented Feb 10, 2025

andykaylor commented Feb 11, 2025

[MLIR][TableGen] Add gen-attrdef-list #126332

[MLIR][TableGen] Add gen-attrdef-list #126332

Conversation

andykaylor commented Feb 8, 2025

llvmbot commented Feb 8, 2025

llvmbot commented Feb 8, 2025

andykaylor commented Feb 8, 2025

River707 commented Feb 8, 2025 • edited Loading

River707 left a comment

Choose a reason for hiding this comment

andykaylor commented Feb 10, 2025

River707 commented Feb 10, 2025

andykaylor commented Feb 11, 2025

River707 commented Feb 8, 2025 •

edited

Loading