[Mlir-commits] [mlir] [mlir] Move GET_ATTR/TYPEDEF_CLASSES after declarations (PR #171444)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: None (Harald-R)

<details>
<summary>Changes</summary>

Currently, the `GET_ATTRDEF_CLASSES` and `GET_TYPEDEF_CLASSES` macros get introduced at the very top of the generated header for attributes and types, which allows the entire header to be skipped from inclusion when they are not specified. Inside the scope of these macros, the classes are forward-declared in case the attributes / types refer to each other. Example for `BuiltinAttributes.h.inc` (the descriptions of each class are removed for simplicity):

```C++
#ifdef GET_ATTRDEF_CLASSES
#undef GET_ATTRDEF_CLASSES
// Symbols commonly used in the attribute classes below
namespace mlir {
class AsmParser;
class AsmPrinter;
} // namespace mlir

namespace mlir {
class AffineMapAttr;
class ArrayAttr;
class DenseArrayAttr;
// ...
class UnitAttr;
} // namespace mlir

// Attribute classes declarations...
```

The change in this PR proposes to move the macros after the forward-declarations. This allows these declarations to be included separately if desired, which allows them to be used as forward declarations in other parts of the project. The goal of this is to allow MLIR projects to reduce the build time by employing forward declarations where possible, thus reducing the amount of expensive includes. The impact of this change on `BuiltinAttributes.h.inc`:

```C++
namespace mlir {
class AffineMapAttr;
class ArrayAttr;
class DenseArrayAttr;
// ...
class UnitAttr;
} // namespace mlir

#ifdef GET_ATTRDEF_CLASSES
#undef GET_ATTRDEF_CLASSES
// Symbols commonly used in the attribute classes below
namespace mlir {
class AsmParser;
class AsmPrinter;
} // namespace mlir

// Attribute classes declarations...
```

This change is not expected to break the compatibility with the existing code which includes the generated `.h.inc` files. It also exposes the same forward-declaration feature that is available for operations; for example, `BuiltinOps.h.inc`:

```C++
namespace mlir {
class ModuleOp;
} // namespace mlir
namespace mlir {
class UnrealizedConversionCastOp;
} // namespace mlir
#ifdef GET_OP_CLASSES
#undef GET_OP_CLASSES

// Op classes declarations...
```

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


3 Files Affected:

- (modified) mlir/test/mlir-tblgen/attrdefs.td (+7) 
- (modified) mlir/test/mlir-tblgen/typedefs.td (+8) 
- (modified) mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp (+12-6) 


``````````diff
diff --git a/mlir/test/mlir-tblgen/attrdefs.td b/mlir/test/mlir-tblgen/attrdefs.td
index a809611fd0aec..bd0c663228015 100644
--- a/mlir/test/mlir-tblgen/attrdefs.td
+++ b/mlir/test/mlir-tblgen/attrdefs.td
@@ -4,6 +4,13 @@
 include "mlir/IR/AttrTypeBase.td"
 include "mlir/IR/OpBase.td"
 
+// DECL: namespace test {
+// DECL: class IndexAttr;
+// DECL: class SimpleAAttr;
+// DECL: class CompoundAAttr;
+// DECL: class SingleParameterAttr;
+// DECL: } // namespace test
+
 // DECL: #ifdef GET_ATTRDEF_CLASSES
 // DECL: #undef GET_ATTRDEF_CLASSES
 
diff --git a/mlir/test/mlir-tblgen/typedefs.td b/mlir/test/mlir-tblgen/typedefs.td
index b9e3a7954e361..ebdb172e9aff3 100644
--- a/mlir/test/mlir-tblgen/typedefs.td
+++ b/mlir/test/mlir-tblgen/typedefs.td
@@ -4,6 +4,14 @@
 include "mlir/IR/AttrTypeBase.td"
 include "mlir/IR/OpBase.td"
 
+// DECL: namespace test {
+// DECL: class SimpleAType;
+// DECL: class CompoundAType;
+// DECL: class IndexType;
+// DECL: class SingleParameterType;
+// DECL: class IntegerType;
+// DECL: } // namespace test
+
 // DECL: #ifdef GET_TYPEDEF_CLASSES
 // DECL: #undef GET_TYPEDEF_CLASSES
 
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index 2a513c3b8cc9b..e51c776b905a8 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -869,29 +869,35 @@ class AsmPrinter;
 
 bool DefGenerator::emitDecls(StringRef selectedDialect) {
   emitSourceFileHeader((defType + "Def Declarations").str(), os);
-  llvm::IfDefEmitter scope(os, "GET_" + defType.upper() + "DEF_CLASSES");
-
-  // Output the common "header".
-  os << typeDefDeclHeader;
 
   SmallVector<AttrOrTypeDef, 16> defs;
   collectAllDefs(selectedDialect, defRecords, defs);
   if (defs.empty())
     return false;
+
   {
     DialectNamespaceEmitter nsEmitter(os, defs.front().getDialect());
-
-    // Declare all the def classes first (in case they reference each other).
+    // Declare all the def classes first (in case they reference each other and
+    // for forward declarations).
     for (const AttrOrTypeDef &def : defs) {
       tblgen::emitSummaryAndDescComments(os, def.getSummary(),
                                          def.getDescription());
       os << "class " << def.getCppClassName() << ";\n";
     }
+  }
 
+  llvm::IfDefEmitter scope(os, "GET_" + defType.upper() + "DEF_CLASSES");
+
+  // Output the common "header".
+  os << typeDefDeclHeader;
+
+  {
+    DialectNamespaceEmitter nsEmitter(os, defs.front().getDialect());
     // Emit the declarations.
     for (const AttrOrTypeDef &def : defs)
       DefGen(def).emitDecl(os);
   }
+
   // Emit the TypeID explicit specializations to have a single definition for
   // each of these.
   for (const AttrOrTypeDef &def : defs)

``````````

</details>


https://github.com/llvm/llvm-project/pull/171444