[llvm] [TableGen] Allow emitter actions to use `const RecordKeeper &` (PR #104716)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 18 10:44:38 PDT 2024


https://github.com/jurahul created https://github.com/llvm/llvm-project/pull/104716

- Refactor TableGen backend options to allow specifying an action function that takes a const reference to `RecordKeeper`. This will enable gradual migration of code to use const references and pointers to `RecordKeeper` and `Record` in the TableGen backend.

- Refactor handling of these action command line options. Split `Action` into 2 global variables `ActionConst` for actions that take a const reference and `ActionNonConst` for actions that take a non-const reference. Also move these variables from the header to the CPP file and added a function `ApplyAction` to apply one of these actions.

- Change some existing actions to take const reference.

- Change global variables in TableGen.cpp to be static instead of being in an anonymous namespace per LLVM coding standards.

>From 5d7311c58ff2ff0f2fdb6ae5105cef6f09052df3 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Sun, 18 Aug 2024 09:06:16 -0700
Subject: [PATCH] [TableGen] Allow emitter actions to use `const RecordKeeper
 &`

- Refactor TableGen backend options to allow specifying an action
  function that takes a const reference to `RecordKeeper`. This
  will enable gradual migration of code to use const references
  and pointers to `RecordKeeper` and `Record` in the TableGen
  backend.

- Refactor handling of these action command line options. Split
  `Action` into 2 global variables `ActionConst` for actions that
  take a const reference and `ActionNonConst` for actions that
  take a non-const reference. Also move these variables from the
  header to the CPP file and added a function `ApplyAction` to
  apply one of these actions.

- Change some existing actions to take const reference.

- Change global variables in TableGen.cpp to be static instead of
  being in an anonymous namespace per LLVM coding standards.
---
 llvm/include/llvm/TableGen/Record.h          |  2 +-
 llvm/include/llvm/TableGen/SetTheory.h       |  2 +-
 llvm/include/llvm/TableGen/TableGenBackend.h | 23 ++++------
 llvm/lib/TableGen/Main.cpp                   | 11 ++---
 llvm/lib/TableGen/SetTheory.cpp              | 27 ++++++-----
 llvm/lib/TableGen/TableGenBackend.cpp        | 48 ++++++++++++++++++--
 llvm/utils/TableGen/DisassemblerEmitter.cpp  |  1 +
 llvm/utils/TableGen/IntrinsicEmitter.cpp     |  4 +-
 llvm/utils/TableGen/TableGen.cpp             | 23 ++++------
 9 files changed, 84 insertions(+), 57 deletions(-)

diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index e6e87eee2c6ba9..e215a06f03a479 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -1757,7 +1757,7 @@ class Record {
   ArrayRef<AssertionInfo> getAssertions() const { return Assertions; }
   ArrayRef<DumpInfo> getDumps() const { return Dumps; }
 
-  ArrayRef<std::pair<Record *, SMRange>>  getSuperClasses() const {
+  ArrayRef<std::pair<Record *, SMRange>> getSuperClasses() const {
     return SuperClasses;
   }
 
diff --git a/llvm/include/llvm/TableGen/SetTheory.h b/llvm/include/llvm/TableGen/SetTheory.h
index 4cff688164b0c4..bf1eb1f5a2141a 100644
--- a/llvm/include/llvm/TableGen/SetTheory.h
+++ b/llvm/include/llvm/TableGen/SetTheory.h
@@ -95,7 +95,7 @@ class SetTheory {
 private:
   // Map set defs to their fully expanded contents. This serves as a memoization
   // cache and it makes it possible to return const references on queries.
-  using ExpandMap = std::map<Record *, RecVec>;
+  using ExpandMap = std::map<const Record *, RecVec>;
   ExpandMap Expansions;
 
   // Known DAG operators by name.
diff --git a/llvm/include/llvm/TableGen/TableGenBackend.h b/llvm/include/llvm/TableGen/TableGenBackend.h
index 9c5a785f45a403..68858d33ed99b6 100644
--- a/llvm/include/llvm/TableGen/TableGenBackend.h
+++ b/llvm/include/llvm/TableGen/TableGenBackend.h
@@ -14,8 +14,6 @@
 #define LLVM_TABLEGEN_TABLEGENBACKEND_H
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/TableGen/Record.h"
 
 namespace llvm {
@@ -24,20 +22,13 @@ class RecordKeeper;
 class raw_ostream;
 
 namespace TableGen::Emitter {
-using FnT = void (*)(RecordKeeper &Records, raw_ostream &OS);
-
-struct OptCreatorT {
-  static void *call();
-};
-
-extern ManagedStatic<cl::opt<FnT>, OptCreatorT> Action;
+// Support for const and non-const forms of action functions.
+using FnNonConstT = void (*)(RecordKeeper &Records, raw_ostream &OS);
+using FnConstT = void (*)(const RecordKeeper &Records, raw_ostream &OS);
 
 struct Opt {
-  Opt(StringRef Name, FnT CB, StringRef Desc, bool ByDefault = false) {
-    if (ByDefault)
-      Action->setInitialValue(CB);
-    Action->getParser().addLiteralOption(Name, CB, Desc);
-  }
+  Opt(StringRef Name, FnNonConstT CB, StringRef Desc, bool ByDefault = false);
+  Opt(StringRef Name, FnConstT CB, StringRef Desc, bool ByDefault = false);
 };
 
 template <class EmitterC> class OptClass : Opt {
@@ -47,6 +38,10 @@ template <class EmitterC> class OptClass : Opt {
   OptClass(StringRef Name, StringRef Desc) : Opt(Name, run, Desc) {}
 };
 
+/// Apply action specified on the command line. Returns false is an action
+/// was applied.
+bool ApplyAction(RecordKeeper &Records, raw_ostream &OS);
+
 } // namespace TableGen::Emitter
 
 /// emitSourceFileHeader - Output an LLVM style file header to the specified
diff --git a/llvm/lib/TableGen/Main.cpp b/llvm/lib/TableGen/Main.cpp
index 841fa7c3f3690b..0f505f78d8ef9b 100644
--- a/llvm/lib/TableGen/Main.cpp
+++ b/llvm/lib/TableGen/Main.cpp
@@ -131,13 +131,10 @@ int llvm::TableGenMain(const char *argv0,
   std::string OutString;
   raw_string_ostream Out(OutString);
   unsigned status = 0;
-  TableGen::Emitter::FnT ActionFn = TableGen::Emitter::Action->getValue();
-  if (ActionFn)
-    ActionFn(Records, Out);
-  else if (MainFn)
-    status = MainFn(Out, Records);
-  else
-    return 1;
+  // ApplyAction will return true if it did not apply any action. In that case,
+  // attempt to apply the MainFn.
+  if (TableGen::Emitter::ApplyAction(Records, Out))
+    status = MainFn ? MainFn(Out, Records) : 1;
   Records.stopBackendTimer();
   if (status)
     return 1;
diff --git a/llvm/lib/TableGen/SetTheory.cpp b/llvm/lib/TableGen/SetTheory.cpp
index f4e3e3d4ce473b..09a49caab8949f 100644
--- a/llvm/lib/TableGen/SetTheory.cpp
+++ b/llvm/lib/TableGen/SetTheory.cpp
@@ -209,8 +209,8 @@ struct SequenceOp : public SetTheory::Operator {
     if (To < 0 || To >= (1 << 30))
       PrintFatalError(Loc, "To out of range");
 
-    RecordKeeper &Records =
-      cast<DefInit>(Expr->getOperator())->getDef()->getRecords();
+    const RecordKeeper &Records =
+        cast<DefInit>(Expr->getOperator())->getDef()->getRecords();
 
     Step *= From <= To ? 1 : -1;
     while (true) {
@@ -311,20 +311,19 @@ const RecVec *SetTheory::expand(Record *Set) {
     return &I->second;
 
   // This is the first time we see Set. Find a suitable expander.
-  ArrayRef<std::pair<Record *, SMRange>> SC = Set->getSuperClasses();
-  for (const auto &SCPair : SC) {
+  for (const auto &[SuperClass, Locs] : Set->getSuperClasses()) {
     // Skip unnamed superclasses.
-    if (!isa<StringInit>(SCPair.first->getNameInit()))
+    if (!isa<StringInit>(SuperClass->getNameInit()))
       continue;
-    auto I = Expanders.find(SCPair.first->getName());
-    if (I != Expanders.end()) {
-      // This breaks recursive definitions.
-      RecVec &EltVec = Expansions[Set];
-      RecSet Elts;
-      I->second->expand(*this, Set, Elts);
-      EltVec.assign(Elts.begin(), Elts.end());
-      return &EltVec;
-    }
+    auto I = Expanders.find(SuperClass->getName());
+    if (I == Expanders.end())
+      continue;
+    // This breaks recursive definitions.
+    RecVec &EltVec = Expansions[Set];
+    RecSet Elts;
+    I->second->expand(*this, Set, Elts);
+    EltVec.assign(Elts.begin(), Elts.end());
+    return &EltVec;
   }
 
   // Set is not expandable.
diff --git a/llvm/lib/TableGen/TableGenBackend.cpp b/llvm/lib/TableGen/TableGenBackend.cpp
index 035abe936e114d..81257b664759be 100644
--- a/llvm/lib/TableGen/TableGenBackend.cpp
+++ b/llvm/lib/TableGen/TableGenBackend.cpp
@@ -12,22 +12,60 @@
 
 #include "llvm/TableGen/TableGenBackend.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
+#include <variant>
 
 using namespace llvm;
+using namespace TableGen;
+using namespace Emitter;
 
 const size_t MAX_LINE_LEN = 80U;
 
-namespace llvm::TableGen::Emitter {
-ManagedStatic<cl::opt<FnT>, OptCreatorT> Action;
-void *OptCreatorT::call() {
-  return new cl::opt<FnT>(cl::desc("Action to perform:"));
+namespace {
+template <typename FnT> struct OptCreatorT {
+  static void *call() {
+    return new cl::opt<FnT>(cl::desc("Action to perform:"));
+  }
+};
+} // namespace
+
+static ManagedStatic<cl::opt<FnNonConstT>, OptCreatorT<FnNonConstT>>
+    ActionNonConst;
+static ManagedStatic<cl::opt<FnConstT>, OptCreatorT<FnConstT>> ActionConst;
+static std::variant<FnNonConstT, FnConstT> DefaultAction;
+
+Opt::Opt(StringRef Name, FnNonConstT CB, StringRef Desc, bool ByDefault) {
+  if (ByDefault)
+    DefaultAction = CB;
+  ActionNonConst->getParser().addLiteralOption(Name, CB, Desc);
+}
+
+Opt::Opt(StringRef Name, FnConstT CB, StringRef Desc, bool ByDefault) {
+  if (ByDefault)
+    DefaultAction = CB;
+  ActionConst->getParser().addLiteralOption(Name, CB, Desc);
+}
+
+bool llvm::TableGen::Emitter::ApplyAction(RecordKeeper &Records,
+                                          raw_ostream &OS) {
+  if (auto Fn = ActionNonConst->getValue())
+    Fn(Records, OS);
+  else if (auto Fn = ActionConst->getValue())
+    Fn(Records, OS);
+  else if (auto *Fn = std::get_if<FnNonConstT>(&DefaultAction); Fn && *Fn)
+    (*Fn)(Records, OS);
+  else if (auto *Fn = std::get_if<FnConstT>(&DefaultAction); Fn && *Fn)
+    (*Fn)(Records, OS);
+  else
+    return true;
+  return false;
 }
-} // namespace llvm::TableGen::Emitter
 
 static void printLine(raw_ostream &OS, const Twine &Prefix, char Fill,
                       StringRef Suffix) {
diff --git a/llvm/utils/TableGen/DisassemblerEmitter.cpp b/llvm/utils/TableGen/DisassemblerEmitter.cpp
index d41750075b41f2..f2c25d38ad2a7d 100644
--- a/llvm/utils/TableGen/DisassemblerEmitter.cpp
+++ b/llvm/utils/TableGen/DisassemblerEmitter.cpp
@@ -11,6 +11,7 @@
 #include "WebAssemblyDisassemblerEmitter.h"
 #include "X86DisassemblerTables.h"
 #include "X86RecognizableInstr.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
diff --git a/llvm/utils/TableGen/IntrinsicEmitter.cpp b/llvm/utils/TableGen/IntrinsicEmitter.cpp
index b1307153e9109b..1d2d13e2925f56 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -681,14 +681,14 @@ void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(
   OS << "#endif\n\n";
 }
 
-static void EmitIntrinsicEnums(RecordKeeper &RK, raw_ostream &OS) {
+static void EmitIntrinsicEnums(const RecordKeeper &RK, raw_ostream &OS) {
   IntrinsicEmitter(RK).run(OS, /*Enums=*/true);
 }
 
 static TableGen::Emitter::Opt X("gen-intrinsic-enums", EmitIntrinsicEnums,
                                 "Generate intrinsic enums");
 
-static void EmitIntrinsicImpl(RecordKeeper &RK, raw_ostream &OS) {
+static void EmitIntrinsicImpl(const RecordKeeper &RK, raw_ostream &OS) {
   IntrinsicEmitter(RK).run(OS, /*Enums=*/false);
 }
 
diff --git a/llvm/utils/TableGen/TableGen.cpp b/llvm/utils/TableGen/TableGen.cpp
index b2ed48cffe6be5..14c42d31149ef7 100644
--- a/llvm/utils/TableGen/TableGen.cpp
+++ b/llvm/utils/TableGen/TableGen.cpp
@@ -33,37 +33,36 @@ cl::opt<bool> EmitLongStrLiterals(
     cl::Hidden, cl::init(true));
 } // end namespace llvm
 
-namespace {
+static cl::OptionCategory PrintEnumsCat("Options for -print-enums");
+static cl::opt<std::string> Class("class",
+                                  cl::desc("Print Enum list for this class"),
+                                  cl::value_desc("class name"),
+                                  cl::cat(PrintEnumsCat));
 
-cl::OptionCategory PrintEnumsCat("Options for -print-enums");
-cl::opt<std::string> Class("class", cl::desc("Print Enum list for this class"),
-                           cl::value_desc("class name"),
-                           cl::cat(PrintEnumsCat));
-
-void PrintRecords(RecordKeeper &Records, raw_ostream &OS) {
+static void PrintRecords(const RecordKeeper &Records, raw_ostream &OS) {
   OS << Records; // No argument, dump all contents
 }
 
-void PrintEnums(RecordKeeper &Records, raw_ostream &OS) {
+static void PrintEnums(RecordKeeper &Records, raw_ostream &OS) {
   for (Record *Rec : Records.getAllDerivedDefinitions(Class))
     OS << Rec->getName() << ", ";
   OS << "\n";
 }
 
-void PrintSets(RecordKeeper &Records, raw_ostream &OS) {
+static void PrintSets(const RecordKeeper &Records, raw_ostream &OS) {
   SetTheory Sets;
   Sets.addFieldExpander("Set", "Elements");
   for (Record *Rec : Records.getAllDerivedDefinitions("Set")) {
     OS << Rec->getName() << " = [";
     const std::vector<Record *> *Elts = Sets.expand(Rec);
     assert(Elts && "Couldn't expand Set instance");
-    for (Record *Elt : *Elts)
+    for (const Record *Elt : *Elts)
       OS << ' ' << Elt->getName();
     OS << " ]\n";
   }
 }
 
-TableGen::Emitter::Opt X[] = {
+static llvm::TableGen::Emitter::Opt X[] = {
     {"print-records", PrintRecords, "Print all records to stdout (default)",
      true},
     {"print-detailed-records", EmitDetailedRecords,
@@ -75,8 +74,6 @@ TableGen::Emitter::Opt X[] = {
     {"print-sets", PrintSets, "Print expanded sets for testing DAG exprs"},
 };
 
-} // namespace
-
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   cl::ParseCommandLineOptions(argc, argv);



More information about the llvm-commits mailing list