[NewPM][CodeGen] add TargetPassConfig like API

[paperchalice]

Add TargetPassConfig like API in CodeGenBuilder, most of them are copied from TargetPassConfig.cpp except GC related pass.
Part of #69879, blocked by #70904.
@arsenm Could you have a look here? Thanks!

[paperchalice]

Which part should run MachineVerifier? If in MachinePassManager, it can simplify the code in CodeGenPassBuilder.
The MachinePassManager already has following codes:

llvm-project/llvm/lib/CodeGen/MachinePassManager.cpp

Lines 37 to 50 in fd8be1e

	if (VerifyMachineFunction) {
	PassInstrumentation PI = MFAM.getResult<PassInstrumentationAnalysis>(M);
	// No need to pop this callback later since MIR pipeline is flat which means
	// current pipeline is the top-level pipeline. Callbacks are not used after
	// current pipeline.
	PI.pushBeforeNonSkippedPassCallback([&MFAM](StringRef PassID, Any IR) {
	assert(llvm::any_cast<const MachineFunction *>(&IR));
	const MachineFunction *MF = llvm::any_cast<const MachineFunction *>(IR);
	assert(MF && "Machine function should be valid for printing");
	std::string Banner = std::string("After ") + std::string(PassID);
	verifyMachineFunction(&MFAM, Banner, *MF);
	});
	}

If in CodeGenPassBuilder then llc can support flags like -stop-after=machineverifier,n, and compatible with the old behavior.

[arsenm]

Might as well turn this to early return and reduce indentation

[arsenm]

I'd expect the "After" part of the banner to be produced during the emission of the message down after the actual addition, and this would just pass the raw pass name. I guess this is a pre-existing wart

[arsenm]

Typo passs

[arsenm]

Which part should run MachineVerifier? If in MachinePassManager, it can simplify the code in CodeGenPassBuilder. The MachinePassManager already has following codes:

Not sure I understand the question

If in CodeGenPassBuilder then llc can support flags like -stop-after=machineverifier,n, and compatible with the old behavior.

What else would we do if not preserve this behavior?

[paperchalice]

Which part should run MachineVerifier? If in MachinePassManager, it can simplify the code in CodeGenPassBuilder. The MachinePassManager already has following codes:

Not sure I understand the question

If in CodeGenPassBuilder then llc can support flags like -stop-after=machineverifier,n, and compatible with the old behavior.

What else would we do if not preserve this behavior?

There are several tests use -stop-after=machineverifier to stop after the first machine pass. if we verify machine functions in PassInstrumentation, we would replace machineverifier with a concrete pass name e.g. early-tail-duplicate or local-stack-slot-allocation, which depends on the optimization level.

[paperchalice]

Currently, it might be more reasonable for me to add MachineVerifierPass in CodeGenBuilder, due to the flat design of MachineFunctionPassManager, otherwise, we need to extend StandardInstrumentation. But in future, we should move "verify" and "debugyfiy" to instrumentation, like VerifyInstrumentation.
BTW, I don't know why verifier runs before any machine pass and the banner is After ... in code base. It is also not friendly to module pass.

llvm-project/llvm/lib/CodeGen/MachinePassManager.cpp

Lines 43 to 49 in 4effdc4

	PI.pushBeforeNonSkippedPassCallback([&MFAM](StringRef PassID, Any IR) {
	assert(llvm::any_cast<const MachineFunction *>(&IR));
	const MachineFunction *MF = llvm::any_cast<const MachineFunction *>(IR);
	assert(MF && "Machine function should be valid for printing");
	std::string Banner = std::string("After ") + std::string(PassID);
	verifyMachineFunction(&MFAM, Banner, *MF);
	});

[paperchalice]

Rebased. Move debugify related passes to pass manager.

[paperchalice]

Ping? @aeubanks

[aeubanks]

initial review, I'll need to spend some more time understanding the current implementation of CodeGenPassBuilder; there are some things that don't seem very elegant at a cursory glance

[aeubanks]

can make this pure virtual

[aeubanks]

ping

[paperchalice]

CodeGenPassBuilder is a CRTP based class, just delete it or an implementation that always return true would be better?

[aeubanks]

why is this necessary?

[paperchalice]

Just follow the PassBuilder, PIC is allowed to be nullptr.

[aeubanks]

