[llvm] 34dee0a - [TableGen] Allow emitter callbacks to use `const RecordKeeper &` (#104716)

via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 26 12:10:36 PDT 2024


Author: Rahul Joshi
Date: 2024-08-26T12:10:33-07:00
New Revision: 34dee0a96105d6aeb8b386efbbbfe437ab1be02e

URL: https://github.com/llvm/llvm-project/commit/34dee0a96105d6aeb8b386efbbbfe437ab1be02e
DIFF: https://github.com/llvm/llvm-project/commit/34dee0a96105d6aeb8b386efbbbfe437ab1be02e.diff

LOG: [TableGen] Allow emitter callbacks to use `const RecordKeeper &` (#104716)

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

- Refactor handling of the callback command line options. Move variable
  for the command line option from the header to the CPP file, and add a
  function `ApplyCallback` to apply the selected callback.

- Change callbacks in TableGen.cpp to take const reference. They use the
  `Opt` class to register their callbacks.

- Change IntrinsicEmitter to use the `OptClass` to define its callbacks.
  It already uses a const reference in the implementation.

Added: 
    

Modified: 
    llvm/include/llvm/ADT/STLFunctionalExtras.h
    llvm/include/llvm/TableGen/TableGenBackend.h
    llvm/lib/TableGen/Main.cpp
    llvm/lib/TableGen/TableGenBackend.cpp
    llvm/unittests/ADT/FunctionRefTest.cpp
    llvm/utils/TableGen/DisassemblerEmitter.cpp
    llvm/utils/TableGen/IntrinsicEmitter.cpp
    llvm/utils/TableGen/TableGen.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/STLFunctionalExtras.h b/llvm/include/llvm/ADT/STLFunctionalExtras.h
index dd7fc6dc748645..6f172504b3c167 100644
--- a/llvm/include/llvm/ADT/STLFunctionalExtras.h
+++ b/llvm/include/llvm/ADT/STLFunctionalExtras.h
@@ -69,6 +69,10 @@ class function_ref<Ret(Params...)> {
   }
 
   explicit operator bool() const { return callback; }
+
+  bool operator==(const function_ref<Ret(Params...)> &Other) const {
+    return callable == Other.callable;
+  }
 };
 
 } // end namespace llvm

diff  --git a/llvm/include/llvm/TableGen/TableGenBackend.h b/llvm/include/llvm/TableGen/TableGenBackend.h
index 9c5a785f45a403..80134179850b0e 100644
--- a/llvm/include/llvm/TableGen/TableGenBackend.h
+++ b/llvm/include/llvm/TableGen/TableGenBackend.h
@@ -13,9 +13,8 @@
 #ifndef LLVM_TABLEGEN_TABLEGENBACKEND_H
 #define LLVM_TABLEGEN_TABLEGENBACKEND_H
 
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/TableGen/Record.h"
 
 namespace llvm {
@@ -24,22 +23,19 @@ 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;
+// Supports const and non-const forms of callback functions.
+using FnT = function_ref<void(RecordKeeper &Records, raw_ostream &OS)>;
 
+/// Creating an `Opt` object registers the command line option \p Name with
+/// TableGen backend and associates the callback \p CB with that option. If
+/// \p ByDefault is true, then that callback is applied by default if no
+/// command line option was specified.
 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, FnT CB, StringRef Desc, bool ByDefault = false);
 };
 
+/// Convienence wrapper around `Opt` that registers `EmitterClass::run` as the
+/// callback.
 template <class EmitterC> class OptClass : Opt {
   static void run(RecordKeeper &RK, raw_ostream &OS) { EmitterC(RK).run(OS); }
 
@@ -47,6 +43,10 @@ template <class EmitterC> class OptClass : Opt {
   OptClass(StringRef Name, StringRef Desc) : Opt(Name, run, Desc) {}
 };
 
+/// Apply callback for any command line option registered above. Returns false
+/// is no callback was applied.
+bool ApplyCallback(RecordKeeper &Records, raw_ostream &OS);
+
 } // namespace TableGen::Emitter
 
 /// emitSourceFileHeader - Output an LLVM style file header to the specified
@@ -54,6 +54,6 @@ template <class EmitterC> class OptClass : Opt {
 void emitSourceFileHeader(StringRef Desc, raw_ostream &OS,
                           const RecordKeeper &Record = RecordKeeper());
 
-} // End llvm namespace
+} // namespace llvm
 
-#endif
+#endif // LLVM_TABLEGEN_TABLEGENBACKEND_H

diff  --git a/llvm/lib/TableGen/Main.cpp b/llvm/lib/TableGen/Main.cpp
index 841fa7c3f3690b..a4c41223c07620 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;
+  // ApplyCallback will return true if it did not apply any callback. In that
+  // case, attempt to apply the MainFn.
+  if (TableGen::Emitter::ApplyCallback(Records, Out))
+    status = MainFn ? MainFn(Out, Records) : 1;
   Records.stopBackendTimer();
   if (status)
     return 1;

diff  --git a/llvm/lib/TableGen/TableGenBackend.cpp b/llvm/lib/TableGen/TableGenBackend.cpp
index 035abe936e114d..210fff65458627 100644
--- a/llvm/lib/TableGen/TableGenBackend.cpp
+++ b/llvm/lib/TableGen/TableGenBackend.cpp
@@ -12,6 +12,8 @@
 
 #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>
@@ -19,15 +21,53 @@
 #include <cstddef>
 
 using namespace llvm;
+using namespace TableGen::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:"));
+// CommandLine options of class type are not directly supported with some
+// specific exceptions like std::string which are safe to copy. In our case,
+// the `FnT` function_ref object is also safe to copy. So provide a
+// specialization of `OptionValue` for `FnT` type that stores it as a copy.
+// This is essentially similar to OptionValue<std::string> specialization for
+// strings.
+template <> struct cl::OptionValue<FnT> final : cl::OptionValueCopy<FnT> {
+  OptionValue() = default;
+
+  OptionValue(const FnT &V) { this->setValue(V); }
+
+  OptionValue<FnT> &operator=(const FnT &V) {
+    setValue(V);
+    return *this;
+  }
+};
+
+namespace {
+struct OptCreatorT {
+  static void *call() {
+    return new cl::opt<FnT>(cl::desc("Action to perform:"));
+  }
+};
+} // namespace
+
+static ManagedStatic<cl::opt<FnT>, OptCreatorT> CallbackFunction;
+
+Opt::Opt(StringRef Name, FnT CB, StringRef Desc, bool ByDefault) {
+  if (ByDefault)
+    CallbackFunction->setInitialValue(CB);
+  CallbackFunction->getParser().addLiteralOption(Name, CB, Desc);
+}
+
+/// Apply callback specified on the command line. Returns true if no callback
+/// was applied.
+bool llvm::TableGen::Emitter::ApplyCallback(RecordKeeper &Records,
+                                            raw_ostream &OS) {
+  FnT Fn = CallbackFunction->getValue();
+  if (!Fn)
+    return true;
+  Fn(Records, OS);
+  return false;
 }
