Use template instead of void* -based type erasure in MachineRegistry

[serge-sans-paille]

Summary

Type safe version of MachinePassRegistry

Previous version used type erasure through a void* (*)() pointer,
which triggered gcc warning and implied a lot of reinterpret_cast.

This version should make it harder to hit ourselves in the foot.

[kristina]

Pass manager related stuff, probably good idea to have Chandler or Fedor look over it since they've been doing a lot of work related to PM infrastructure.

[chandlerc]

This is the legacy PM, and specifically as it is implemented in codegen, so my opinions are few and far between... Not sure the benefits here are all that important, but maybe? Looping in folks w/ more backend feelings about this kind of thing.

[MatzeB]

meh

The whole registry system isn't used much. The whole machine seems a little overengineered in retrospect (there only ever is a single listener for example). But making it typesafe seems sensible.

[MatzeB]

Take my "meh" as LGTM if nobody objects in the next couple days.

[serge-sans-paille]

ok, thanks. The only goal of that patch is to silent disturbing warning, I'll apply it at the end of the day.

llvm/trunk/include/llvm/CodeGen/MachinePassRegistry.h

 #define LLVM_CODEGEN_MACHINEPASSREGISTRY_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 
-using MachinePassCtor = void *(*)();
-
 //===----------------------------------------------------------------------===//
 ///
 /// MachinePassRegistryListener - Listener to adds and removals of nodes in
 /// registration list.
 ///
 //===----------------------------------------------------------------------===//
