[llvm][PassSupport] don't require passes to be default constructible

[nickdesaulniers]

Summary

Quite a few passes are not default constructible. In order to properly
support -{start|stop}-{before|after}= for these passes, we would like to
continue to use INITIALIZE_PASS, but not necessarily provide a default
constructor.

Delete the default constructors of classes derived from
SelectionDAGISel.

[nickdesaulniers]

• more AMDGPU fixes

[arsenm]

llvm/include/llvm/PassSupport.h

92

Should just be llvm_unreachable? Is this checked anywhere?

[nickdesaulniers]

it builds, but tests crash pretty hard...

[nickdesaulniers]

• fix crashes (don't touch non-SelectionDAGISel derived classes)

[nickdesaulniers]

• add llvm_unreachable

llvm/include/llvm/PassSupport.h

92

Looks like that would be called if you run the pass with opt and LPM: opt -<passname>.

[nickdesaulniers]

• rebase

[efriedma]

llvm/include/llvm/PassSupport.h

92

Please use report_fatal_error for codepaths that might actually be reachable.

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h

96–98

I don't think there's any point to explicitly deleting the default constructor (here and elsewhere); there's no implicitly-declared default constructor if there's an explicit constructor.

[nickdesaulniers]

• report_fatal_error

[nickdesaulniers]

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h

96–98

@arsenm from discord:

really the default constructor should be deleted

@arsenm WDYT? (I don't care either way; but I'd prefer agreement between you and @efriedma on how I should proceed).

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h

96–98

I don't particularly care as long as there's no way to default construct. = delete just makes it clear that you shouldn't add it

[nickdesaulniers]

llvm/include/llvm/PassSupport.h

81–90

I can use C++14 std::enable_if_t to make this slightly more concise here.

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h

96–98

I also prefer being explicit rather than try to remember the rule of 5 or 7 or gang of 4 or whatever. Marking this thread as done, but please re-comment/re-open @efriedma if you feel strongly otherwise.

[nickdesaulniers]

• replace with enable_if w/ enable_if_t (c++14)

[nickdesaulniers]

• git clang-format HEAD~

[efriedma]

LGTM

[arsenm]

llvm/include/llvm/PassSupport.h

95

Dead return

[nickdesaulniers]

• delete dead return

llvm/include/llvm/PassSupport.h

 #endif
 
 #ifndef LLVM_PASSSUPPORT_H
 #define LLVM_PASSSUPPORT_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/PassInfo.h"
 #include "llvm/PassRegistry.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/Threading.h"
 #include <functional>
 
 namespace llvm {
 
 class Pass;
 
 #define INITIALIZE_PASS(passName, arg, name, cfg, analysis)                    \
#define INITIALIZE_PASS_WITH_OPTIONS(PassName, Arg, Name, Cfg, Analysis)       \
   INITIALIZE_PASS_BEGIN(PassName, Arg, Name, Cfg, Analysis)                    \
   PassName::registerOptions();                                                 \
   INITIALIZE_PASS_END(PassName, Arg, Name, Cfg, Analysis)
 
 #define INITIALIZE_PASS_WITH_OPTIONS_BEGIN(PassName, Arg, Name, Cfg, Analysis) \
   INITIALIZE_PASS_BEGIN(PassName, Arg, Name, Cfg, Analysis)                    \
   PassName::registerOptions();
 
-template <typename PassName> Pass *callDefaultCtor() { return new PassName(); }
+template <class PassName,
+          typename std::enable_if<std::is_default_constructible<PassName>{},
+                                  bool>::type = true>
+Pass *callDefaultCtor() {
+  return new PassName();
+}
+
+template <class PassName,
+          typename std::enable_if<!std::is_default_constructible<PassName>{},
+                                  bool>::type = true>
+Pass *callDefaultCtor() {
+  // Some codegen passes should only be testable via
+  // `llc -{start|stop}-{before|after}=<passname>`, not via `opt -<passname>`.
+  report_fatal_error("target-specific codegen-only pass");
+  return nullptr;
+}
 
 //===---------------------------------------------------------------------------
 /// RegisterPass<t> template - This template class is used to notify the system
 /// that a Pass is available for use, and registers it into the internal
 /// database maintained by the PassManager.  Unless this template is used, opt,
 /// for example will not be able to see the pass and attempts to create the pass
 /// will fail. This template is used in the follow manner (at global scope, in
 /// your .cpp file):

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

class AArch64DAGToDAGISel : public SelectionDAGISel {
 
   /// Subtarget - Keep a pointer to the AArch64Subtarget around so that we can
   /// make the right decision when generating code for different targets.
   const AArch64Subtarget *Subtarget;
 
 public:
   static char ID;
 
+  AArch64DAGToDAGISel() = delete;
+
   explicit AArch64DAGToDAGISel(AArch64TargetMachine &tm,
                                CodeGenOpt::Level OptLevel)
       : SelectionDAGISel(ID, tm, OptLevel), Subtarget(nullptr) {}
 
   StringRef getPassName() const override {
     return "AArch64 Instruction Selection";
   }
 

llvm/lib/Target/AMDGPU/AMDGPU.h

struct AMDGPUPromoteAllocaToVectorPass
   AMDGPUPromoteAllocaToVectorPass(TargetMachine &TM) : TM(TM) {}
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 
 private:
   TargetMachine &TM;
 };
 
 Pass *createAMDGPUStructurizeCFGPass();
-FunctionPass *createAMDGPUISelDag(
-  TargetMachine *TM = nullptr,
-  CodeGenOpt::Level OptLevel = CodeGenOpt::Default);
+FunctionPass *createAMDGPUISelDag(TargetMachine &TM,
+                                  CodeGenOpt::Level OptLevel);
 ModulePass *createAMDGPUAlwaysInlinePass(bool GlobalOpt = true);
 
 struct AMDGPUAlwaysInlinePass : PassInfoMixin<AMDGPUAlwaysInlinePass> {
   AMDGPUAlwaysInlinePass(bool GlobalOpt = true) : GlobalOpt(GlobalOpt) {}
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
 private:
   bool GlobalOpt;

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h

class AMDGPUDAGToDAGISel : public SelectionDAGISel {
 
   // Instructions that will be lowered with a final instruction that zeros the
   // high result bits.
   bool fp16SrcZerosHighBits(unsigned Opc) const;
 
 public:
   static char ID;
 
-  explicit AMDGPUDAGToDAGISel(TargetMachine *TM = nullptr,
-                              CodeGenOpt::Level OptLevel = CodeGenOpt::Default);
+  AMDGPUDAGToDAGISel() = delete;
+
+  explicit AMDGPUDAGToDAGISel(TargetMachine &TM, CodeGenOpt::Level OptLevel);
   ~AMDGPUDAGToDAGISel() override = default;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
   bool matchLoadD16FromBuildVector(SDNode *N) const;
 
   bool runOnMachineFunction(MachineFunction &MF) override;
   void PreprocessISelDAG() override;

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
 #endif
 INITIALIZE_PASS_END(AMDGPUDAGToDAGISel, "amdgpu-isel",
                     "AMDGPU DAG->DAG Pattern Instruction Selection", false, false)
 
 /// This pass converts a legalized DAG into a AMDGPU-specific
 // DAG, ready for instruction scheduling.
-FunctionPass *llvm::createAMDGPUISelDag(TargetMachine *TM,
+FunctionPass *llvm::createAMDGPUISelDag(TargetMachine &TM,
                                         CodeGenOpt::Level OptLevel) {
   return new AMDGPUDAGToDAGISel(TM, OptLevel);
 }
 
-AMDGPUDAGToDAGISel::AMDGPUDAGToDAGISel(
-    TargetMachine *TM /*= nullptr*/,
-    CodeGenOpt::Level OptLevel /*= CodeGenOpt::Default*/)
-    : SelectionDAGISel(ID, *TM, OptLevel) {
+AMDGPUDAGToDAGISel::AMDGPUDAGToDAGISel(TargetMachine &TM,
+                                       CodeGenOpt::Level OptLevel)
+    : SelectionDAGISel(ID, TM, OptLevel) {
   EnableLateStructurizeCFG = AMDGPUTargetMachine::EnableLateStructurizeCFG;
 }
 
 bool AMDGPUDAGToDAGISel::runOnMachineFunction(MachineFunction &MF) {
 #ifdef EXPENSIVE_CHECKS
   DominatorTree & DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
   LoopInfo * LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   for (auto &L : LI->getLoopsInPreorder()) {

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

 
 bool AMDGPUPassConfig::addPreISel() {
   if (TM->getOptLevel() > CodeGenOpt::None)
     addPass(createFlattenCFGPass());
   return false;
 }
 
 bool AMDGPUPassConfig::addInstSelector() {
-  addPass(createAMDGPUISelDag(&getAMDGPUTargetMachine(), getOptLevel()));
+  addPass(createAMDGPUISelDag(getAMDGPUTargetMachine(), getOptLevel()));
   return false;
 }
 
 bool AMDGPUPassConfig::addGCPasses() {
   // Do nothing. GC is not supported.
   return false;
 }
 

llvm/lib/Target/AMDGPU/R600.h

 // R600 Passes
 FunctionPass *createR600VectorRegMerger();
 FunctionPass *createR600ExpandSpecialInstrsPass();
 FunctionPass *createR600EmitClauseMarkers();
 FunctionPass *createR600ClauseMergePass();
 FunctionPass *createR600Packetizer();
 FunctionPass *createR600ControlFlowFinalizer();
 FunctionPass *createR600MachineCFGStructurizerPass();
-FunctionPass *createR600ISelDag(TargetMachine *TM, CodeGenOpt::Level OptLevel);
+FunctionPass *createR600ISelDag(TargetMachine &TM, CodeGenOpt::Level OptLevel);
 ModulePass *createR600OpenCLImageTypeLoweringPass();
 
 void initializeR600ClauseMergePassPass(PassRegistry &);
 extern char &R600ClauseMergePassID;
 
 void initializeR600ControlFlowFinalizerPass(PassRegistry &);
 extern char &R600ControlFlowFinalizerID;
 

llvm/lib/Target/AMDGPU/R600ISelDAGToDAG.cpp

class R600DAGToDAGISel : public AMDGPUDAGToDAGISel {
   const R600Subtarget *Subtarget;
 
   bool isConstantLoad(const MemSDNode *N, int cbID) const;
   bool SelectGlobalValueConstantOffset(SDValue Addr, SDValue &IntPtr);
   bool SelectGlobalValueVariableOffset(SDValue Addr, SDValue &BaseReg,
                                        SDValue &Offset);
 
 public:
-  explicit R600DAGToDAGISel(TargetMachine *TM, CodeGenOpt::Level OptLevel)
+  R600DAGToDAGISel() = delete;
+
+  explicit R600DAGToDAGISel(TargetMachine &TM, CodeGenOpt::Level OptLevel)
       : AMDGPUDAGToDAGISel(TM, OptLevel) {}
 
   void Select(SDNode *N) override;
 
   bool SelectADDRIndirect(SDValue Addr, SDValue &Base,
                           SDValue &Offset) override;
   bool SelectADDRVTX_READ(SDValue Addr, SDValue &Base,
                           SDValue &Offset) override;
bool R600DAGToDAGISel::SelectADDRVTX_READ(SDValue Addr, SDValue &Base,
   // Default case, no offset
   Base = Addr;
   Offset = CurDAG->getTargetConstant(0, SDLoc(Addr), MVT::i32);
   return true;
 }
 
 /// This pass converts a legalized DAG into a R600-specific
 // DAG, ready for instruction scheduling.
-FunctionPass *llvm::createR600ISelDag(TargetMachine *TM,
+FunctionPass *llvm::createR600ISelDag(TargetMachine &TM,
                                       CodeGenOpt::Level OptLevel) {
   return new R600DAGToDAGISel(TM, OptLevel);
 }

llvm/lib/Target/AMDGPU/R600TargetMachine.cpp

bool R600PassConfig::addPreISel() {
   AMDGPUPassConfig::addPreISel();
 
   if (EnableR600StructurizeCFG)
     addPass(createStructurizeCFGPass());
   return false;
 }
 
 bool R600PassConfig::addInstSelector() {
-  addPass(createR600ISelDag(&getAMDGPUTargetMachine(), getOptLevel()));
+  addPass(createR600ISelDag(getAMDGPUTargetMachine(), getOptLevel()));
   return false;
 }
 
 void R600PassConfig::addPreRegAlloc() { addPass(createR600VectorRegMerger()); }
 
 void R600PassConfig::addPreSched2() {
   addPass(createR600EmitClauseMarkers());
   if (EnableR600IfConvert)

llvm/lib/Target/X86/X86ISelDAGToDAG.cpp

class X86DAGToDAGISel final : public SelectionDAGISel {
     bool OptForMinSize;
 
     /// Disable direct TLS access through segment registers.
     bool IndirectTlsSegRefs;
 
   public:
     static char ID;
 
+    X86DAGToDAGISel() = delete;
+
     explicit X86DAGToDAGISel(X86TargetMachine &tm, CodeGenOpt::Level OptLevel)
         : SelectionDAGISel(ID, tm, OptLevel), Subtarget(nullptr),
           OptForMinSize(false), IndirectTlsSegRefs(false) {}
 
     StringRef getPassName() const override {
       return "X86 DAG->DAG Instruction Selection";
     }