Skip to content

[NewPM][CodeGen] Port regallocfast to new pass manager #94426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

paperchalice
Copy link
Contributor

@paperchalice paperchalice commented Jun 5, 2024

This pull request port regallocfast to new pass manager. It exposes the parameter filter to handle different register classes for AMDGPU. IIUC AMDGPU need to allocate different register classes separately so it need implement its own --<reg-class>-regalloc. Now users can use e.g. -passe=regallocfast<filter=sgpr> to allocate specific register class.
The command line option --regalloc-npm is still in work progress, plan to reuse the syntax of passes, e.g. use --regalloc-npm=regallocfast<filter=sgpr>,greedy<filter=vgpr> to replace --sgpr-regalloc and --vgpr-regalloc.

@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2024

@llvm/pr-subscribers-backend-systemz
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-backend-amdgpu

Author: None (paperchalice)

Changes

This pull request port regallocfast to new pass manager. It exposes the parameter filter to handle different register classes for AMDGPU. IIUC AMDGPU need to allocate different register classes separately so it need implement its own --&lt;reg-class&gt;-regalloc.
The command line option --regalloc-npm is still in work progress.


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

35 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachinePassManager.h (+3-3)
  • (added) llvm/include/llvm/CodeGen/RegAllocFast.h (+57)
  • (modified) llvm/include/llvm/Passes/CodeGenPassBuilder.h (+2-1)
  • (modified) llvm/include/llvm/Passes/MachinePassRegistry.def (+13-1)
  • (modified) llvm/include/llvm/Passes/PassBuilder.h (+16)
  • (modified) llvm/lib/CodeGen/RegAllocFast.cpp (+112-77)
  • (modified) llvm/lib/Passes/PassBuilder.cpp (+57)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (+8)
  • (modified) llvm/test/CodeGen/AArch64/fast-regalloc-empty-bb-with-liveins.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/fast-ra-kills-vcc.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/fast-regalloc-bundles.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/fastregalloc-illegal-subreg-physreg.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/fastregalloc-self-loop-heuristic.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/spill-agpr.mir (+2)
  • (modified) llvm/test/CodeGen/AMDGPU/spill192.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/spill224.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/spill288.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/spill320.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/spill352.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/spill384.mir (+1)
  • (modified) llvm/test/CodeGen/AMDGPU/unexpected-reg-unit-state.mir (+1)
  • (modified) llvm/test/CodeGen/ARM/regalloc-fast-rewrite-implicits.mir (+1)
  • (modified) llvm/test/CodeGen/MIR/Generic/runPass.mir (+1)
  • (modified) llvm/test/CodeGen/PowerPC/spill-nor0.mir (+1)
  • (modified) llvm/test/CodeGen/SystemZ/regalloc-fast-invalid-kill-flag.mir (+1)
  • (modified) llvm/test/CodeGen/Thumb/high-reg-clobber.mir (+1)
  • (modified) llvm/test/CodeGen/Thumb2/high-reg-spill.mir (+1)
  • (modified) llvm/test/CodeGen/X86/bug47278-eflags-error.mir (+1)
  • (modified) llvm/test/CodeGen/X86/bug47278.mir (+1)
  • (modified) llvm/test/CodeGen/X86/callbr-asm-outputs-regallocfast.mir (+1)
  • (modified) llvm/test/CodeGen/X86/fastregalloc-selfloop.mir (+1)
  • (modified) llvm/test/CodeGen/X86/fastregalloc-tied-undef.mir (+1)
  • (modified) llvm/test/CodeGen/X86/regalloc-fast-missing-live-out-spill.mir (+1)
  • (modified) llvm/test/CodeGen/X86/statepoint-fastregalloc.mir (+1)
  • (modified) llvm/test/CodeGen/X86/virtreg-physreg-def-regallocfast.mir (+1)
diff --git a/llvm/include/llvm/CodeGen/MachinePassManager.h b/llvm/include/llvm/CodeGen/MachinePassManager.h
index 852b1a0f41613..7d15664fbe754 100644
--- a/llvm/include/llvm/CodeGen/MachinePassManager.h
+++ b/llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -61,7 +61,7 @@ struct MachinePassModel
 #ifndef NDEBUG
     if constexpr (is_detected<has_get_required_properties_t, PassT>::value) {
       auto &MFProps = IR.getProperties();
-      auto RequiredProperties = PassT::getRequiredProperties();
+      auto RequiredProperties = this->Pass.getRequiredProperties();
       if (!MFProps.verifyRequiredProperties(RequiredProperties)) {
         errs() << "MachineFunctionProperties required by " << PassT::name()
                << " pass are not met by function " << IR.getName() << ".\n"
@@ -78,9 +78,9 @@ struct MachinePassModel
     auto PA = this->Pass.run(IR, AM);
 
     if constexpr (is_detected<has_get_set_properties_t, PassT>::value)
-      IR.getProperties().set(PassT::getSetProperties());
+      IR.getProperties().set(this->Pass.getSetProperties());
     if constexpr (is_detected<has_get_cleared_properties_t, PassT>::value)
-      IR.getProperties().reset(PassT::getClearedProperties());
+      IR.getProperties().reset(this->Pass.getClearedProperties());
     return PA;
   }
 
diff --git a/llvm/include/llvm/CodeGen/RegAllocFast.h b/llvm/include/llvm/CodeGen/RegAllocFast.h
new file mode 100644
index 0000000000000..c50deccabd995
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/RegAllocFast.h
@@ -0,0 +1,57 @@
+//==- RegAllocFast.h ----------- fast register allocator  ----------*-C++-*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_REGALLOCFAST_H
+#define LLVM_CODEGEN_REGALLOCFAST_H
+
+#include "llvm/CodeGen/MachinePassManager.h"
+#include "llvm/CodeGen/RegAllocCommon.h"
+
+namespace llvm {
+
+struct RegAllocFastPassOptions {
+  RegClassFilterFunc Filter = allocateAllRegClasses;
+  StringRef FilterName = "all";
+  bool ClearVRegs = true;
+};
+
+class RegAllocFastPass : public PassInfoMixin<RegAllocFastPass> {
+  RegAllocFastPassOptions Opts;
+
+public:
+  RegAllocFastPass(RegAllocFastPassOptions Opts = RegAllocFastPassOptions())
+      : Opts(Opts) {}
+
+  MachineFunctionProperties getRequiredProperties() {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::NoPHIs);
+  }
+
+  MachineFunctionProperties getSetProperties() {
+    if (Opts.ClearVRegs) {
+      return MachineFunctionProperties().set(
+          MachineFunctionProperties::Property::NoVRegs);
+    }
+
+    return MachineFunctionProperties();
+  }
+
+  MachineFunctionProperties getClearedProperties() {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::IsSSA);
+  }
+
+  PreservedAnalyses run(MachineFunction &MF, MachineFunctionAnalysisManager &);
+
+  void printPipeline(raw_ostream &OS,
+                     function_ref<StringRef(StringRef)> MapClassName2PassName);
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_REGALLOCFAST_H
diff --git a/llvm/include/llvm/Passes/CodeGenPassBuilder.h b/llvm/include/llvm/Passes/CodeGenPassBuilder.h
index 3c4723a0513ce..c8263575e3045 100644
--- a/llvm/include/llvm/Passes/CodeGenPassBuilder.h
+++ b/llvm/include/llvm/Passes/CodeGenPassBuilder.h
@@ -42,6 +42,7 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/MachinePassManager.h"
 #include "llvm/CodeGen/PreISelIntrinsicLowering.h"
+#include "llvm/CodeGen/RegAllocFast.h"
 #include "llvm/CodeGen/ReplaceWithVeclib.h"
 #include "llvm/CodeGen/SafeStack.h"
 #include "llvm/CodeGen/SelectOptimize.h"
@@ -1037,7 +1038,7 @@ void CodeGenPassBuilder<Derived, TargetMachineT>::addTargetRegisterAllocator(
   if (Optimized)
     addPass(RAGreedyPass());
   else
-    addPass(RAFastPass());
+    addPass(RegAllocFastPass());
 }
 
 /// Find and instantiate the register allocation pass requested by this target
diff --git a/llvm/include/llvm/Passes/MachinePassRegistry.def b/llvm/include/llvm/Passes/MachinePassRegistry.def
index fc2beb7286455..5a2a50d5b9946 100644
--- a/llvm/include/llvm/Passes/MachinePassRegistry.def
+++ b/llvm/include/llvm/Passes/MachinePassRegistry.def
@@ -132,6 +132,19 @@ MACHINE_FUNCTION_PASS("require-all-machine-function-properties",
 MACHINE_FUNCTION_PASS("trigger-verifier-error", TriggerVerifierErrorPass())
 #undef MACHINE_FUNCTION_PASS
 
+#ifndef MACHINE_FUNCTION_PASS_WITH_PARAMS
+#define MACHINE_FUNCTION_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER,    \
+                                          PARAMS)
+#endif
+MACHINE_FUNCTION_PASS_WITH_PARAMS(
+    "regallocfast", "RegAllocFast",
+    [](RegAllocFastPassOptions Opts) { return RegAllocFastPass(Opts); },
+    [PB = this](StringRef Params) {
+      return parseRegAllocFastPassOptions(*PB, Params);
+    },
+    "filter=all|reg-class;clear-vregs;no-clear-vregs")
+#undef MACHINE_FUNCTION_PASS_WITH_PARAMS
+
 // After a pass is converted to new pass manager, its entry should be moved from
 // dummy table to the normal one. For example, for a machine function pass,
 // DUMMY_MACHINE_FUNCTION_PASS to MACHINE_FUNCTION_PASS.
@@ -211,7 +224,6 @@ DUMMY_MACHINE_FUNCTION_PASS("processimpdefs", ProcessImplicitDefsPass)
 DUMMY_MACHINE_FUNCTION_PASS("prologepilog", PrologEpilogInserterPass)
 DUMMY_MACHINE_FUNCTION_PASS("prologepilog-code", PrologEpilogCodeInserterPass)
 DUMMY_MACHINE_FUNCTION_PASS("ra-basic", RABasicPass)
-DUMMY_MACHINE_FUNCTION_PASS("ra-fast", RAFastPass)
 DUMMY_MACHINE_FUNCTION_PASS("ra-greedy", RAGreedyPass)
 DUMMY_MACHINE_FUNCTION_PASS("ra-pbqp", RAPBQPPass)
 DUMMY_MACHINE_FUNCTION_PASS("reg-usage-collector", RegUsageInfoCollectorPass)
diff --git a/llvm/include/llvm/Passes/PassBuilder.h b/llvm/include/llvm/Passes/PassBuilder.h
index 8de0f024a4b11..8267f45eb5de1 100644
--- a/llvm/include/llvm/Passes/PassBuilder.h
+++ b/llvm/include/llvm/Passes/PassBuilder.h
@@ -17,6 +17,7 @@
 
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/CodeGen/MachinePassManager.h"
+#include "llvm/CodeGen/RegAllocCommon.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Support/Error.h"
@@ -388,6 +389,10 @@ class PassBuilder {
   /// returns false.
   Error parseAAPipeline(AAManager &AA, StringRef PipelineText);
 
+  /// Parse FilterName to get RegClassFilterFunc for given FilterName
+  /// when register allocator need a RegClassFilterFunc.
+  RegClassFilterFunc parseRegAllocFilter(StringRef FilterName);
+
   /// Print pass names.
   void printPassNames(raw_ostream &OS);
 
@@ -576,6 +581,14 @@ class PassBuilder {
   }
   /// @}}
 
+  /// Register callbacks to parse target specific filter field if regalloc pass
+  /// needs it. E.g. AMDGPU requires regalloc passes can handle sgpr and vgpr
+  /// separately.
+  void registerRegClassFilterParsingCallback(
+      const std::function<RegClassFilterFunc(StringRef)> &C) {
+    RegClassFilterParsingCallbacks.push_back(C);
+  }
+
   /// Register a callback for a top-level pipeline entry.
   ///
   /// If the PassManager type is not given at the top level of the pipeline
@@ -792,6 +805,9 @@ class PassBuilder {
                                  ArrayRef<PipelineElement>)>,
               2>
       MachineFunctionPipelineParsingCallbacks;
+  // Callbacks to parse `filter` parameter in register allocation passes
+  SmallVector<std::function<RegClassFilterFunc(StringRef)>, 2>
+      RegClassFilterParsingCallbacks;
 };
 
 /// This utility template takes care of adding require<> and invalidate<>
diff --git a/llvm/lib/CodeGen/RegAllocFast.cpp b/llvm/lib/CodeGen/RegAllocFast.cpp
index 6740e1f0edb4f..8394634c6c564 100644
--- a/llvm/lib/CodeGen/RegAllocFast.cpp
+++ b/llvm/lib/CodeGen/RegAllocFast.cpp
@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/CodeGen/RegAllocFast.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IndexedMap.h"
@@ -174,14 +175,12 @@ class InstrPosIndexes {
   DenseMap<const MachineInstr *, uint64_t> Instr2PosIndex;
 };
 
-class RegAllocFast : public MachineFunctionPass {
+class RegAllocFastImpl {
 public:
-  static char ID;
-
-  RegAllocFast(const RegClassFilterFunc F = allocateAllRegClasses,
-               bool ClearVirtRegs_ = true)
-      : MachineFunctionPass(ID), ShouldAllocateClass(F),
-        StackSlotForVirtReg(-1), ClearVirtRegs(ClearVirtRegs_) {}
+  RegAllocFastImpl(const RegClassFilterFunc F = allocateAllRegClasses,
+                   bool ClearVirtRegs_ = true)
+      : ShouldAllocateClass(F), StackSlotForVirtReg(-1),
+        ClearVirtRegs(ClearVirtRegs_) {}
 
 private:
   MachineFrameInfo *MFI = nullptr;
@@ -197,8 +196,6 @@ class RegAllocFast : public MachineFunctionPass {
   /// Maps virtual regs to the frame index where these values are spilled.
   IndexedMap<int, VirtReg2IndexFunctor> StackSlotForVirtReg;
 
-  bool ClearVirtRegs;
-
   /// Everything we know about a live virtual register.
   struct LiveReg {
     MachineInstr *LastUse = nullptr; ///< Last instr to use reg.
@@ -318,35 +315,11 @@ class RegAllocFast : public MachineFunctionPass {
   };
 
 public:
-  StringRef getPassName() const override { return "Fast Register Allocator"; }
-
-  void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesCFG();
-    MachineFunctionPass::getAnalysisUsage(AU);
-  }
-
-  MachineFunctionProperties getRequiredProperties() const override {
-    return MachineFunctionProperties().set(
-        MachineFunctionProperties::Property::NoPHIs);
-  }
-
-  MachineFunctionProperties getSetProperties() const override {
-    if (ClearVirtRegs) {
-      return MachineFunctionProperties().set(
-          MachineFunctionProperties::Property::NoVRegs);
-    }
-
-    return MachineFunctionProperties();
-  }
+  bool ClearVirtRegs;
 
-  MachineFunctionProperties getClearedProperties() const override {
-    return MachineFunctionProperties().set(
-        MachineFunctionProperties::Property::IsSSA);
-  }
+  bool runOnMachineFunction(MachineFunction &MF);
 
 private:
-  bool runOnMachineFunction(MachineFunction &MF) override;
-
   void allocateBasicBlock(MachineBasicBlock &MBB);
 
   void addRegClassDefCounts(std::vector<unsigned> &RegClassDefCounts,
@@ -408,6 +381,47 @@ class RegAllocFast : public MachineFunctionPass {
   void dumpState() const;
 };
 
+class RegAllocFast : public MachineFunctionPass {
+  RegAllocFastImpl Impl;
+
+public:
+  static char ID;
+
+  RegAllocFast(const RegClassFilterFunc F = allocateAllRegClasses,
+               bool ClearVirtRegs_ = true)
+      : MachineFunctionPass(ID), Impl(F, ClearVirtRegs_) {}
+
+  bool runOnMachineFunction(MachineFunction &MF) override {
+    return Impl.runOnMachineFunction(MF);
+  }
+
+  StringRef getPassName() const override { return "Fast Register Allocator"; }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesCFG();
+    MachineFunctionPass::getAnalysisUsage(AU);
+  }
+
+  MachineFunctionProperties getRequiredProperties() const override {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::NoPHIs);
+  }
+
+  MachineFunctionProperties getSetProperties() const override {
+    if (Impl.ClearVirtRegs) {
+      return MachineFunctionProperties().set(
+          MachineFunctionProperties::Property::NoVRegs);
+    }
+
+    return MachineFunctionProperties();
+  }
+
+  MachineFunctionProperties getClearedProperties() const override {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::IsSSA);
+  }
+};
+
 } // end anonymous namespace
 
 char RegAllocFast::ID = 0;
@@ -415,18 +429,18 @@ char RegAllocFast::ID = 0;
 INITIALIZE_PASS(RegAllocFast, "regallocfast", "Fast Register Allocator", false,
                 false)
 
-bool RegAllocFast::shouldAllocateRegister(const Register Reg) const {
+bool RegAllocFastImpl::shouldAllocateRegister(const Register Reg) const {
   assert(Reg.isVirtual());
   const TargetRegisterClass &RC = *MRI->getRegClass(Reg);
   return ShouldAllocateClass(*TRI, RC);
 }
 
-void RegAllocFast::setPhysRegState(MCPhysReg PhysReg, unsigned NewState) {
+void RegAllocFastImpl::setPhysRegState(MCPhysReg PhysReg, unsigned NewState) {
   for (MCRegUnit Unit : TRI->regunits(PhysReg))
     RegUnitStates[Unit] = NewState;
 }
 
-bool RegAllocFast::isPhysRegFree(MCPhysReg PhysReg) const {
+bool RegAllocFastImpl::isPhysRegFree(MCPhysReg PhysReg) const {
   for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
     if (RegUnitStates[Unit] != regFree)
       return false;
@@ -436,7 +450,7 @@ bool RegAllocFast::isPhysRegFree(MCPhysReg PhysReg) const {
 
 /// This allocates space for the specified virtual register to be held on the
 /// stack.
-int RegAllocFast::getStackSpaceFor(Register VirtReg) {
+int RegAllocFastImpl::getStackSpaceFor(Register VirtReg) {
   // Find the location Reg would belong...
   int SS = StackSlotForVirtReg[VirtReg];
   // Already has space allocated?
@@ -464,7 +478,7 @@ static bool dominates(InstrPosIndexes &PosIndexes, const MachineInstr &A,
 }
 
 /// Returns false if \p VirtReg is known to not live out of the current block.
-bool RegAllocFast::mayLiveOut(Register VirtReg) {
+bool RegAllocFastImpl::mayLiveOut(Register VirtReg) {
   if (MayLiveAcrossBlocks.test(Register::virtReg2Index(VirtReg))) {
     // Cannot be live-out if there are no successors.
     return !MBB->succ_empty();
@@ -517,7 +531,7 @@ bool RegAllocFast::mayLiveOut(Register VirtReg) {
 }
 
 /// Returns false if \p VirtReg is known to not be live into the current block.
-bool RegAllocFast::mayLiveIn(Register VirtReg) {
+bool RegAllocFastImpl::mayLiveIn(Register VirtReg) {
   if (MayLiveAcrossBlocks.test(Register::virtReg2Index(VirtReg)))
     return !MBB->pred_empty();
 
@@ -536,8 +550,9 @@ bool RegAllocFast::mayLiveIn(Register VirtReg) {
 
 /// Insert spill instruction for \p AssignedReg before \p Before. Update
 /// DBG_VALUEs with \p VirtReg operands with the stack slot.
-void RegAllocFast::spill(MachineBasicBlock::iterator Before, Register VirtReg,
-                         MCPhysReg AssignedReg, bool Kill, bool LiveOut) {
+void RegAllocFastImpl::spill(MachineBasicBlock::iterator Before,
+                             Register VirtReg, MCPhysReg AssignedReg, bool Kill,
+                             bool LiveOut) {
   LLVM_DEBUG(dbgs() << "Spilling " << printReg(VirtReg, TRI) << " in "
                     << printReg(AssignedReg, TRI));
   int FI = getStackSpaceFor(VirtReg);
@@ -596,8 +611,8 @@ void RegAllocFast::spill(MachineBasicBlock::iterator Before, Register VirtReg,
 }
 
 /// Insert reload instruction for \p PhysReg before \p Before.
-void RegAllocFast::reload(MachineBasicBlock::iterator Before, Register VirtReg,
-                          MCPhysReg PhysReg) {
+void RegAllocFastImpl::reload(MachineBasicBlock::iterator Before,
+                              Register VirtReg, MCPhysReg PhysReg) {
   LLVM_DEBUG(dbgs() << "Reloading " << printReg(VirtReg, TRI) << " into "
                     << printReg(PhysReg, TRI) << '\n');
   int FI = getStackSpaceFor(VirtReg);
@@ -610,7 +625,7 @@ void RegAllocFast::reload(MachineBasicBlock::iterator Before, Register VirtReg,
 /// This is not just MBB.begin() because surprisingly we have EH_LABEL
 /// instructions marking the begin of a basic block. This means we must insert
 /// new instructions after such labels...
-MachineBasicBlock::iterator RegAllocFast::getMBBBeginInsertionPoint(
+MachineBasicBlock::iterator RegAllocFastImpl::getMBBBeginInsertionPoint(
     MachineBasicBlock &MBB, SmallSet<Register, 2> &PrologLiveIns) const {
   MachineBasicBlock::iterator I = MBB.begin();
   while (I != MBB.end()) {
@@ -637,7 +652,7 @@ MachineBasicBlock::iterator RegAllocFast::getMBBBeginInsertionPoint(
 }
 
 /// Reload all currently assigned virtual registers.
-void RegAllocFast::reloadAtBegin(MachineBasicBlock &MBB) {
+void RegAllocFastImpl::reloadAtBegin(MachineBasicBlock &MBB) {
   if (LiveVirtRegs.empty())
     return;
 
@@ -680,7 +695,7 @@ void RegAllocFast::reloadAtBegin(MachineBasicBlock &MBB) {
 /// Handle the direct use of a physical register.  Check that the register is
 /// not used by a virtreg. Kill the physreg, marking it free. This may add
 /// implicit kills to MO->getParent() and invalidate MO.
-bool RegAllocFast::usePhysReg(MachineInstr &MI, MCPhysReg Reg) {
+bool RegAllocFastImpl::usePhysReg(MachineInstr &MI, MCPhysReg Reg) {
   assert(Register::isPhysicalRegister(Reg) && "expected physreg");
   bool displacedAny = displacePhysReg(MI, Reg);
   setPhysRegState(Reg, regPreAssigned);
@@ -688,7 +703,7 @@ bool RegAllocFast::usePhysReg(MachineInstr &MI, MCPhysReg Reg) {
   return displacedAny;
 }
 
-bool RegAllocFast::definePhysReg(MachineInstr &MI, MCPhysReg Reg) {
+bool RegAllocFastImpl::definePhysReg(MachineInstr &MI, MCPhysReg Reg) {
   bool displacedAny = displacePhysReg(MI, Reg);
   setPhysRegState(Reg, regPreAssigned);
   return displacedAny;
@@ -697,7 +712,7 @@ bool RegAllocFast::definePhysReg(MachineInstr &MI, MCPhysReg Reg) {
 /// Mark PhysReg as reserved or free after spilling any virtregs. This is very
 /// similar to defineVirtReg except the physreg is reserved instead of
 /// allocated.
-bool RegAllocFast::displacePhysReg(MachineInstr &MI, MCPhysReg PhysReg) {
+bool RegAllocFastImpl::displacePhysReg(MachineInstr &MI, MCPhysReg PhysReg) {
   bool displacedAny = false;
 
   for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
@@ -726,7 +741,7 @@ bool RegAllocFast::displacePhysReg(MachineInstr &MI, MCPhysReg PhysReg) {
   return displacedAny;
 }
 
-void RegAllocFast::freePhysReg(MCPhysReg PhysReg) {
+void RegAllocFastImpl::freePhysReg(MCPhysReg PhysReg) {
   LLVM_DEBUG(dbgs() << "Freeing " << printReg(PhysReg, TRI) << ':');
 
   MCRegister FirstUnit = *TRI->regunits(PhysReg).begin();
@@ -753,7 +768,7 @@ void RegAllocFast::freePhysReg(MCPhysReg PhysReg) {
 /// for allocation. Returns 0 when PhysReg is free or disabled with all aliases
 /// disabled - it can be allocated directly.
 /// \returns spillImpossible when PhysReg or an alias can't be spilled.
-unsigned RegAllocFast::calcSpillCost(MCPhysReg PhysReg) const {
+unsigned RegAllocFastImpl::calcSpillCost(MCPhysReg PhysReg) const {
   for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
     switch (unsigned VirtReg = RegUnitStates[Unit]) {
     case regFree:
@@ -772,8 +787,9 @@ unsigned RegAllocFast::calcSpillCost(MCPhysReg PhysReg) const {
   return 0;
 }
 
-void RegAllocFast::assignDanglingDebugValues(MachineInstr &Definition,
-                                             Register VirtReg, MCPhysReg Reg) {
+void RegAllocFastImpl::assignDanglingDebugValues(MachineInstr &Definition,
+                                                 Register VirtReg,
+                                                 MCPhysReg Reg) {
   auto UDBGValIter = DanglingDbgValues.find(VirtReg);
   if (UDBGValIter == DanglingDbgValues.end())
     return;
@@ -809,8 +825,8 @@ void RegAllocFast::assignDanglingDebugValues(MachineInstr &Definition,
 /// This method updates local state so that we know that PhysReg is the
 /// proper container for VirtReg now.  The physical register must not be used
 /// for anything else when this is called.
-void RegAllocFast::assignVirtToPhysReg(MachineInstr &AtMI, LiveReg &LR,
-                                       MCPhysReg PhysReg) {
+void RegAllocFastImpl::assignVirtToPhysReg(MachineInstr &AtMI, LiveReg &LR,
+                                           MCPhysReg PhysReg) {
   Register VirtReg = LR.VirtReg;
   LLVM_DEBUG(dbgs() << "Assigning " << printReg(VirtReg, TRI) << " to "
                     << printReg(PhysReg, TRI) << '\n');
@@ -824,7 +840,7 @@ void RegAllocFast::assignVirtToPhysReg(MachineInstr &AtMI, LiveReg &LR,
 
 static bool isCoalescable(const MachineInstr &MI) { return MI.isFullCopy(); }
 
-Register RegAllocFast::traceCopyChain(Register Reg) const {
+Register RegAllocFastImpl::traceCopyChain(Register Reg) const {
   static const unsigned ChainLengthLimit = 3;
   unsigned C = 0;
   do {
@@ -843,7 +859,7 @@ Register RegAllocFast::traceCopyChain(Register Reg) const {
 /// Check if any of \p VirtReg's definitions is a copy. If it is follow the
 /// chain o...
[truncated]

if constexpr (is_detected<has_get_cleared_properties_t, PassT>::value)
IR.getProperties().reset(PassT::getClearedProperties());
IR.getProperties().reset(this->Pass.getClearedProperties());
Copy link
Contributor Author

@paperchalice paperchalice Jun 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should move them into a dedicated function, like getMachineFunctionPassPreservedAnalyses, so PassT().run(MF, MFAM) can set machine function properties properly...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking with Chandler a while back about the nicest way to do this, and he agreed that my proposal of a helper function that takes in a MachineFunction/MachineFunctionProperties to verify, and manually calling that at the beginning of run() was his preferred way. But anyway, not relevant to this PR

[PB = this](StringRef Params) {
return parseRegAllocFastPassOptions(*PB, Params);
},
"filter=all|reg-class;clear-vregs;no-clear-vregs")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will clash with another PR that changes this to filter individual registers. Maybe just change reg-class to something else?

Also the default should just be clear-vregs. Only need an option for no-clear-vregs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now renamed to reg-class, also update printPipeline so we can test output in --regalloc-npm in future.

[PB = this](StringRef Params) {
return parseRegAllocFastPassOptions(*PB, Params);
},
"reg-class=name;no-clear-vregs")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear the move is away from reg-class. How about reg-filter, or register-filter

Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg with test for textual pipeline

if constexpr (is_detected<has_get_cleared_properties_t, PassT>::value)
IR.getProperties().reset(PassT::getClearedProperties());
IR.getProperties().reset(this->Pass.getClearedProperties());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking with Chandler a while back about the nicest way to do this, and he agreed that my proposal of a helper function that takes in a MachineFunction/MachineFunctionProperties to verify, and manually calling that at the beginning of run() was his preferred way. But anyway, not relevant to this PR

@@ -1161,6 +1162,38 @@ Expected<SmallVector<std::string, 0>> parseInternalizeGVs(StringRef Params) {
return Expected<SmallVector<std::string, 0>>(std::move(PreservedGVs));
}

Expected<RegAllocFastPassOptions>
parseRegAllocFastPassOptions(PassBuilder &PB, StringRef Params) {
RegAllocFastPassOptions Opts;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test for parsing/pipeline printing roundtrip

Copy link
Collaborator

@cdevadas cdevadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@paperchalice paperchalice merged commit 1bc8b32 into llvm:main Jun 7, 2024
5 of 6 checks passed
paperchalice added a commit to paperchalice/llvm-project that referenced this pull request Jun 7, 2024
The test will generate an empty regalloc.s file in test, which causes an unresolved test.
paperchalice added a commit that referenced this pull request Jun 7, 2024
The test will generate an empty `regalloc-amdgpu.s` file in test, which
causes an unresolved test.
@paperchalice paperchalice deleted the ra-fast branch June 7, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants