Skip to content

Conversation

@akshayrdeodhar
Copy link
Contributor

Add a NewPM interface for NVPTXLowerArgs

@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-backend-nvptx

Author: Akshay Deodhar (akshayrdeodhar)

Changes

Add a NewPM interface for NVPTXLowerArgs


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

2 Files Affected:

  • (modified) llvm/lib/Target/NVPTX/NVPTX.h (+10)
  • (modified) llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp (+18-20)
diff --git a/llvm/lib/Target/NVPTX/NVPTX.h b/llvm/lib/Target/NVPTX/NVPTX.h
index ca915cd3f3732..44be26b12f0c4 100644
--- a/llvm/lib/Target/NVPTX/NVPTX.h
+++ b/llvm/lib/Target/NVPTX/NVPTX.h
@@ -18,6 +18,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Target/TargetMachine.h"
 
 namespace llvm {
 class FunctionPass;
@@ -74,6 +75,15 @@ struct NVPTXCopyByValArgsPass : PassInfoMixin<NVPTXCopyByValArgsPass> {
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 };
 
+struct NVPTXLowerArgsPass : PassInfoMixin<NVPTXLowerArgsPass> {
+  private:
+    TargetMachine &TM;
+
+  public:
+    NVPTXLowerArgsPass(TargetMachine& TM) : TM(TM) {};
+    PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+};
+
 namespace NVPTX {
 enum DrvInterface {
   NVCL,
diff --git a/llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp b/llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
index c763b54c8dbfe..0a65a0df8af3c 100644
--- a/llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
@@ -168,17 +168,6 @@ namespace {
 class NVPTXLowerArgs : public FunctionPass {
   bool runOnFunction(Function &F) override;
 
-  bool runOnKernelFunction(const NVPTXTargetMachine &TM, Function &F);
-  bool runOnDeviceFunction(const NVPTXTargetMachine &TM, Function &F);
-
-  // handle byval parameters
-  void handleByValParam(const NVPTXTargetMachine &TM, Argument *Arg);
-  // Knowing Ptr must point to the global address space, this function
-  // addrspacecasts Ptr to global and then back to generic. This allows
-  // NVPTXInferAddressSpaces to fold the global-to-generic cast into
-  // loads/stores that appear later.
-  void markPointerAsGlobal(Value *Ptr);
-
 public:
   static char ID; // Pass identification, replacement for typeid
   NVPTXLowerArgs() : FunctionPass(ID) {}
@@ -571,8 +560,7 @@ void copyByValParam(Function &F, Argument &Arg) {
 }
 } // namespace
 
-void NVPTXLowerArgs::handleByValParam(const NVPTXTargetMachine &TM,
-                                      Argument *Arg) {
+static void handleByValParam(const NVPTXTargetMachine &TM, Argument *Arg) {
   Function *Func = Arg->getParent();
   bool HasCvtaParam =
       TM.getSubtargetImpl(*Func)->hasCvtaParam() && isKernelFunction(*Func);
@@ -641,7 +629,11 @@ void NVPTXLowerArgs::handleByValParam(const NVPTXTargetMachine &TM,
     copyByValParam(*Func, *Arg);
 }
 
-void NVPTXLowerArgs::markPointerAsGlobal(Value *Ptr) {
+// Knowing Ptr must point to the global address space, this function
+// addrspacecasts Ptr to global and then back to generic. This allows
+// NVPTXInferAddressSpaces to fold the global-to-generic cast into
+// loads/stores that appear later.
+static void markPointerAsGlobal(Value *Ptr) {
   if (Ptr->getType()->getPointerAddressSpace() != ADDRESS_SPACE_GENERIC)
     return;
 
@@ -670,13 +662,12 @@ void NVPTXLowerArgs::markPointerAsGlobal(Value *Ptr) {
 // =============================================================================
 // Main function for this pass.
 // =============================================================================
-bool NVPTXLowerArgs::runOnKernelFunction(const NVPTXTargetMachine &TM,
-                                         Function &F) {
+static bool runOnKernelFunction(const NVPTXTargetMachine &TM,Function &F) {
   // Copying of byval aggregates + SROA may result in pointers being loaded as
   // integers, followed by intotoptr. We may want to mark those as global, too,
   // but only if the loaded integer is used exclusively for conversion to a
   // pointer with inttoptr.
-  auto HandleIntToPtr = [this](Value &V) {
+  auto HandleIntToPtr = [](Value &V) {
     if (llvm::all_of(V.users(), [](User *U) { return isa<IntToPtrInst>(U); })) {
       SmallVector<User *, 16> UsersToUpdate(V.users());
       for (User *U : UsersToUpdate)
@@ -721,8 +712,7 @@ bool NVPTXLowerArgs::runOnKernelFunction(const NVPTXTargetMachine &TM,
 }
 
 // Device functions only need to copy byval args into local memory.
-bool NVPTXLowerArgs::runOnDeviceFunction(const NVPTXTargetMachine &TM,
-                                         Function &F) {
+static bool runOnDeviceFunction(const NVPTXTargetMachine &TM, Function &F) {
   LLVM_DEBUG(dbgs() << "Lowering function args of " << F.getName() << "\n");
   for (Argument &Arg : F.args())
     if (Arg.getType()->isPointerTy() && Arg.hasByValAttr())
@@ -736,7 +726,6 @@ bool NVPTXLowerArgs::runOnFunction(Function &F) {
   return isKernelFunction(F) ? runOnKernelFunction(TM, F)
                              : runOnDeviceFunction(TM, F);
 }
-
 FunctionPass *llvm::createNVPTXLowerArgsPass() { return new NVPTXLowerArgs(); }
 
 static bool copyFunctionByValArgs(Function &F) {
@@ -757,3 +746,12 @@ PreservedAnalyses NVPTXCopyByValArgsPass::run(Function &F,
   return copyFunctionByValArgs(F) ? PreservedAnalyses::none()
                                   : PreservedAnalyses::all();
 }
+
+PreservedAnalyses NVPTXLowerArgsPass::run(Function &F,
+                                          FunctionAnalysisManager &AM) {
+  auto &NTM = static_cast<NVPTXTargetMachine &>(TM);
+  bool Changed = isKernelFunction(F) ? runOnKernelFunction(NTM, F)
+                                     : runOnDeviceFunction(NTM, F);
+  return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
+
+}
\ No newline at end of file

Copy link
Member

@AlexMaclean AlexMaclean left a comment

Choose a reason for hiding this comment

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

Could you please add this new pass to NVPTXPassRegistry.def and write a simple lit test for it as well? Right now this looks like it is going to be dead code.

@github-actions
Copy link

github-actions bot commented Feb 27, 2025

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

Copy link
Member

@Artem-B Artem-B left a comment

Choose a reason for hiding this comment

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

LGTM with a few nits.

Copy link
Member

@AlexMaclean AlexMaclean left a comment

Choose a reason for hiding this comment

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

LGTM

@akshayrdeodhar akshayrdeodhar force-pushed the upstream/lowerargs-newpm branch from e47285b to 5aac60e Compare February 28, 2025 23:15
@akshayrdeodhar akshayrdeodhar force-pushed the upstream/lowerargs-newpm branch from 5aac60e to 7ddab29 Compare February 28, 2025 23:23
@akshayrdeodhar akshayrdeodhar merged commit cb2ee1e into llvm:main Mar 21, 2025
11 checks passed
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