-class MachinePassRegistryListener {
-  virtual void anchor();
+template <class PassCtorTy> class MachinePassRegistryListener {
+  virtual void anchor() {}
 
 public:
   MachinePassRegistryListener() = default;
   virtual ~MachinePassRegistryListener() = default;
 
-  virtual void NotifyAdd(StringRef N, MachinePassCtor C, StringRef D) = 0;
+  virtual void NotifyAdd(StringRef N, PassCtorTy C, StringRef D) = 0;
   virtual void NotifyRemove(StringRef N) = 0;
 };
 
 //===----------------------------------------------------------------------===//
 ///
 /// MachinePassRegistryNode - Machine pass node stored in registration list.
 ///
 //===----------------------------------------------------------------------===//
-class MachinePassRegistryNode {
+template <typename PassCtorTy> class MachinePassRegistryNode {
 private:
   MachinePassRegistryNode *Next = nullptr; // Next function pass in list.
   StringRef Name;                       // Name of function pass.
   StringRef Description;                // Description string.
-  MachinePassCtor Ctor;                 // Function pass creator.
+  PassCtorTy Ctor;                      // Pass creator.
 
 public:
-  MachinePassRegistryNode(const char *N, const char *D, MachinePassCtor C)
+  MachinePassRegistryNode(const char *N, const char *D, PassCtorTy C)
       : Name(N), Description(D), Ctor(C) {}
 
   // Accessors
   MachinePassRegistryNode *getNext()      const { return Next; }
   MachinePassRegistryNode **getNextAddress()    { return &Next; }
   StringRef getName()                   const { return Name; }
   StringRef getDescription()            const { return Description; }
-  MachinePassCtor getCtor()               const { return Ctor; }
+  PassCtorTy getCtor() const { return Ctor; }
   void setNext(MachinePassRegistryNode *N)      { Next = N; }
 };
 
 //===----------------------------------------------------------------------===//
 ///
 /// MachinePassRegistry - Track the registration of machine passes.
 ///
 //===----------------------------------------------------------------------===//
-class MachinePassRegistry {
+template <typename PassCtorTy> class MachinePassRegistry {
 private:
-  MachinePassRegistryNode *List;        // List of registry nodes.
-  MachinePassCtor Default;              // Default function pass creator.
-  MachinePassRegistryListener *Listener; // Listener for list adds are removes.
+  MachinePassRegistryNode<PassCtorTy> *List; // List of registry nodes.
+  PassCtorTy Default;                        // Default function pass creator.
+  MachinePassRegistryListener<PassCtorTy>
+      *Listener; // Listener for list adds are removes.
 
 public:
   // NO CONSTRUCTOR - we don't want static constructor ordering to mess
   // with the registry.
 
   // Accessors.
   //
-  MachinePassRegistryNode *getList()                    { return List; }
-  MachinePassCtor getDefault()                          { return Default; }
-  void setDefault(MachinePassCtor C)                    { Default = C; }
-  void setDefault(StringRef Name);
-  void setListener(MachinePassRegistryListener *L)      { Listener = L; }
+  MachinePassRegistryNode<PassCtorTy> *getList() { return List; }
+  PassCtorTy getDefault() { return Default; }
+  void setDefault(PassCtorTy C) { Default = C; }
+  /// setDefault - Set the default constructor by name.
+  void setDefault(StringRef Name) {
+    PassCtorTy Ctor = nullptr;
+    for (MachinePassRegistryNode<PassCtorTy> *R = getList(); R;
+         R = R->getNext()) {
+      if (R->getName() == Name) {
+        Ctor = R->getCtor();
+        break;
+      }
+    }
+    assert(Ctor && "Unregistered pass name");
+    setDefault(Ctor);
+  }
+  void setListener(MachinePassRegistryListener<PassCtorTy> *L) { Listener = L; }
 
   /// Add - Adds a function pass to the registration list.
   ///
-  void Add(MachinePassRegistryNode *Node);
+  void Add(MachinePassRegistryNode<PassCtorTy> *Node) {
+    Node->setNext(List);
+    List = Node;
+    if (Listener)
+      Listener->NotifyAdd(Node->getName(), Node->getCtor(),
+                          Node->getDescription());
+  }
 
   /// Remove - Removes a function pass from the registration list.
   ///
-  void Remove(MachinePassRegistryNode *Node);
+  void Remove(MachinePassRegistryNode<PassCtorTy> *Node) {
+    for (MachinePassRegistryNode<PassCtorTy> **I = &List; *I;
+         I = (*I)->getNextAddress()) {
+      if (*I == Node) {
+        if (Listener)
+          Listener->NotifyRemove(Node->getName());
+        *I = (*I)->getNext();
+        break;
+      }
+    }
+  }
 };
 
 //===----------------------------------------------------------------------===//
 ///
 /// RegisterPassParser class - Handle the addition of new machine passes.
 ///
 //===----------------------------------------------------------------------===//
-template<class RegistryClass>
-class RegisterPassParser : public MachinePassRegistryListener,
-                   public cl::parser<typename RegistryClass::FunctionPassCtor> {
+template <class RegistryClass>
+class RegisterPassParser
+    : public MachinePassRegistryListener<
+          typename RegistryClass::FunctionPassCtor>,
 public:
   RegisterPassParser(cl::Option &O)
       : cl::parser<typename RegistryClass::FunctionPassCtor>(O) {}
   ~RegisterPassParser() override { RegistryClass::setListener(nullptr); }
 
   void initialize() {
     cl::parser<typename RegistryClass::FunctionPassCtor>::initialize();
 
     // Add existing passes to option.
     for (RegistryClass *Node = RegistryClass::getList();
          Node; Node = Node->getNext()) {
       this->addLiteralOption(Node->getName(),
                       (typename RegistryClass::FunctionPassCtor)Node->getCtor(),
                              Node->getDescription());
     }
 
     // Make sure we listen for list changes.
     RegistryClass::setListener(this);
   }
 
   // Implement the MachinePassRegistryListener callbacks.
-  void NotifyAdd(StringRef N, MachinePassCtor C, StringRef D) override {
-    this->addLiteralOption(N, (typename RegistryClass::FunctionPassCtor)C, D);
+  void NotifyAdd(StringRef N, typename RegistryClass::FunctionPassCtor C,
+                 StringRef D) override {
+    this->addLiteralOption(N, C, D);
   }
   void NotifyRemove(StringRef N) override {
     this->removeLiteralOption(N);
   }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_MACHINEPASSREGISTRY_H

llvm/trunk/include/llvm/CodeGen/MachineScheduler.h

struct MachineSchedContext {
   RegisterClassInfo *RegClassInfo;
 
   MachineSchedContext();
   virtual ~MachineSchedContext();
 };
 
 /// MachineSchedRegistry provides a selection of available machine instruction
 /// schedulers.
-class MachineSchedRegistry : public MachinePassRegistryNode {
+class MachineSchedRegistry
+    : public MachinePassRegistryNode<
+          ScheduleDAGInstrs *(*)(MachineSchedContext *)> {
 public:
   using ScheduleDAGCtor = ScheduleDAGInstrs *(*)(MachineSchedContext *);
 
   // RegisterPassParser requires a (misnamed) FunctionPassCtor type.
   using FunctionPassCtor = ScheduleDAGCtor;
 
-  static MachinePassRegistry Registry;
+  static MachinePassRegistry<ScheduleDAGCtor> Registry;
 
   MachineSchedRegistry(const char *N, const char *D, ScheduleDAGCtor C)
-    : MachinePassRegistryNode(N, D, (MachinePassCtor)C) {
+      : MachinePassRegistryNode(N, D, C) {
     Registry.Add(this);
   }
 
   ~MachineSchedRegistry() { Registry.Remove(this); }
 
   // Accessors.
   //
   MachineSchedRegistry *getNext() const {
     return (MachineSchedRegistry *)MachinePassRegistryNode::getNext();
   }
 
   static MachineSchedRegistry *getList() {
     return (MachineSchedRegistry *)Registry.getList();
   }
 
-  static void setListener(MachinePassRegistryListener *L) {
+  static void setListener(MachinePassRegistryListener<FunctionPassCtor> *L) {
     Registry.setListener(L);
   }
 };
 
 class ScheduleDAGMI;
 
 /// Define a generic scheduling policy for targets that don't provide their own
 /// MachineSchedStrategy. This can be overriden for each scheduling region

llvm/trunk/include/llvm/CodeGen/RegAllocRegistry.h

 
 class FunctionPass;
 
 //===----------------------------------------------------------------------===//
 ///
 /// RegisterRegAlloc class - Track the registration of register allocators.
 ///
 //===----------------------------------------------------------------------===//
-class RegisterRegAlloc : public MachinePassRegistryNode {
+class RegisterRegAlloc : public MachinePassRegistryNode<FunctionPass *(*)()> {
 public:
   using FunctionPassCtor = FunctionPass *(*)();
 
-  static MachinePassRegistry Registry;
+  static MachinePassRegistry<FunctionPassCtor> Registry;
 
   RegisterRegAlloc(const char *N, const char *D, FunctionPassCtor C)
-      : MachinePassRegistryNode(N, D, (MachinePassCtor)C) {
+      : MachinePassRegistryNode(N, D, C) {
     Registry.Add(this);
   }
 
   ~RegisterRegAlloc() { Registry.Remove(this); }
 
   // Accessors.
   RegisterRegAlloc *getNext() const {
     return (RegisterRegAlloc *)MachinePassRegistryNode::getNext();
   }
 
   static RegisterRegAlloc *getList() {
     return (RegisterRegAlloc *)Registry.getList();
   }
 
-  static FunctionPassCtor getDefault() {
-    return (FunctionPassCtor)Registry.getDefault();
-  }
+  static FunctionPassCtor getDefault() { return Registry.getDefault(); }
 
-  static void setDefault(FunctionPassCtor C) {
-    Registry.setDefault((MachinePassCtor)C);
-  }
+  static void setDefault(FunctionPassCtor C) { Registry.setDefault(C); }
 
-  static void setListener(MachinePassRegistryListener *L) {
+  static void setListener(MachinePassRegistryListener<FunctionPassCtor> *L) {
     Registry.setListener(L);
   }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_REGALLOCREGISTRY_H

llvm/trunk/include/llvm/CodeGen/SchedulerRegistry.h

 ///
 /// RegisterScheduler class - Track the registration of instruction schedulers.
 ///
 //===----------------------------------------------------------------------===//
 
 class ScheduleDAGSDNodes;
 class SelectionDAGISel;
 
-class RegisterScheduler : public MachinePassRegistryNode {
+class RegisterScheduler
+    : public MachinePassRegistryNode<
+          ScheduleDAGSDNodes *(*)(SelectionDAGISel *, CodeGenOpt::Level)> {
 public:
   using FunctionPassCtor = ScheduleDAGSDNodes *(*)(SelectionDAGISel*,
                                                    CodeGenOpt::Level);
 
-  static MachinePassRegistry Registry;
+  static MachinePassRegistry<FunctionPassCtor> Registry;
 
   RegisterScheduler(const char *N, const char *D, FunctionPassCtor C)
-  : MachinePassRegistryNode(N, D, (MachinePassCtor)C)
-  { Registry.Add(this); }
+      : MachinePassRegistryNode(N, D, C) {
+    Registry.Add(this);
+  }
   ~RegisterScheduler() { Registry.Remove(this); }
 
 
   // Accessors.
   RegisterScheduler *getNext() const {
     return (RegisterScheduler *)MachinePassRegistryNode::getNext();
   }
 
   static RegisterScheduler *getList() {
     return (RegisterScheduler *)Registry.getList();
   }
 
-  static void setListener(MachinePassRegistryListener *L) {
+  static void setListener(MachinePassRegistryListener<FunctionPassCtor> *L) {
     Registry.setListener(L);
   }
 };
 
 /// createBURRListDAGScheduler - This creates a bottom up register usage
 /// reduction list scheduler.
 ScheduleDAGSDNodes *createBURRListDAGScheduler(SelectionDAGISel *IS,
                                                CodeGenOpt::Level OptLevel);

llvm/trunk/lib/CodeGen/MachinePassRegistry.cpp

 // This file contains the machine function pass registry for register allocators
 // and instruction schedulers.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/MachinePassRegistry.h"
 
 using namespace llvm;
-
-void MachinePassRegistryListener::anchor() { }
-
-/// setDefault - Set the default constructor by name.
-void MachinePassRegistry::setDefault(StringRef Name) {
-  MachinePassCtor Ctor = nullptr;
-  for(MachinePassRegistryNode *R = getList(); R; R = R->getNext()) {
-    if (R->getName() == Name) {
-      Ctor = R->getCtor();
-      break;
-    }
-  }
-  assert(Ctor && "Unregistered pass name");
-  setDefault(Ctor);
-}
-
-/// Add - Adds a function pass to the registration list.
-///
-void MachinePassRegistry::Add(MachinePassRegistryNode *Node) {
-  Node->setNext(List);
-  List = Node;
-  if (Listener) Listener->NotifyAdd(Node->getName(),
-                                    Node->getCtor(),
-                                    Node->getDescription());
-}
-
-
-/// Remove - Removes a function pass from the registration list.
-///
-void MachinePassRegistry::Remove(MachinePassRegistryNode *Node) {
-  for (MachinePassRegistryNode **I = &List; *I; I = (*I)->getNextAddress()) {
-    if (*I == Node) {
-      if (Listener) Listener->NotifyRemove(Node->getName());
-      *I = (*I)->getNext();
-      break;
-    }
-  }
-}

llvm/trunk/lib/CodeGen/MachineScheduler.cpp

 void PostMachineScheduler::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesCFG();
   AU.addRequiredID(MachineDominatorsID);
   AU.addRequired<MachineLoopInfo>();
   AU.addRequired<TargetPassConfig>();
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-MachinePassRegistry MachineSchedRegistry::Registry;
+MachinePassRegistry<MachineSchedRegistry::ScheduleDAGCtor>
+    MachineSchedRegistry::Registry;
 
 /// A dummy default scheduler factory indicates whether the scheduler
 /// is overridden on the command line.
 static ScheduleDAGInstrs *useDefaultMachineSched(MachineSchedContext *C) {
   return nullptr;
 }
 
 /// MachineSchedOpt allows command line selection of the scheduler.

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

static const bool ViewDAGCombine1 = false,
                   ViewSUnitDAGs = false;
 #endif
 
 //===---------------------------------------------------------------------===//
 ///
 /// RegisterScheduler class - Track the registration of instruction schedulers.
 ///
 //===---------------------------------------------------------------------===//
-MachinePassRegistry RegisterScheduler::Registry;
+MachinePassRegistry<RegisterScheduler::FunctionPassCtor>
+    RegisterScheduler::Registry;
 
 //===---------------------------------------------------------------------===//
 ///
 /// ISHeuristic command line option for instruction schedulers.
 ///
 //===---------------------------------------------------------------------===//
 static cl::opt<RegisterScheduler::FunctionPassCtor, false,
                RegisterPassParser<RegisterScheduler>>

llvm/trunk/lib/CodeGen/TargetPassConfig.cpp

bool TargetPassConfig::getOptimizeRegAlloc() const {
   case cl::BOU_UNSET: return getOptLevel() != CodeGenOpt::None;
   case cl::BOU_TRUE:  return true;
   case cl::BOU_FALSE: return false;
   }
   llvm_unreachable("Invalid optimize-regalloc state");
 }
 
 /// RegisterRegAlloc's global Registry tracks allocator registration.
-MachinePassRegistry RegisterRegAlloc::Registry;
+MachinePassRegistry<RegisterRegAlloc::FunctionPassCtor>
+    RegisterRegAlloc::Registry;
 
 /// A dummy default pass factory indicates whether the register allocator is
 /// overridden on the command line.
 static llvm::once_flag InitializeDefaultRegisterAllocatorFlag;
 
 static RegisterRegAlloc
 defaultRegAlloc("default",
                 "pick register allocator based on -O option",