Skip to content
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

[AMDGPU][NewPM] Port SIFixVGPRCopies to NPM #123592

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

optimisan
Copy link
Contributor

Extends NPM pipeline support till PostRegAlloc passes (greedy is in the works)

Extends NPM pipeline till PostRegAlloc passes.
@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Akshat Oke (optimisan)

Changes

Extends NPM pipeline support till PostRegAlloc passes (greedy is in the works)


Full diff: https://github.com/llvm/llvm-project/pull/123592.diff

5 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPU.h (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def (+1)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (+3-2)
  • (modified) llvm/lib/Target/AMDGPU/SIFixVGPRCopies.cpp (+24-8)
  • (modified) llvm/test/CodeGen/AMDGPU/fix-vgpr-copies.mir (+2)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPU.h b/llvm/lib/Target/AMDGPU/AMDGPU.h
index 5d9a830f041a74..12a8c155d3de2e 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPU.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPU.h
@@ -177,7 +177,7 @@ extern char &SIShrinkInstructionsLegacyID;
 void initializeSIFixSGPRCopiesLegacyPass(PassRegistry &);
 extern char &SIFixSGPRCopiesLegacyID;
 
-void initializeSIFixVGPRCopiesPass(PassRegistry &);
+void initializeSIFixVGPRCopiesLegacyPass(PassRegistry &);
 extern char &SIFixVGPRCopiesID;
 
 void initializeSILowerWWMCopiesPass(PassRegistry &);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def b/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
index 09a39d23d801b9..ce3aeb93ec4caa 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
@@ -99,6 +99,7 @@ FUNCTION_PASS_WITH_PARAMS(
 MACHINE_FUNCTION_PASS("amdgpu-isel", AMDGPUISelDAGToDAGPass(*this))
 MACHINE_FUNCTION_PASS("si-fix-sgpr-copies", SIFixSGPRCopiesPass())
 MACHINE_FUNCTION_PASS("si-i1-copies", SILowerI1CopiesPass())
+MACHINE_FUNCTION_PASS("si-fix-vgpr-copies", SIFixVGPRCopiesPass())
 MACHINE_FUNCTION_PASS("si-fold-operands", SIFoldOperandsPass());
 MACHINE_FUNCTION_PASS("gcn-dpp-combine", GCNDPPCombinePass())
 MACHINE_FUNCTION_PASS("si-load-store-opt", SILoadStoreOptimizerPass())
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index 53ec80b8f72049..3fe17457cb3606 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -36,6 +36,7 @@
 #include "R600.h"
 #include "R600TargetMachine.h"
 #include "SIFixSGPRCopies.h"
+#include "SIFixVGPRCopies.h"
 #include "SIFoldOperands.h"
 #include "SILoadStoreOptimizer.h"
 #include "SILowerControlFlow.h"
@@ -486,7 +487,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeAMDGPUTarget() {
   initializeAMDGPUMarkLastScratchLoadPass(*PR);
   initializeSILowerSGPRSpillsLegacyPass(*PR);
   initializeSIFixSGPRCopiesLegacyPass(*PR);
-  initializeSIFixVGPRCopiesPass(*PR);
+  initializeSIFixVGPRCopiesLegacyPass(*PR);
   initializeSIFoldOperandsLegacyPass(*PR);
   initializeSIPeepholeSDWALegacyPass(*PR);
   initializeSIShrinkInstructionsLegacyPass(*PR);
@@ -2107,7 +2108,7 @@ void AMDGPUCodeGenPassBuilder::addMachineSSAOptimization(
 }
 
 void AMDGPUCodeGenPassBuilder::addPostRegAlloc(AddMachinePass &addPass) const {
-  // addPass(SIFixVGPRCopiesID);
+  addPass(SIFixVGPRCopiesPass());
   if (TM.getOptLevel() > CodeGenOptLevel::None)
     addPass(SIOptimizeExecMaskingPass());
   Base::addPostRegAlloc(addPass);
diff --git a/llvm/lib/Target/AMDGPU/SIFixVGPRCopies.cpp b/llvm/lib/Target/AMDGPU/SIFixVGPRCopies.cpp
index 08272a9ddfd307..d0d679221eee03 100644
--- a/llvm/lib/Target/AMDGPU/SIFixVGPRCopies.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFixVGPRCopies.cpp
@@ -11,6 +11,7 @@
 ///
 //===----------------------------------------------------------------------===//
 
+#include "SIFixVGPRCopies.h"
 #include "AMDGPU.h"
 #include "GCNSubtarget.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
@@ -22,13 +23,12 @@ using namespace llvm;
 
 namespace {
 
-class SIFixVGPRCopies : public MachineFunctionPass {
+class SIFixVGPRCopiesLegacy : public MachineFunctionPass {
 public:
   static char ID;
 
-public:
-  SIFixVGPRCopies() : MachineFunctionPass(ID) {
-    initializeSIFixVGPRCopiesPass(*PassRegistry::getPassRegistry());
+  SIFixVGPRCopiesLegacy() : MachineFunctionPass(ID) {
+    initializeSIFixVGPRCopiesLegacyPass(*PassRegistry::getPassRegistry());
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
@@ -41,15 +41,31 @@ class SIFixVGPRCopies : public MachineFunctionPass {
   StringRef getPassName() const override { return "SI Fix VGPR copies"; }
 };
 
+class SIFixVGPRCopies {
+public:
+  bool run(MachineFunction &MF);
+};
+
 } // End anonymous namespace.
 
-INITIALIZE_PASS(SIFixVGPRCopies, DEBUG_TYPE, "SI Fix VGPR copies", false, false)
+INITIALIZE_PASS(SIFixVGPRCopiesLegacy, DEBUG_TYPE, "SI Fix VGPR copies", false,
+                false)
 
-char SIFixVGPRCopies::ID = 0;
+char SIFixVGPRCopiesLegacy::ID = 0;
 
-char &llvm::SIFixVGPRCopiesID = SIFixVGPRCopies::ID;
+char &llvm::SIFixVGPRCopiesID = SIFixVGPRCopiesLegacy::ID;
+
+PreservedAnalyses SIFixVGPRCopiesPass::run(MachineFunction &MF,
+                                           MachineFunctionAnalysisManager &) {
+  SIFixVGPRCopies().run(MF);
+  return PreservedAnalyses::all();
+}
+
+bool SIFixVGPRCopiesLegacy::runOnMachineFunction(MachineFunction &MF) {
+  return SIFixVGPRCopies().run(MF);
+}
 
-bool SIFixVGPRCopies::runOnMachineFunction(MachineFunction &MF) {
+bool SIFixVGPRCopies::run(MachineFunction &MF) {
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
   const SIRegisterInfo *TRI = ST.getRegisterInfo();
   const SIInstrInfo *TII = ST.getInstrInfo();
diff --git a/llvm/test/CodeGen/AMDGPU/fix-vgpr-copies.mir b/llvm/test/CodeGen/AMDGPU/fix-vgpr-copies.mir
index 7886ea16e67427..bc1f5416507a98 100644
--- a/llvm/test/CodeGen/AMDGPU/fix-vgpr-copies.mir
+++ b/llvm/test/CodeGen/AMDGPU/fix-vgpr-copies.mir
@@ -1,4 +1,6 @@
 # RUN: llc -mtriple=amdgcn -start-after=greedy -disable-copyprop -stop-after=si-optimize-exec-masking -o - %s | FileCheck %s
+# RUN: llc -mtriple=amdgcn -passes=si-fix-vgpr-copies,si-optimize-exec-masking -o - %s | FileCheck %s
+
 # Check that we first do all vector instructions and only then change exec
 # CHECK-DAG:  COPY $vgpr10_vgpr11
 # CHECK-DAG:  COPY $vgpr12_vgpr13

@optimisan optimisan requested review from arsenm and cdevadas January 20, 2025 11:26
PreservedAnalyses SIFixVGPRCopiesPass::run(MachineFunction &MF,
MachineFunctionAnalysisManager &) {
SIFixVGPRCopies().run(MF);
return PreservedAnalyses::all();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it OK to return all unconditionally?

Copy link
Contributor

@arsenm arsenm Jan 20, 2025

Choose a reason for hiding this comment

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

the legacy one does setPreservesAll, so this is just replicating that

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack. (s/DAG/legacy/?)

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.

4 participants