PassBuilder doesn't have a static PIC though. it's ok if it's nullptr since that's handled here

[paperchalice]

Currently this method is referenced by disablePass, which is used by several backends (AMDGPU, NVPTX, SPRIV and WASM).

[aeubanks]

similar to the start/stop, I think this would be better handled by not adding the pass at all to the pipeline by checking in addPass, rather than working around it later on using pass instrumentation

[aeubanks]

shouldn't this something that goes into PIC, rather than a separate RAII class? a random

PrePostPasses Hook(...);

in the code below goes against the entire purpose of pass instrumentation

[paperchalice]

Yes, put them in instrument would be a better choice. Currently the IR pass instrument cannot distinguish between machine module (or machine module info) and IR module, because it always passes module to the callbacks in current implementation.
https://github.com/llvm/llvm-project/blob/5caae72d1a4f58c9525977a93d86c3c833da4b34/llvm/lib/CodeGen/MachinePassManager.cpp#L62C8-L67
IIUC, pass instrumentations should not modify IR units, IR units are qualified with const, but in codegen phase these two hook passes modify the IR units indeed.
Anyway, I will try to add some instruments that belongs to codegen, e.g. verifier, which needs a different initializer, because it needs some analysis information.

[aeubanks]

Can we defer the debugify stuff to a later patch and figure out a cleaner way of implementing it, and put the verifier stuff in pass instrumentation (can also do that in a separate change)? I'd really like to not have this PrePostPasses in the middle of MachineFunctionPassManager::run

[aeubanks]

I realize that you didn't add this, but is it possible to do something like PassBuilder extension point callbacks? This seems very ad-hoc.

[paperchalice]

Currently not. I checked backends codes, seems only insertPass, disablePass and substitute some specific passes (in fact,
only PostRAScheduler) are needed but adding extension points is ok.

[paperchalice]

Address some trivial fixes.

[aeubanks]

let's drop this for now since we don't even support it right now, and I'd like to move off of CallGraphAnalysis and switch everything to use LazyCallGraph

[paperchalice]

Done. BTW, AMDGPU relies on this.

[aeubanks]

Can we defer the debugify stuff to a later patch and figure out a cleaner way of implementing it, and put the verifier stuff in pass instrumentation (can also do that in a separate change)? I'd really like to not have this PrePostPasses in the middle of MachineFunctionPassManager::run

[aeubanks]

ultra nit: extra ;

[paperchalice]

Done

[aeubanks]

unnecessary llvm::?

[paperchalice]

Done

[aeubanks]

why this change?

[paperchalice]

Reverted, just want to keep the include style with normal header files.

[aeubanks]

ping

[aeubanks]

PassBuilder doesn't have a static PIC though. it's ok if it's nullptr since that's handled here

[aeubanks]

is this one used?

[paperchalice]

Deleted.

[paperchalice]

Could move this part to PassBuilder.cpp so it can reuse codes and support machine passes with parameter. Also, many backends provide options to enable/disable passes, plan to unify them with options like enable-passes=pass1<param>,.../disable-passes=pass1,pass2....

[aeubanks]

this FIXME doesn't look like it's in the right place

[aeubanks]

why introduce a variable?

[paperchalice]

Propagate error correctly now.

[aeubanks]

remove

[aeubanks]

similar to the start/stop, I think this would be better handled by not adding the pass at all to the pipeline by checking in addPass, rather than working around it later on using pass instrumentation

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[paperchalice]

Will refactor related code if #70912 is merged.

[yashssh]

Can you state the dependency order of these patches so that I can build them locally, I'm facing some issues right now.
Thanks

[paperchalice]

Can you state the dependency order of these patches so that I can build them locally, I'm facing some issues right now. Thanks

Sorry for the breaking, currently it needs #77084 to compile, the original POC PR is #69879, but as the porting work progressed, it was found that there were some problems with the original implementation, will refine this PR in future.

[yashssh]

Is there an order in which I should bring in all these open pull requests(particularly #70906 #77084 #70912 #70922 #76380 #70921) locally? I understand these are still under development but I need to test something urgently.

[paperchalice]

Is there an order in which I should bring in all these open pull requests(particularly #70906 #77084 #70912 #70922 #76380 #70921) locally? I understand these are still under development but I need to test something urgently.

There is no explicit dependency as far as I know, but they may need rebase to resolve confliction. Since all (?) target independent IR passes are ported or in review, I will focus on these patches these days.
BTW, if you have suggestions during the test, any comments are welcome.

[paperchalice]

Remove the MachinePassKey use pass name instead, add place holder for register allocator, remove some functions that are already implemented by PassBuilder.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: None (paperchalice)

Changes

Add TargetPassConfig like API in CodeGenBuilder, most of them are copied from TargetPassConfig.cpp except GC related pass.
Part of #69879, blocked by #70904.
@arsenm Could you have a look here? Thanks!

---

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

14 Files Affected:

• (modified) llvm/include/llvm/CodeGen/MachinePassManager.h (+4-16)
• (modified) llvm/include/llvm/Passes/CodeGenPassBuilder.h (+251-222)
• (modified) llvm/include/llvm/Passes/MachinePassRegistry.def (+17-5)
• (modified) llvm/include/llvm/Passes/PassBuilder.h (+14)
• (modified) llvm/include/llvm/Target/TargetMachine.h (+3-3)
• (modified) llvm/lib/CodeGen/MachinePassManager.cpp (+2-16)
• (modified) llvm/lib/Passes/CodeGenPassBuilder.cpp (-5)
• (modified) llvm/lib/Passes/PassBuilder.cpp (+37)
• (modified) llvm/lib/Target/X86/CMakeLists.txt (+1)
• (modified) llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp (+5-6)
• (modified) llvm/lib/Target/X86/X86TargetMachine.h (+3-3)
• (modified) llvm/test/tools/llc/new-pm/pipeline.ll (+1-2)
• (modified) llvm/tools/llc/NewPMDriver.cpp (+1-1)
• (modified) llvm/unittests/MIR/PassBuilderCallbacksTest.cpp (-4)

diff --git a/llvm/include/llvm/CodeGen/MachinePassManager.h b/llvm/include/llvm/CodeGen/MachinePassManager.h
index a2641a8223646db..e69b10b7eaafde9 100644
--- a/llvm/include/llvm/CodeGen/MachinePassManager.h
+++ b/llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -27,6 +27,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Target/CGPassBuilderOption.h"
 
 #include <map>
 
@@ -37,20 +38,13 @@ class MachineFunction;
 
 extern template class AnalysisManager<MachineFunction>;
 
-/// Like \c AnalysisKey, but only for machine passes.
-struct alignas(8) MachinePassKey {};
-
 /// A CRTP mix-in that provides informational APIs needed for machine passes.
 ///
 /// This provides some boilerplate for types that are machine passes. It
 /// automatically mixes in \c PassInfoMixin.
 template <typename DerivedT>
 struct MachinePassInfoMixin : public PassInfoMixin<DerivedT> {
-  static MachinePassKey *ID() {
-    static_assert(std::is_base_of<MachinePassInfoMixin, DerivedT>::value,
-                  "Must pass the derived type as the template argument!");
-    return &DerivedT::Key;
-  }
+  // TODO: Add MachineFunctionProperties support.
 };
 
 /// An AnalysisManager<MachineFunction> that also exposes IR analysis results.
@@ -150,10 +144,7 @@ class MachineFunctionPassManager
   using Base = PassManager<MachineFunction, MachineFunctionAnalysisManager>;
 
 public:
-  MachineFunctionPassManager(bool RequireCodeGenSCCOrder = false,
-                             bool VerifyMachineFunction = false)
-      : RequireCodeGenSCCOrder(RequireCodeGenSCCOrder),
-        VerifyMachineFunction(VerifyMachineFunction) {}
+  MachineFunctionPassManager() : Opt(getCGPassBuilderOption()) {}
   MachineFunctionPassManager(MachineFunctionPassManager &&) = default;
   MachineFunctionPassManager &
   operator=(MachineFunctionPassManager &&) = default;
@@ -261,10 +252,7 @@ class MachineFunctionPassManager
   using PassIndex = decltype(Passes)::size_type;
   std::map<PassIndex, llvm::unique_function<FuncTy>> MachineModulePasses;
 
-  // Run codegen in the SCC order.
-  bool RequireCodeGenSCCOrder;
-
-  bool VerifyMachineFunction;
+  CGPassBuilderOption Opt;
 };
 
 } // end namespace llvm
diff --git a/llvm/include/llvm/Passes/CodeGenPassBuilder.h b/llvm/include/llvm/Passes/CodeGenPassBuilder.h
index 07afac3bcf8401f..9dc58f3ed85671f 100644
--- a/llvm/include/llvm/Passes/CodeGenPassBuilder.h
+++ b/llvm/include/llvm/Passes/CodeGenPassBuilder.h
@@ -27,8 +27,11 @@
 #include "llvm/CodeGen/CallBrPrepare.h"
 #include "llvm/CodeGen/CodeGenPrepare.h"
 #include "llvm/CodeGen/DwarfEHPrepare.h"
+#include "llvm/CodeGen/ExpandLargeDivRem.h"
+#include "llvm/CodeGen/ExpandLargeFpConvert.h"
 #include "llvm/CodeGen/ExpandMemCmp.h"
 #include "llvm/CodeGen/ExpandReductions.h"
+#include "llvm/CodeGen/ExpandVectorPredication.h"
 #include "llvm/CodeGen/GCMetadata.h"
 #include "llvm/CodeGen/GlobalMerge.h"
 #include "llvm/CodeGen/IndirectBrExpand.h"
@@ -53,10 +56,12 @@
 #include "llvm/IRPrinter/IRPrintingPasses.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
+#include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/WithColor.h"
 #include "llvm/Target/CGPassBuilderOption.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/CFGuard.h"
@@ -67,7 +72,10 @@
 #include "llvm/Transforms/Scalar/MergeICmps.h"
 #include "llvm/Transforms/Scalar/PartiallyInlineLibCalls.h"
 #include "llvm/Transforms/Scalar/ScalarizeMaskedMemIntrin.h"
+#include "llvm/Transforms/Scalar/TLSVariableHoist.h"
+#include "llvm/Transforms/Utils/CanonicalizeFreezeInLoops.h"
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
+#include "llvm/Transforms/Utils/LowerGlobalDtors.h"
 #include "llvm/Transforms/Utils/LowerInvoke.h"
 #include <cassert>
 #include <type_traits>
@@ -94,7 +102,6 @@ namespace llvm {
                           MachineFunctionAnalysisManager &) {                  \
       llvm_unreachable("this api is to make new PM api happy");                \
     }                                                                          \
