Skip to content

[AMDGPU] Always Inline preserved analyses #91198

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
May 9, 2024

Conversation

jofrn
Copy link
Contributor

@jofrn jofrn commented May 6, 2024

When replacing all uses, the structural-hash of the IR can change, so keep track of changes using Changed variable and return it to pass manager.

@jofrn jofrn requested review from arsenm and shiltian May 6, 2024 12:29
@llvmbot
Copy link
Member

llvmbot commented May 6, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: None (jofrn)

Changes

When replacing all uses, the structural-hash of the IR can change, so keep track of changes using Preserved variable and return the correct IR state (changed or not).


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp (+10-6)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
index b53def912ab618..1ba3669f523352 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
@@ -42,7 +42,7 @@ class AMDGPUAlwaysInline : public ModulePass {
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
- }
+  }
 };
 
 } // End anonymous namespace
@@ -54,7 +54,8 @@ char AMDGPUAlwaysInline::ID = 0;
 
 static void
 recursivelyVisitUsers(GlobalValue &GV,
-                      SmallPtrSetImpl<Function *> &FuncsToAlwaysInline) {
+                      SmallPtrSetImpl<Function *> &FuncsToAlwaysInline,
+                      bool &Preserved) {
   SmallVector<User *, 16> Stack(GV.users());
 
   SmallPtrSet<const Value *, 8> Visited;
@@ -73,6 +74,7 @@ recursivelyVisitUsers(GlobalValue &GV,
         // Unfortunately, clang adds noinline to all functions at -O0. We have
         // to override this here until that's fixed.
         F->removeFnAttr(Attribute::NoInline);
+        Preserved = false;
 
         FuncsToAlwaysInline.insert(F);
         Stack.push_back(F);
@@ -89,6 +91,7 @@ recursivelyVisitUsers(GlobalValue &GV,
 static bool alwaysInlineImpl(Module &M, bool GlobalOpt) {
   std::vector<GlobalAlias*> AliasesToRemove;
 
+  bool Preserved = true;
   SmallPtrSet<Function *, 8> FuncsToAlwaysInline;
   SmallPtrSet<Function *, 8> FuncsToNoInline;
   Triple TT(M.getTargetTriple());
@@ -98,6 +101,7 @@ static bool alwaysInlineImpl(Module &M, bool GlobalOpt) {
       if (TT.getArch() == Triple::amdgcn &&
           A.getLinkage() != GlobalValue::InternalLinkage)
         continue;
+      Preserved = false;
       A.replaceAllUsesWith(F);
       AliasesToRemove.push_back(&A);
     }
@@ -128,7 +132,7 @@ static bool alwaysInlineImpl(Module &M, bool GlobalOpt) {
     if ((AS == AMDGPUAS::REGION_ADDRESS) ||
         (AS == AMDGPUAS::LOCAL_ADDRESS &&
          (!AMDGPUTargetMachine::EnableLowerModuleLDS)))
-      recursivelyVisitUsers(GV, FuncsToAlwaysInline);
+      recursivelyVisitUsers(GV, FuncsToAlwaysInline, Preserved);
   }
 
   if (!AMDGPUTargetMachine::EnableFunctionCalls || StressCalls) {
@@ -153,7 +157,7 @@ static bool alwaysInlineImpl(Module &M, bool GlobalOpt) {
   for (Function *F : FuncsToNoInline)
     F->addFnAttr(Attribute::NoInline);
 
-  return !FuncsToAlwaysInline.empty() || !FuncsToNoInline.empty();
+  return !Preserved || !FuncsToAlwaysInline.empty() || !FuncsToNoInline.empty();
 }
 
 bool AMDGPUAlwaysInline::runOnModule(Module &M) {
@@ -166,6 +170,6 @@ ModulePass *llvm::createAMDGPUAlwaysInlinePass(bool GlobalOpt) {
 
 PreservedAnalyses AMDGPUAlwaysInlinePass::run(Module &M,
                                               ModuleAnalysisManager &AM) {
-  alwaysInlineImpl(M, GlobalOpt);
-  return PreservedAnalyses::all();
+  const bool Changed = alwaysInlineImpl(M, GlobalOpt);
+  return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
 }

@jofrn jofrn force-pushed the alwaysinline-structuralhash branch from ac1b079 to afd1b5d Compare May 6, 2024 12:52
When replacing all uses, the structural-hash of the IR can change, so keep track of changes using Changed variable and return it to pass manager.
@jofrn jofrn force-pushed the alwaysinline-structuralhash branch from afd1b5d to 8841b04 Compare May 6, 2024 14:15
Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

LGTM

@jofrn jofrn merged commit 1494d88 into llvm:main May 9, 2024
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