-} // namespace llvm::TableGen::Emitter
 
 static void printLine(raw_ostream &OS, const Twine &Prefix, char Fill,
                       StringRef Suffix) {
@@ -59,7 +99,7 @@ void llvm::emitSourceFileHeader(StringRef Desc, raw_ostream &OS,
   printLine(OS, Prefix + "Automatically generated file, do not edit!", ' ',
             Suffix);
 
-  // Print the filename of source file
+  // Print the filename of source file.
   if (!Record.getInputFilename().empty())
     printLine(
         OS, Prefix + "From: " + sys::path::filename(Record.getInputFilename()),

diff  --git a/llvm/unittests/ADT/FunctionRefTest.cpp b/llvm/unittests/ADT/FunctionRefTest.cpp
index 47633cada0f3cc..b1819339730370 100644
--- a/llvm/unittests/ADT/FunctionRefTest.cpp
+++ b/llvm/unittests/ADT/FunctionRefTest.cpp
@@ -59,4 +59,14 @@ TEST(FunctionRefTest, SFINAE) {
   EXPECT_EQ("string", returns([] { return "hello"; }));
 }
 
+TEST(FunctionRefTest, Equality) {
+  function_ref<int()> X = [] { return 1; };
+  function_ref<int()> Y = X;
+  EXPECT_EQ(X, Y);
+
+  const auto Lambda = []() { return 0; };
+  function_ref<int()> A(Lambda), B(Lambda);
+  EXPECT_EQ(A, B);
+}
+
 } // namespace

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 8e536c99f627f5..4c211cdca84c5f 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -65,6 +65,14 @@ class IntrinsicEmitter {
   void EmitIntrinsicToBuiltinMap(const CodeGenIntrinsicTable &Ints,
                                  bool IsClang, raw_ostream &OS);
 };
+
+// Helper class to use with `TableGen::Emitter::OptClass`.
+template <bool Enums> class IntrinsicEmitterOpt : public IntrinsicEmitter {
+public:
+  IntrinsicEmitterOpt(const RecordKeeper &R) : IntrinsicEmitter(R) {}
+  void run(raw_ostream &OS) { IntrinsicEmitter::run(OS, Enums); }
+};
+
 } // End anonymous namespace
 
 //===----------------------------------------------------------------------===//
@@ -770,16 +778,8 @@ Intrinsic::getIntrinsicFor{1}Builtin(StringRef TargetPrefix,
                 UpperCompilerName);
 }
 
-static void EmitIntrinsicEnums(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) {
-  IntrinsicEmitter(RK).run(OS, /*Enums=*/false);
-}
+static TableGen::Emitter::OptClass<IntrinsicEmitterOpt</*Enums=*/true>>
+    X("gen-intrinsic-enums", "Generate intrinsic enums");
 
-static TableGen::Emitter::Opt Y("gen-intrinsic-impl", EmitIntrinsicImpl,
-                                "Generate intrinsic implementation code");
+static TableGen::Emitter::OptClass<IntrinsicEmitterOpt</*Enums=*/false>>
+    Y("gen-intrinsic-impl", "Generate intrinsic implementation code");

diff  --git a/llvm/utils/TableGen/TableGen.cpp b/llvm/utils/TableGen/TableGen.cpp
index c420843574cbf3..7ee6fa5c832114 100644
--- a/llvm/utils/TableGen/TableGen.cpp
+++ b/llvm/utils/TableGen/TableGen.cpp
@@ -39,7 +39,7 @@ static cl::opt<std::string> Class("class",
                                   cl::value_desc("class name"),
                                   cl::cat(PrintEnumsCat));
 
-static void PrintRecords(RecordKeeper &Records, raw_ostream &OS) {
+static void PrintRecords(const RecordKeeper &Records, raw_ostream &OS) {
   OS << Records; // No argument, dump all contents
 }
 
@@ -49,14 +49,14 @@ static void PrintEnums(RecordKeeper &Records, raw_ostream &OS) {
   OS << "\n";
 }
 
-static 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";
   }
@@ -67,7 +67,7 @@ static TableGen::Emitter::Opt X[] = {
      true},
     {"print-detailed-records", EmitDetailedRecords,
      "Print full details of all records to stdout"},
-    {"null-backend", [](RecordKeeper &Records, raw_ostream &OS) {},
+    {"null-backend", [](const RecordKeeper &Records, raw_ostream &OS) {},
      "Do nothing after parsing (useful for timing)"},
     {"dump-json", EmitJSON, "Dump all records as machine-readable JSON"},
     {"print-enums", PrintEnums, "Print enum values for a class"},


        


More information about the llvm-commits mailing list