Skip to content

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 2, 2025

Migrate IfDef and Namespace emitters from MLIR to LLVM TableGen and do a minimal adoption these emitters in LLVM TableGen.

Introduce a DialectNamespaceEmitter for MLIR specific use and eliminate duplicated version of IfDef emitter in DirectiveEmitter.

Verified before/after diff for the generated .inc files for MLIR and LLVM.


private:
void emitNamespaceStarts(StringRef Name) {
llvm::SplitString(Name, Namespaces, "::");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A follow-on improvement would be to get rid of this namespace parsing and emit this directly using C++17 nested namespace definition.

@jurahul jurahul force-pushed the llvm_tablegen_ifdef_ns_emitter branch from bc19435 to 1e9167a Compare October 2, 2025 23:00
Migrate IfDef and Namespace emitters from MLIR to LLVM TableGen.
Introduce a `DialectNamespaceEmitter` for MLIR specific use, and
do a minimal adoption these emitters in LLVM TableGen.

Eliminate duplicated version of IfDef emitter in DirectiveEmitter.
@jurahul jurahul force-pushed the llvm_tablegen_ifdef_ns_emitter branch from 1e9167a to 3f343bb Compare October 3, 2025 00:12
@jurahul jurahul marked this pull request as ready for review October 3, 2025 00:53
@jurahul jurahul requested a review from s-barannikov October 3, 2025 00:53
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2025

@llvm/pr-subscribers-mlir-spirv
@llvm/pr-subscribers-tablegen
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Rahul Joshi (jurahul)

Changes

Migrate IfDef and Namespace emitters from MLIR to LLVM TableGen and do a minimal adoption these emitters in LLVM TableGen.

Introduce a DialectNamespaceEmitter for MLIR specific use and eliminate duplicated version of IfDef emitter in DirectiveEmitter.

Verified before/after diff for the generated .inc files for MLIR and LLVM.


Patch is 23.79 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/161744.diff

11 Files Affected:

  • (added) llvm/include/llvm/TableGen/CodeGenHelpers.h (+63)
  • (modified) llvm/utils/TableGen/Basic/DirectiveEmitter.cpp (+14-42)
  • (modified) llvm/utils/TableGen/Basic/TargetFeaturesEmitter.cpp (+17-19)
  • (modified) mlir/include/mlir/TableGen/CodeGenHelpers.h (+8-34)
  • (modified) mlir/include/mlir/TableGen/Dialect.h (+1)
  • (modified) mlir/lib/TableGen/CodeGenHelpers.cpp (+1)
  • (modified) mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp (+10-10)
  • (modified) mlir/tools/mlir-tblgen/DialectGen.cpp (+2-2)
  • (modified) mlir/tools/mlir-tblgen/OmpOpGen.cpp (+1-1)
  • (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+6-5)
  • (modified) mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp (+1-2)
diff --git a/llvm/include/llvm/TableGen/CodeGenHelpers.h b/llvm/include/llvm/TableGen/CodeGenHelpers.h
new file mode 100644
index 0000000000000..2a9ae8b4e995d
--- /dev/null
+++ b/llvm/include/llvm/TableGen/CodeGenHelpers.h
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines common utilities for generating C++ code.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TABLEGEN_CODEGENHELPERS_H
+#define LLVM_TABLEGEN_CODEGENHELPERS_H
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include <string>
+
+namespace llvm {
+// Simple RAII helper for emitting ifdef-undef-endif scope.
+class IfDefEmitter {
+public:
+  IfDefEmitter(raw_ostream &OS, StringRef Name) : Name(Name.str()), OS(OS) {
+    OS << "#ifdef " << Name << "\n"
+       << "#undef " << Name << "\n\n";
+  }
+  ~IfDefEmitter() { OS << "\n#endif // " << Name << "\n\n"; }
+
+private:
+  std::string Name;
+  raw_ostream &OS;
+};
+
+// Simple RAII helper for emitting namespace scope. Name can be a single
+// namespace (empty for anonymous namespace) or nested namespace.
+class NamespaceEmitter {
+public:
+  NamespaceEmitter(raw_ostream &OS, StringRef Name) : OS(OS) {
+    emitNamespaceStarts(Name);
+  }
+
+  ~NamespaceEmitter() {
+    for (StringRef NS : llvm::reverse(Namespaces))
+      OS << "} // namespace " << NS << "\n";
+  }
+
+private:
+  void emitNamespaceStarts(StringRef Name) {
+    llvm::SplitString(Name, Namespaces, "::");
+    for (StringRef NS : Namespaces)
+      OS << "namespace " << NS << " {\n";
+  }
+
+  SmallVector<StringRef, 2> Namespaces;
+  raw_ostream &OS;
+};
+
+} // end namespace llvm
+
+#endif // LLVM_TABLEGEN_CODEGENHELPERS_H
diff --git a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
index f0e23690367db..f99c1797428fe 100644
--- a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/TableGen/CodeGenHelpers.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
@@ -29,27 +30,11 @@
 
 using namespace llvm;
 
-namespace {
-// Simple RAII helper for defining ifdef-undef-endif scopes.
-class IfDefScope {
-public:
-  IfDefScope(StringRef Name, raw_ostream &OS) : Name(Name), OS(OS) {
-    OS << "#ifdef " << Name << "\n"
-       << "#undef " << Name << "\n";
-  }
-
-  ~IfDefScope() { OS << "\n#endif // " << Name << "\n\n"; }
-
-private:
-  StringRef Name;
-  raw_ostream &OS;
-};
-} // namespace
-
 namespace {
 enum class Frontend { LLVM, Flang, Clang };
+} // namespace
 
-StringRef getFESpelling(Frontend FE) {
+static StringRef getFESpelling(Frontend FE) {
   switch (FE) {
   case Frontend::LLVM:
     return "llvm";
@@ -60,7 +45,6 @@ StringRef getFESpelling(Frontend FE) {
   }
   llvm_unreachable("unknown FE kind");
 }
-} // namespace
 
 // Get the full namespace qualifier for the directive language.
 static std::string getQualifier(const DirectiveLanguage &DirLang,
@@ -971,11 +955,10 @@ static void generateDirectiveClauseSets(const DirectiveLanguage &DirLang,
   std::string IfDefName{"GEN_"};
   IfDefName += getFESpelling(FE).upper();
   IfDefName += "_DIRECTIVE_CLAUSE_SETS";
-  IfDefScope Scope(IfDefName, OS);
+  IfDefEmitter Scope(OS, IfDefName);
 
   StringRef Namespace =
       getFESpelling(FE == Frontend::Flang ? Frontend::LLVM : FE);
-  OS << "\n";
   // The namespace has to be different for clang vs flang, as 2 structs with the
   // same name but different layout is UB.  So just put the 'clang' on in the
   // clang namespace.
@@ -1016,9 +999,8 @@ static void generateDirectiveClauseMap(const DirectiveLanguage &DirLang,
   std::string IfDefName{"GEN_"};
   IfDefName += getFESpelling(FE).upper();
   IfDefName += "_DIRECTIVE_CLAUSE_MAP";
-  IfDefScope Scope(IfDefName, OS);
+  IfDefEmitter Scope(OS, IfDefName);
 
-  OS << "\n";
   OS << "{\n";
 
   // The namespace has to be different for clang vs flang, as 2 structs with the
@@ -1062,9 +1044,7 @@ static void generateDirectiveClauseMap(const DirectiveLanguage &DirLang,
 static void generateFlangClauseParserClass(const DirectiveLanguage &DirLang,
                                            raw_ostream &OS) {
 
-  IfDefScope Scope("GEN_FLANG_CLAUSE_PARSER_CLASSES", OS);
-
-  OS << "\n";
+  IfDefEmitter Scope(OS, "GEN_FLANG_CLAUSE_PARSER_CLASSES");
 
   for (const Clause Clause : DirLang.getClauses()) {
     if (!Clause.getFlangClass().empty()) {
@@ -1089,9 +1069,8 @@ static void generateFlangClauseParserClass(const DirectiveLanguage &DirLang,
 static void generateFlangClauseParserClassList(const DirectiveLanguage &DirLang,
                                                raw_ostream &OS) {
 
-  IfDefScope Scope("GEN_FLANG_CLAUSE_PARSER_CLASSES_LIST", OS);
+  IfDefEmitter Scope(OS, "GEN_FLANG_CLAUSE_PARSER_CLASSES_LIST");
 
-  OS << "\n";
   interleaveComma(DirLang.getClauses(), OS, [&](const Record *C) {
     Clause Clause(C);
     OS << Clause.getFormattedParserClassName() << "\n";
@@ -1102,9 +1081,8 @@ static void generateFlangClauseParserClassList(const DirectiveLanguage &DirLang,
 static void generateFlangClauseDump(const DirectiveLanguage &DirLang,
                                     raw_ostream &OS) {
 
-  IfDefScope Scope("GEN_FLANG_DUMP_PARSE_TREE_CLAUSES", OS);
+  IfDefEmitter Scope(OS, "GEN_FLANG_DUMP_PARSE_TREE_CLAUSES");
 
-  OS << "\n";
   for (const Clause Clause : DirLang.getClauses()) {
     OS << "NODE(" << DirLang.getFlangClauseBaseClass() << ", "
        << Clause.getFormattedParserClassName() << ")\n";
@@ -1116,10 +1094,9 @@ static void generateFlangClauseDump(const DirectiveLanguage &DirLang,
 static void generateFlangClauseUnparse(const DirectiveLanguage &DirLang,
                                        raw_ostream &OS) {
 
-  IfDefScope Scope("GEN_FLANG_CLAUSE_UNPARSE", OS);
+  IfDefEmitter Scope(OS, "GEN_FLANG_CLAUSE_UNPARSE");
 
   StringRef Base = DirLang.getFlangClauseBaseClass();
-  OS << "\n";
 
   for (const Clause Clause : DirLang.getClauses()) {
     if (Clause.skipFlangUnparser())
@@ -1172,9 +1149,8 @@ static void generateFlangClauseUnparse(const DirectiveLanguage &DirLang,
 static void generateFlangClauseCheckPrototypes(const DirectiveLanguage &DirLang,
                                                raw_ostream &OS) {
 
-  IfDefScope Scope("GEN_FLANG_CLAUSE_CHECK_ENTER", OS);
+  IfDefEmitter Scope(OS, "GEN_FLANG_CLAUSE_CHECK_ENTER");
 
-  OS << "\n";
   for (const Clause Clause : DirLang.getClauses()) {
     OS << "void Enter(const parser::" << DirLang.getFlangClauseBaseClass()
        << "::" << Clause.getFormattedParserClassName() << " &);\n";
@@ -1186,12 +1162,11 @@ static void generateFlangClauseCheckPrototypes(const DirectiveLanguage &DirLang,
 static void generateFlangClauseParserKindMap(const DirectiveLanguage &DirLang,
                                              raw_ostream &OS) {
 
-  IfDefScope Scope("GEN_FLANG_CLAUSE_PARSER_KIND_MAP", OS);
+  IfDefEmitter Scope(OS, "GEN_FLANG_CLAUSE_PARSER_KIND_MAP");
 
   StringRef Prefix = DirLang.getClausePrefix();
   std::string Qual = getQualifier(DirLang);
 
-  OS << "\n";
   for (const Record *R : DirLang.getClauses()) {
     Clause C(R);
     OS << "if constexpr (std::is_same_v<A, parser::"
@@ -1216,11 +1191,10 @@ static void generateFlangClausesParser(const DirectiveLanguage &DirLang,
   llvm::sort(Names, [](const auto &A, const auto &B) {
     return A.second.Name > B.second.Name;
   });
-  IfDefScope Scope("GEN_FLANG_CLAUSES_PARSER", OS);
+  IfDefEmitter Scope(OS, "GEN_FLANG_CLAUSES_PARSER");
   StringRef Base = DirLang.getFlangClauseBaseClass();
 
   unsigned LastIndex = Names.size() - 1;
-  OS << "\n";
   OS << "TYPE_PARSER(\n";
   for (auto [Index, RecSp] : llvm::enumerate(Names)) {
     auto [R, S] = RecSp;
@@ -1313,10 +1287,9 @@ static void emitDirectivesFlangImpl(const DirectiveLanguage &DirLang,
 static void generateClauseClassMacro(const DirectiveLanguage &DirLang,
                                      raw_ostream &OS) {
   // Generate macros style information for legacy code in clang
-  IfDefScope Scope("GEN_CLANG_CLAUSE_CLASS", OS);
+  IfDefEmitter Scope(OS, "GEN_CLANG_CLAUSE_CLASS");
 
   StringRef Prefix = DirLang.getClausePrefix();
-  OS << "\n";
 
   OS << "#ifndef CLAUSE\n";
   OS << "#define CLAUSE(Enum, Str, Implicit)\n";
@@ -1375,12 +1348,11 @@ static void generateClauseClassMacro(const DirectiveLanguage &DirLang,
 // language. This code can be included in library.
 void emitDirectivesBasicImpl(const DirectiveLanguage &DirLang,
                              raw_ostream &OS) {
-  IfDefScope Scope("GEN_DIRECTIVES_IMPL", OS);
+  IfDefEmitter Scope(OS, "GEN_DIRECTIVES_IMPL");
 
   StringRef DPrefix = DirLang.getDirectivePrefix();
   StringRef CPrefix = DirLang.getClausePrefix();
 
-  OS << "\n";
   OS << "#include \"llvm/Frontend/Directive/Spelling.h\"\n";
   OS << "#include \"llvm/Support/ErrorHandling.h\"\n";
   OS << "#include <utility>\n";
diff --git a/llvm/utils/TableGen/Basic/TargetFeaturesEmitter.cpp b/llvm/utils/TableGen/Basic/TargetFeaturesEmitter.cpp
index 6b723bc0fb02d..e2b524155a72c 100644
--- a/llvm/utils/TableGen/Basic/TargetFeaturesEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/TargetFeaturesEmitter.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "TargetFeaturesEmitter.h"
+#include "llvm/TableGen/CodeGenHelpers.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include "llvm/TargetParser/SubtargetFeature.h"
@@ -43,7 +44,7 @@ FeatureMapTy TargetFeaturesEmitter::enumeration(raw_ostream &OS) {
     PrintFatalError(
         "Too many subtarget features! Bump MAX_SUBTARGET_FEATURES.");
 
-  OS << "namespace " << Target << " {\n";
+  NamespaceEmitter NS(OS, Target);
 
   OS << "enum {\n";
 
@@ -58,9 +59,8 @@ FeatureMapTy TargetFeaturesEmitter::enumeration(raw_ostream &OS) {
 
   OS << "  " << "NumSubtargetFeatures = " << N << "\n";
 
-  // Close enumeration and namespace
+  // Close enumeration.
   OS << "};\n";
-  OS << "} // end namespace " << Target << "\n";
   return FeatureMap;
 }
 
@@ -149,25 +149,23 @@ void TargetFeaturesEmitter::printCPUKeyValues(raw_ostream &OS,
 void TargetFeaturesEmitter::run(raw_ostream &OS) {
   OS << "// Autogenerated by TargetFeatureEmitter.cpp\n\n";
 
-  OS << "\n#ifdef GET_SUBTARGETFEATURES_ENUM\n";
-  OS << "#undef GET_SUBTARGETFEATURES_ENUM\n\n";
-
-  OS << "namespace llvm {\n";
-  auto FeatureMap = enumeration(OS);
-  OS << "} // end namespace llvm\n\n";
-  OS << "#endif // GET_SUBTARGETFEATURES_ENUM\n\n";
+  FeatureMapTy FeatureMap;
+  {
+    IfDefEmitter IfDef(OS, "GET_SUBTARGETFEATURES_ENUM");
+    NamespaceEmitter NS(OS, "llvm");
+    FeatureMap = enumeration(OS);
+  }
 
-  OS << "\n#ifdef GET_SUBTARGETFEATURES_KV\n";
-  OS << "#undef GET_SUBTARGETFEATURES_KV\n\n";
+  {
+    IfDefEmitter IfDef(OS, "GET_SUBTARGETFEATURES_KV");
+    NamespaceEmitter NS(OS, "llvm");
 
-  OS << "namespace llvm {\n";
-  printFeatureKeyValues(OS, FeatureMap);
-  OS << "\n";
+    printFeatureKeyValues(OS, FeatureMap);
+    OS << "\n";
 
-  printCPUKeyValues(OS, FeatureMap);
-  OS << "\n";
-  OS << "} // end namespace llvm\n\n";
-  OS << "#endif // GET_SUBTARGETFEATURES_KV\n\n";
+    printCPUKeyValues(OS, FeatureMap);
+    OS << "\n";
+  }
 }
 
 static TableGen::Emitter::OptClass<TargetFeaturesEmitter>
diff --git a/mlir/include/mlir/TableGen/CodeGenHelpers.h b/mlir/include/mlir/TableGen/CodeGenHelpers.h
index cf14f65b93ed2..252da21419b6e 100644
--- a/mlir/include/mlir/TableGen/CodeGenHelpers.h
+++ b/mlir/include/mlir/TableGen/CodeGenHelpers.h
@@ -1,3 +1,4 @@
+//===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -20,6 +21,8 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/TableGen/CodeGenHelpers.h"
+#include <utility>
 
 namespace llvm {
 class RecordKeeper;
@@ -36,46 +39,17 @@ std::string strfmt(const char *fmt, Parameters &&...parameters) {
   return llvm::formatv(fmt, std::forward<Parameters>(parameters)...).str();
 }
 
-// Simple RAII helper for defining ifdef-undef-endif scopes.
-class IfDefScope {
-public:
-  IfDefScope(llvm::StringRef name, llvm::raw_ostream &os)
-      : name(name.str()), os(os) {
-    os << "#ifdef " << name << "\n"
-       << "#undef " << name << "\n\n";
-  }
-  ~IfDefScope() { os << "\n#endif  // " << name << "\n\n"; }
-
-private:
-  std::string name;
-  llvm::raw_ostream &os;
-};
-
-// A helper RAII class to emit nested namespaces for this op.
-class NamespaceEmitter {
+// A helper RAII class to emit nested namespaces for a dialect.
+class DialectNamespaceEmitter {
 public:
-  NamespaceEmitter(raw_ostream &os, const Dialect &dialect) : os(os) {
+  DialectNamespaceEmitter(raw_ostream &os, const Dialect &dialect) {
     if (!dialect)
       return;
-    emitNamespaceStarts(os, dialect.getCppNamespace());
-  }
-  NamespaceEmitter(raw_ostream &os, StringRef cppNamespace) : os(os) {
-    emitNamespaceStarts(os, cppNamespace);
-  }
-
-  ~NamespaceEmitter() {
-    for (StringRef ns : llvm::reverse(namespaces))
-      os << "} // namespace " << ns << "\n";
+    nsEmitter.emplace(os, dialect.getCppNamespace());
   }
 
 private:
-  void emitNamespaceStarts(raw_ostream &os, StringRef cppNamespace) {
-    llvm::SplitString(cppNamespace, namespaces, "::");
-    for (StringRef ns : namespaces)
-      os << "namespace " << ns << " {\n";
-  }
-  raw_ostream &os;
-  SmallVector<StringRef, 2> namespaces;
+  std::optional<llvm::NamespaceEmitter> nsEmitter;
 };
 
 /// This class deduplicates shared operation verification code by emitting
diff --git a/mlir/include/mlir/TableGen/Dialect.h b/mlir/include/mlir/TableGen/Dialect.h
index ea8f40555e445..37c6427a9282a 100644
--- a/mlir/include/mlir/TableGen/Dialect.h
+++ b/mlir/include/mlir/TableGen/Dialect.h
@@ -107,6 +107,7 @@ class Dialect {
 
   // Returns whether the dialect is defined.
   explicit operator bool() const { return def != nullptr; }
+  bool isDefined() const { return def != nullptr; }
 
 private:
   const llvm::Record *def;
diff --git a/mlir/lib/TableGen/CodeGenHelpers.cpp b/mlir/lib/TableGen/CodeGenHelpers.cpp
index 2c119fd680b69..cb90ef82c1c39 100644
--- a/mlir/lib/TableGen/CodeGenHelpers.cpp
+++ b/mlir/lib/TableGen/CodeGenHelpers.cpp
@@ -16,6 +16,7 @@
 #include "mlir/TableGen/Pattern.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/TableGen/CodeGenHelpers.h"
 #include "llvm/TableGen/Record.h"
 
 using namespace llvm;
diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
index b9115657d6bf3..06ef396b9b21d 100644
--- a/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
+++ b/mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp
@@ -10,12 +10,12 @@
 #include "CppGenUtilities.h"
 #include "mlir/TableGen/AttrOrTypeDef.h"
 #include "mlir/TableGen/Class.h"
-#include "mlir/TableGen/CodeGenHelpers.h"
 #include "mlir/TableGen/Format.h"
 #include "mlir/TableGen/GenInfo.h"
 #include "mlir/TableGen/Interfaces.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/TableGen/CodeGenHelpers.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/TableGenBackend.h"
 
@@ -71,14 +71,14 @@ class DefGen {
 
   void emitDecl(raw_ostream &os) const {
     if (storageCls && def.genStorageClass()) {
-      NamespaceEmitter ns(os, def.getStorageNamespace());
+      llvm::NamespaceEmitter ns(os, def.getStorageNamespace());
       os << "struct " << def.getStorageClassName() << ";\n";
     }
     defCls.writeDeclTo(os);
   }
   void emitDef(raw_ostream &os) const {
     if (storageCls && def.genStorageClass()) {
-      NamespaceEmitter ns(os, def.getStorageNamespace());
+      llvm::NamespaceEmitter ns(os, def.getStorageNamespace());
       storageCls->writeDeclTo(os); // everything is inline
     }
     defCls.writeDefTo(os);
@@ -850,7 +850,7 @@ class AsmPrinter;
 
 bool DefGenerator::emitDecls(StringRef selectedDialect) {
   emitSourceFileHeader((defType + "Def Declarations").str(), os);
-  IfDefScope scope("GET_" + defType.upper() + "DEF_CLASSES", os);
+  llvm::IfDefEmitter scope(os, "GET_" + defType.upper() + "DEF_CLASSES");
 
   // Output the common "header".
   os << typeDefDeclHeader;
@@ -860,7 +860,7 @@ bool DefGenerator::emitDecls(StringRef selectedDialect) {
   if (defs.empty())
     return false;
   {
-    NamespaceEmitter nsEmitter(os, defs.front().getDialect());
+    DialectNamespaceEmitter nsEmitter(os, defs.front().getDialect());
 
     // Declare all the def classes first (in case they reference each other).
     for (const AttrOrTypeDef &def : defs) {
@@ -892,7 +892,7 @@ bool DefGenerator::emitDecls(StringRef selectedDialect) {
 //===----------------------------------------------------------------------===//
 
 void DefGenerator::emitTypeDefList(ArrayRef<AttrOrTypeDef> defs) {
-  IfDefScope scope("GET_" + defType.upper() + "DEF_LIST", os);
+  llvm::IfDefEmitter scope(os, "GET_" + defType.upper() + "DEF_LIST");
   auto interleaveFn = [&](const AttrOrTypeDef &def) {
     os << def.getDialect().getCppNamespace() << "::" << def.getCppClassName();
   };
@@ -1083,11 +1083,11 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
     return false;
   emitTypeDefList(defs);
 
-  IfDefScope scope("GET_" + defType.upper() + "DEF_CLASSES", os);
+  llvm::IfDefEmitter scope(os, "GET_" + defType.upper() + "DEF_CLASSES");
   emitParsePrintDispatch(defs);
   for (const AttrOrTypeDef &def : defs) {
     {
-      NamespaceEmitter ns(os, def.getDialect());
+      DialectNamespaceEmitter ns(os, def.getDialect());
       DefGen gen(def);
       gen.emitDef(os);
     }
@@ -1102,7 +1102,7 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
 
   // Emit the default parser/printer for Attributes if the dialect asked for it.
   if (isAttrGenerator && firstDialect.useDefaultAttributePrinterParser()) {
-    NamespaceEmitter nsEmitter(os, firstDialect);
+    DialectNamespaceEmitter nsEmitter(os, firstDialect);
     if (firstDialect.isExtensible()) {
       os << llvm::formatv(dialectDefaultAttrPrinterParserDispatch,
                           firstDialect.getCppClassName(),
@@ -1116,7 +1116,7 @@ bool DefGenerator::emitDefs(StringRef selectedDialect) {
 
   // Emit the default parser/printer for Types if the dialect asked for it.
   if (!isAttrGenerator && firstDialect.useDefaultTypePrinterParser()) {
-    NamespaceEmitter nsEmitter(os, firstDialect);
+    DialectNamespaceEmitter nsEmitter(os, firstDialect);
     if (firstDialect.isExtensible()) {
       os << llvm::formatv(dialectDefaultTypePrinterParserDispatch,
                           firstDialect.getCppClassName(),
diff --git a/mlir/tools/mlir-tblgen/DialectGen.cpp b/mlir/tools/mlir-tblgen/DialectGen.cpp
index 02941ec1268cb..2e8810d5d37f4 100644
--- a/mlir/tools/mlir-tblgen/DialectGen.cpp
+++ b/mlir/tools/mlir-tblgen/DialectGen.cpp
@@ -242,7 +242,7 @@ static const char *const discardableAttrHelperDecl = R"(
 static void emitDialectDecl(Dialect &dialect, raw_ostream &os) {
   // Emit all nested namespaces.
   {
-    NamespaceEmitter nsEmitter(os, dialect);
+    DialectNamespaceEmitter nsEmitter(os, dialect);
 
     // Emit the start of the decl.
     std::string cppName = dialect.getCppClassName();
@@ -358,7 +358,7 @@ static void emitDialectDef(Dialect &dialect, const RecordKeeper &records,
        << "::" << cppClassName << ")\n";
 
   // Emit all nested namespaces.
-  NamespaceEmitter nsEmitter(os, dialect);
+  DialectNamespaceEmitter nsEmitter(os, dialect);
 
   /// Build the list of dependent dialects.
   std::string dependentDialectRegistrations;
diff --git a/mlir/tools/mlir-...
[truncated]

@kuhar kuhar removed their request for review October 3, 2025 12:55
@jurahul jurahul requested review from teqdruid and joker-eph October 3, 2025 15:45
@jurahul
Copy link
Contributor Author

jurahul commented Oct 3, 2025

The intent is to adopt this more widely in LLVM TableGen code

@joker-eph
Copy link
Collaborator

There likely should be unit-tests for this in LLVM

@jurahul
Copy link
Contributor Author

jurahul commented Oct 3, 2025

There likely should be unit-tests for this in LLVM

Do you mean lit tests or C++ unit tests? I don't see a lot of C++ unit tests for TableGen (just 3 in unittests/TableGen) but I can add one for these 2 classes. Note that td lit tests also exercise this.

@joker-eph
Copy link
Collaborator

Note that td lit tests also exercise this.

Since you're moving this to LLVM, I don't think there are existing lit tests exercising this, and I don't see any one added/moved in this PR, can you point me to the test coverage in LLVM?

@jurahul
Copy link
Contributor Author

jurahul commented Oct 3, 2025

MLIR still uses this code so its tested there, and on LLVM side directive emitter uses the IfDefEmitter, so those unit test exercise it (llvm/test/TableGen/directive*.td)

@jurahul
Copy link
Contributor Author

jurahul commented Oct 3, 2025

Note that TargetFeaturesEmitter is adopting both, but there is no td unit test that exercises it, but the PowerPC backend does

@jurahul
Copy link
Contributor Author

jurahul commented Oct 3, 2025

Ok, I adopted the NamespaceEmitter in one place, so we now have lit tests in TableGen that exercise both classes. More adoption in LLVM's TableGen will come later

@jurahul jurahul merged commit e9330fd into llvm:main Oct 4, 2025
9 checks passed
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 6, 2025
newling added a commit to iree-org/iree that referenced this pull request Oct 6, 2025
Carries same 2 reverts as previous integrate
#22200
Updates IREE for LLVM tablegen change
llvm/llvm-project#161744

Signed-off-by: James Newling <james.newling@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants