Skip to content

[AMDGPU] Update removeFnAttrFromReachable to accept array of Fn Attrs. #94188

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 1 commit into from
Jun 6, 2024

Conversation

skc7
Copy link
Contributor

@skc7 skc7 commented Jun 3, 2024

This PR updates removeFnAttrFromReachable in AMDGPUMemoryUtils to accept array of function attributes as argument.
Helps to remove multiple attributes in one CallGraph walk.

@skc7 skc7 requested a review from arsenm June 3, 2024 06:02
@llvmbot
Copy link
Member

llvmbot commented Jun 3, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Chaitanya (skc7)

Changes

This PR updates removeFnAttrFromReachable in AMDGPUMemoryUtils to accept array of function attributes as argument.
Helps to remove multiple attributes in one CallGraph walk.


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

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp (+9-5)
  • (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.h (+2-1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
index 625ac0230f160..2bdbf4151dd95 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp
@@ -1017,7 +1017,7 @@ class AMDGPULowerModuleLDS {
       //
       // TODO: We could filter out subgraphs that do not access LDS globals.
       for (Function *F : KernelsThatAllocateTableLDS)
-        removeFnAttrFromReachable(CG, F, "amdgpu-no-lds-kernel-id");
+        removeFnAttrFromReachable(CG, F, {"amdgpu-no-lds-kernel-id"});
     }
 
     DenseMap<Function *, GlobalVariable *> KernelToCreatedDynamicLDS =
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp
index 239e0ee705729..4b046d8b212c9 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp
@@ -235,8 +235,9 @@ LDSUsesInfoTy getTransitiveUsesOfLDS(const CallGraph &CG, Module &M) {
 }
 
 void removeFnAttrFromReachable(CallGraph &CG, Function *KernelRoot,
-                               StringRef FnAttr) {
-  KernelRoot->removeFnAttr(FnAttr);
+                               const ArrayRef<StringRef> &FnAttrs) {
+  for (const auto &Attr : FnAttrs)
+    KernelRoot->removeFnAttr(Attr);
 
   SmallVector<Function *> WorkList = {CG[KernelRoot]->getFunction()};
   SmallPtrSet<Function *, 8> Visited;
@@ -261,12 +262,15 @@ void removeFnAttrFromReachable(CallGraph &CG, Function *KernelRoot,
             Function *PotentialCallee =
                 ExternalCallRecord.second->getFunction();
             assert(PotentialCallee);
-            if (!isKernelLDS(PotentialCallee))
-              PotentialCallee->removeFnAttr(FnAttr);
+            if (!isKernelLDS(PotentialCallee)) {
+              for (const auto &Attr : FnAttrs)
+                PotentialCallee->removeFnAttr(Attr);
+            }
           }
         }
       } else {
-        Callee->removeFnAttr(FnAttr);
+        for (const auto &Attr : FnAttrs)
+          Callee->removeFnAttr(Attr);
         if (Visited.insert(Callee).second)
           WorkList.push_back(Callee);
       }
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.h
index 4d3ad328e1310..fba3f42c51896 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUMEMORYUTILS_H
 #define LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUMEMORYUTILS_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 
@@ -54,7 +55,7 @@ LDSUsesInfoTy getTransitiveUsesOfLDS(const CallGraph &CG, Module &M);
 /// Strip FnAttr attribute from any functions where we may have
 /// introduced its use.
 void removeFnAttrFromReachable(CallGraph &CG, Function *KernelRoot,
-                               StringRef FnAttr);
+                               const ArrayRef<StringRef> &FnAttrs);
 
 /// Given a \p Def clobbering a load from \p Ptr according to the MSSA check
 /// if this is actually a memory update or an artificial clobber to facilitate

@@ -235,8 +235,9 @@ LDSUsesInfoTy getTransitiveUsesOfLDS(const CallGraph &CG, Module &M) {
}

void removeFnAttrFromReachable(CallGraph &CG, Function *KernelRoot,
StringRef FnAttr) {
KernelRoot->removeFnAttr(FnAttr);
const ArrayRef<StringRef> &FnAttrs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ArrayRef should be passed by value

@@ -54,7 +55,7 @@ LDSUsesInfoTy getTransitiveUsesOfLDS(const CallGraph &CG, Module &M);
/// Strip FnAttr attribute from any functions where we may have
/// introduced its use.
void removeFnAttrFromReachable(CallGraph &CG, Function *KernelRoot,
StringRef FnAttr);
const ArrayRef<StringRef> &FnAttrs);
Copy link
Contributor

Choose a reason for hiding this comment

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

by value, no const &

@skc7 skc7 force-pushed the skc7/prereq_swlds_1 branch from 88e96d2 to cd8220d Compare June 6, 2024 12:38
StringRef FnAttr) {
KernelRoot->removeFnAttr(FnAttr);
ArrayRef<StringRef> FnAttrs) {
for (const auto &Attr : FnAttrs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (const auto &Attr : FnAttrs)
for (StringRef Attr : FnAttrs)

if (!isKernelLDS(PotentialCallee))
PotentialCallee->removeFnAttr(FnAttr);
if (!isKernelLDS(PotentialCallee)) {
for (const auto &Attr : FnAttrs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (const auto &Attr : FnAttrs)
for (StringRef Attr : FnAttrs)

No references

}
}
} else {
Callee->removeFnAttr(FnAttr);
for (const auto &Attr : FnAttrs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (const auto &Attr : FnAttrs)
for (StringRef Attr : FnAttrs)

@skc7 skc7 force-pushed the skc7/prereq_swlds_1 branch from cd8220d to 6889c12 Compare June 6, 2024 12:52
@skc7 skc7 merged commit 7573d5e into llvm:main Jun 6, 2024
7 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.

3 participants