-    static MachinePassKey Key;                                                 \
   };
 #define DUMMY_MACHINE_FUNCTION_PASS(NAME, PASS_NAME, CONSTRUCTOR)              \
   struct PASS_NAME : public MachinePassInfoMixin<PASS_NAME> {                  \
@@ -103,7 +110,6 @@ namespace llvm {
                           MachineFunctionAnalysisManager &) {                  \
       return PreservedAnalyses::all();                                         \
     }                                                                          \
-    static MachinePassKey Key;                                                 \
   };
 #define DUMMY_MACHINE_FUNCTION_ANALYSIS(NAME, PASS_NAME, CONSTRUCTOR)          \
   struct PASS_NAME : public AnalysisInfoMixin<PASS_NAME> {                     \
@@ -124,11 +130,23 @@ namespace llvm {
 /// construction. The \c MachinePassRegistry.def file specifies how to construct
 /// all of the built-in passes, and those may reference these members during
 /// construction.
+///
+/// Target should provide following methods:
+/// Parse single target-specific MIR pass
+/// @param Name the pass name
+/// @return true if failed
+/// addPreISel - This method should add any "last minute" LLVM->LLVM
+/// passes (which are run just before instruction selector).
+/// void addPreISel(AddIRPass &) const;
+///
+/// void addAsmPrinter(AddMachinePass &, CreateMCStreamer) const;
+
 template <typename DerivedT> class CodeGenPassBuilder {
 public:
-  explicit CodeGenPassBuilder(LLVMTargetMachine &TM, CGPassBuilderOption Opts,
+  explicit CodeGenPassBuilder(LLVMTargetMachine &TM, PassBuilder &PB,
+                              CGPassBuilderOption Opts,
                               PassInstrumentationCallbacks *PIC)
-      : TM(TM), Opt(Opts), PIC(PIC) {
+      : TM(TM), PB(PB), Opt(Opts), PIC(PIC) {
     // Target could set CGPassBuilderOption::MISchedPostRA to true to achieve
     //     substitutePass(&PostRASchedulerID, &PostMachineSchedulerID)
 
@@ -148,24 +166,12 @@ template <typename DerivedT> class CodeGenPassBuilder {
                       raw_pwrite_stream &Out, raw_pwrite_stream *DwoOut,
                       CodeGenFileType FileType) const;
 
-  void registerModuleAnalyses(ModuleAnalysisManager &) const;
-  void registerFunctionAnalyses(FunctionAnalysisManager &) const;
-  void registerMachineFunctionAnalyses(MachineFunctionAnalysisManager &) const;
-  std::pair<StringRef, bool> getPassNameFromLegacyName(StringRef) const;
-
-  void registerAnalyses(MachineFunctionAnalysisManager &MFAM) const {
-    registerModuleAnalyses(*MFAM.MAM);
-    registerFunctionAnalyses(*MFAM.FAM);
-    registerMachineFunctionAnalyses(MFAM);
-  }
-
   PassInstrumentationCallbacks *getPassInstrumentationCallbacks() const {
-    return PIC;
+    static PassInstrumentationCallbacks PseudoPIC;
+    return PIC ? PIC : &PseudoPIC;
   }
 
 protected:
-  template <typename PassT> using has_key_t = decltype(PassT::Key);
-
   template <typename PassT>
   using is_module_pass_t = decltype(std::declval<PassT &>().run(
       std::declval<Module &>(), std::declval<ModuleAnalysisManager &>()));
@@ -225,10 +231,6 @@ template <typename DerivedT> class CodeGenPassBuilder {
         : PM(PM), PB(PB) {}
 
     template <typename PassT> void operator()(PassT &&Pass) {
-      static_assert(
-          is_detected<has_key_t, PassT>::value,
-          "Machine function pass must define a static member variable `Key`.");
-
       if (!PB.runBeforeAdding(PassT::name()))
         return;
 
@@ -238,14 +240,16 @@ template <typename DerivedT> class CodeGenPassBuilder {
         C(PassT::name());
     }
 
-    template <typename PassT> void insertPass(MachinePassKey *ID, PassT Pass) {
+    template <typename PassT> void insertPass(StringRef Name, PassT Pass) {
       PB.AfterCallbacks.emplace_back(
-          [this, ID, Pass = std::move(Pass)](MachinePassKey *PassID) {
-            if (PassID == ID)
+          [this, Name, Pass = std::move(Pass)](StringRef PassName) {
+            if (Name == PassName)
               this->PM.addPass(std::move(Pass));
           });
     }
 
+    MachineFunctionPassManager &getPM() { return PM; }
+
     MachineFunctionPassManager releasePM() { return std::move(PM); }
 
   private:
@@ -253,18 +257,35 @@ template <typename DerivedT> class CodeGenPassBuilder {
     const DerivedT &PB;
   };
 
+  // Find the FSProfile file name. The internal option takes the precedence
+  // before getting from TargetMachine.
+  // TODO: Use PGOOptions only.
+  std::string getFSProfileFile() const {
+    if (!Opt.FSProfileFile.empty())
+      return Opt.FSProfileFile;
+    const std::optional<PGOOptions> &PGOOpt = TM.getPGOOption();
+    if (PGOOpt == std::nullopt || PGOOpt->Action != PGOOptions::SampleUse)
+      return std::string();
+    return PGOOpt->ProfileFile;
+  }
+
+  // Find the Profile remapping file name. The internal option takes the
+  // precedence before getting from TargetMachine.
+  // TODO: Use PGOOptions only.
+  std::string getFSRemappingFile() const {
+    if (!Opt.FSRemappingFile.empty())
+      return Opt.FSRemappingFile;
+    const std::optional<PGOOptions> &PGOOpt = TM.getPGOOption();
+    if (PGOOpt == std::nullopt || PGOOpt->Action != PGOOptions::SampleUse)
+      return std::string();
+    return PGOOpt->ProfileRemappingFile;
+  }
+
   LLVMTargetMachine &TM;
+  PassBuilder &PB;
   CGPassBuilderOption Opt;
   PassInstrumentationCallbacks *PIC;
 
-  /// Target override these hooks to parse target-specific analyses.
-  void registerTargetAnalysis(ModuleAnalysisManager &) const {}
-  void registerTargetAnalysis(FunctionAnalysisManager &) const {}
-  void registerTargetAnalysis(MachineFunctionAnalysisManager &) const {}
-  std::pair<StringRef, bool> getTargetPassNameFromLegacyName(StringRef) const {
-    return {"", false};
-  }
-
   template <typename TMC> TMC &getTM() const { return static_cast<TMC &>(TM); }
   CodeGenOptLevel getOptLevel() const { return TM.getOptLevel(); }
 
@@ -334,12 +355,14 @@ template <typename DerivedT> class CodeGenPassBuilder {
   /// immediately before machine code is emitted.
   void addPreEmitPass(AddMachinePass &) const {}
 
+  /// This pass may be implemented by targets that want to run passes
+  /// immediately after basic block sections are assigned.
+  void addPostBBSections(AddMachinePass &) const {}
+
   /// Targets may add passes immediately before machine code is emitted in this
   /// callback. This is called even later than `addPreEmitPass`.
-  // FIXME: Rename `addPreEmitPass` to something more sensible given its actual
-  // position and remove the `2` suffix here as this callback is what
-  // `addPreEmitPass` *should* be but in reality isn't.
-  void addPreEmitPass2(AddMachinePass &) const {}
+  /// This function replaces `addPreEmitPass2` in TargetConfig.
+  void addPrecedingEmitPass(AddMachinePass &) const {}
 
   /// {{@ For GlobalISel
   ///
@@ -399,7 +422,7 @@ template <typename DerivedT> class CodeGenPassBuilder {
   /// representation to the MI representation.
   /// Adds IR based lowering and target specific optimization passes and finally
   /// the core instruction selection passes.
-  void addISelPasses(AddIRPass &) const;
+  Error addISelPasses(AddIRPass &, AddMachinePass &) const;
 
   /// Add the actual instruction selection passes. This does not include
   /// preparation passes on IR.
@@ -439,21 +462,58 @@ template <typename DerivedT> class CodeGenPassBuilder {
   /// are required for fast register allocation.
   Error addFastRegAlloc(AddMachinePass &) const;
 
+  /// addPostFastRegAllocRewrite - Add passes to the optimized register
+  /// allocation pipeline after fast register allocation is complete.
+  Error addPostFastRegAllocRewrite(AddMachinePass &) const {
+    return make_error<StringError>(
+        "addPostFastRegAllocRewrite is not overridden",
+        inconvertibleErrorCode());
+  }
+
   /// addOptimizedRegAlloc - Add passes related to register allocation.
   /// LLVMTargetMachine provides standard regalloc passes for most targets.
-  void addOptimizedRegAlloc(AddMachinePass &) const;
+  Error addOptimizedRegAlloc(AddMachinePass &) const;
 
   /// Add passes that optimize machine instructions after register allocation.
   void addMachineLateOptimization(AddMachinePass &) const;
 
-  /// addGCPasses - Add late codegen passes that analyze code for garbage
+  /// registerGCPasses - Add late codegen passes that analyze code for garbage
   /// collection. This should return true if GC info should be printed after
   /// these passes.
-  void addGCPasses(AddMachinePass &) const {}
+  bool registerGCPasses(MachineFunctionAnalysisManager &MFAM) const {
+    MFAM.registerPass([] { return GCMachineCodeAnalysisPass(); });
+    return true;
+  }
 
   /// Add standard basic block placement passes.
   void addBlockPlacement(AddMachinePass &) const;
 
+  /// Add a pass to print the machine function if printing is enabled.
+  void addPrintPass(AddMachinePass &addPass, const std::string &Banner) const {
+    if (Opt.PrintAfterISel)
+      addPass(MachineFunctionPrinterPass(dbgs(), Banner));
+  }
+
+  /// Add a pass to perform basic verification of the machine function if
+  /// verification is enabled.
+  void addVerifyPass(AddMachinePass &addPass, const std::string &Banner) const {
+    bool Verify = Opt.VerifyMachineCode.value_or(false);
+#ifdef EXPENSIVE_CHECKS
+    if (!Opt.VerifyMachineCode)
+      Verify = TM->isMachineVerifierClean();
+#endif
+    if (Verify)
+      addPass(MachineVerifierPass(Banner));
+  }
+
+  /// printAndVerify - Add a pass to dump then verify the machine function, if
+  /// those steps are enabled.
+  void printAndVerify(AddMachinePass &addPass,
+                      const std::string &Banner) const {
+    addPrintPass(addPass, Banner);
+    addVerifyPass(addPass, Banner);
+  }
+
   using CreateMCStreamer =
       std::function<Expected<std::unique_ptr<MCStreamer>>(MCContext &)>;
   void addAsmPrinter(AddMachinePass &, CreateMCStreamer) const {
@@ -463,18 +523,10 @@ template <typename DerivedT> class CodeGenPassBuilder {
   /// Utilities for targets to add passes to the pass manager.
   ///
 
-  /// createTargetRegisterAllocator - Create the register allocator pass for
-  /// this target at the current optimization level.
-  void addTargetRegisterAllocator(AddMachinePass &, bool Optimized) const;
-
-  /// addMachinePasses helper to create the target-selected or overriden
-  /// regalloc pass.
-  void addRegAllocPass(AddMachinePass &, bool Optimized) const;
-
-  /// Add core register alloator passes which do the actual register assignment
-  /// and rewriting. \returns true if any passes were added.
-  Error addRegAssignmentFast(AddMachinePass &) const;
-  Error addRegAssignmentOptimized(AddMachinePass &) const;
+  /// Add core register allocator passes which do the actual register assignment
+  /// and rewriting. \returns Error::success() if any passes were added.
+  Error addRegAssignAndRewriteFast(AddMachinePass &addPass) const;
+  Error addRegAssignAndRewriteOptimized(AddMachinePass &addPass) const;
 
 private:
   DerivedT &derived() { return static_cast<DerivedT &>(*this); }
@@ -515,14 +567,12 @@ Error CodeGenPassBuilder<Derived>::buildPipeline(
   // `ProfileSummaryInfo` is always valid.
   addIRPass(RequireAnalysisPass<ProfileSummaryAnalysis, Module>());
   addIRPass(RequireAnalysisPass<CollectorMetadataAnalysis, Module>());
-  addISelPasses(addIRPass);
-
   AddMachinePass addPass(MFPM, derived());
-  if (auto Err = addCoreISelPasses(addPass))
-    return std::move(Err);
+  if (auto Err = addISelPasses(addIRPass, addPass))
+    return Err;
 
   if (auto Err = derived().addMachinePasses(addPass))
-    return std::move(Err);
+    return Err;
 
   derived().addAsmPrinter(
       addPass, [this, &Out, DwoOut, FileType](MCContext &Ctx) {
@@ -613,93 +663,24 @@ static inline AAManager registerAAAnalyses() {
 }
 
 template <typename Derived>
-void CodeGenPassBuilder<Derived>::registerModuleAnalyses(
-    ModuleAnalysisManager &MAM) const {
-#define MODULE_ANALYSIS(NAME, PASS_NAME, CONSTRUCTOR)                          \
-  MAM.registerPass([&] { return PASS_NAME CONSTRUCTOR; });
-#include "MachinePassRegistry.def"
-  derived().registerTargetAnalysis(MAM);
-}
-
-template <typename Derived>
-void CodeGenPassBuilder<Derived>::registerFunctionAnalyses(
-    FunctionAnalysisManager &FAM) const {
-  FAM.registerPass([this] { return registerAAAnalyses(); });
-
-#define FUNCTION_ANALYSIS(NAME, PASS_NAME, CONSTRUCTOR)                        \
-  FAM.registerPass([&] { return PASS_NAME CONSTRUCTOR; });
-#include "MachinePassRegistry.def"
-  derived().registerTargetAnalysis(FAM);
-}
-
-template <typename Derived>
-void CodeGenPassBuilder<Derived>::registerMachineFunctionAnalyses(
-    MachineFunctionAnalysisManager &MFAM) const {
-#define MACHINE_FUNCTION_ANALYSIS(NAME, PASS_NAME, CONSTRUCTOR)                \
-  MFAM.registerPass([&] { return PASS_NAME CONSTRUCTOR; });
-#include "MachinePassRegistry.def"
-  derived().registerTargetAnalysis(MFAM);
-}
-
-// FIXME: For new PM, use pass name directly in commandline seems good.
-// Translate stringfied pass name to its old commandline name. Returns the
-// matching legacy name and a boolean value indicating if the pass is a machine
-// pass.
-template <typename Derived>
-std::pair<StringRef, bool>
-CodeGenPassBuilder<Derived>::getPassNameFromLegacyName(StringRef Name) const {
-  std::pair<StringRef, bool> Ret;
-  if (Name.empty())
-    return Ret;
-
-#define FUNCTION_PASS(NAME, PASS_NAME, CONSTRUCTOR)                            \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, false};
-#define DUMMY_FUNCTION_PASS(NAME, PASS_NAME, CONSTRUCTOR)                      \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, false};
-#define MODULE_PASS(NAME, PASS_NAME, CONSTRUCTOR)                              \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, false};
-#define DUMMY_MODULE_PASS(NAME, PASS_NAME, CONSTRUCTOR)                        \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, false};
-#define MACHINE_MODULE_PASS(NAME, PASS_NAME, CONSTRUCTOR)                      \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, true};
-#define DUMMY_MACHINE_MODULE_PASS(NAME, PASS_NAME, CONSTRUCTOR)                \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, true};
-#define MACHINE_FUNCTION_PASS(NAME, PASS_NAME, CONSTRUCTOR)                    \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, true};
-#define DUMMY_MACHINE_FUNCTION_PASS(NAME, PASS_NAME, CONSTRUCTOR)              \
-  if (Name == NAME)                                                            \
-    Ret = {#PASS_NAME, true};
-#include "llvm/Passes/MachinePassRegistry.def"
-
-  if (Ret.first.empty())
-    Ret = derived().getTargetPassNameFromLegacyName(Name);
-
-  if (Ret.first.empty())
-    report_fatal_error(Twine('\"') + Twine(Name) +
-                       Twine("\" pass could not be found."));
-
-  return Ret;
-}
-
-template <typename Derived>
-void CodeGenPassBuilder<Derived>::addISelPasses(AddIRPass &addPass) const {
+Error CodeGenPassBuilder<Derived>::addISelPasses(
+    AddIRPass &addPass, AddMachinePass &addMachinePass) const {
   derived().addGlobalMergePass(addPass);
   if (TM.useEmulatedTLS())
     addPass(LowerEmuTLSPass());
 
   addPass(PreISelIntrinsicLoweringPass(TM));
+  addPass(ExpandLargeDivRemPass(&TM));
+  addPass(ExpandLargeFpConvertPass(&TM));
 
   derived().addIRPasses(addPass);
   derived().addCodeGenPrepare(addPass);
   addPassesToHandleExceptions(addPass);
   derived().addISelPrepare(addPass);
+
+  if (auto Err = addCoreISelPasses(addMachinePass))
+    return Err;
+  return Error::success();
 }
 
 /// Add common target configurable passes that perform LLVM IR to IR transforms
@@ -711,16 +692,1...
[truncated]

[paperchalice]

If we support extension points here, the location of extension points is a problem, seems all targets augment the default implementation or rewrite some parts of the pipeline.

[aeubanks]

general feedback: it seems like these sorts of PRs could be split for easier reviewing

don't have to do it here, but something to keep in mind

[aeubanks]

are there targets that add their own regallocs? I'm not seeing any

[paperchalice]

AMDGPU and RISCV, all regalloc passes inherit RegisterRegAllocBase.

[aeubanks]

if no passes were added, this returns an error? that seems weird

[paperchalice]

This PR is not so important now, the legacy codegen pipeline may change in the foreseeable future (like debug info related pass). This part can be postponed.

[lelouch-17]

Hi @paperchalice !

Do you plan on adding certain api's like insertPass and disablePass in a seperate PR ? I am building a basic backend pipeline for AMDGPU therefore I am in need of these.

[paperchalice]

Hi @paperchalice !

Do you plan on adding certain api's like insertPass and disablePass in a seperate PR ? I am building a basic backend pipeline for AMDGPU therefore I am in need of these.

Hi, thanks for your interest in this! Currently the migration work is still in early stage, building pipeline for a specific backend might too early. If you really need it, I will open another pull request to support it.

[lelouch-17]

Hi, thanks for your interest in this! Currently the migration work is still in early stage, building pipeline for a specific backend might too early.

We are creating a dummy pipeline of sort for AMDGPU right now.

If you really need it, I will open another pull request to support it.

It would be very helpful if you can do that.