Skip to content

[AMDGPU][SplitModule] Keep looking for more dependencies after finding an indirect call #93480

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

Conversation

Pierre-vh
Copy link
Contributor

This is just something I noticed while going over this pass logic one more time and didn't cause issues (yet). If we find an indirect call, we stop looking assuming we added all functions to the list, but if not all functions in the module were indirectly callable, some may still be missing.

Just to be safe, keep looking until we did everything we could to find dependencies, so we don't accidentally miss one.

…g an indirect call

This is just something I noticed while going over this pass logic one more time and didn't cause issues (yet).
If we find an indirect call, we stop looking assuming we added all
functions to the list, but if not all functions in the module were
indirectly callable, some may still be missing.

Just to be safe, keep looking until we did everything we could to find dependencies, so we don't accidentally miss one.
@llvmbot
Copy link
Member

llvmbot commented May 27, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Pierre van Houtryve (Pierre-vh)

Changes

This is just something I noticed while going over this pass logic one more time and didn't cause issues (yet). If we find an indirect call, we stop looking assuming we added all functions to the list, but if not all functions in the module were indirectly callable, some may still be missing.

Just to be safe, keep looking until we did everything we could to find dependencies, so we don't accidentally miss one.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp (+1-1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
index dab773f28752d..2449fa581842a 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
@@ -340,7 +340,7 @@ static void addAllDependencies(SplitModuleLogger &SML, const CallGraph &CG,
         // TODO: Print an ORE as well ?
         addAllIndirectCallDependencies(M, Fns);
         HadIndirectCall = true;
-        return;
+        continue;
       }
 
       if (Callee->isDeclaration())

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

testcase?

@Pierre-vh Pierre-vh merged commit 42c4027 into llvm:main May 28, 2024
3 of 4 checks passed
@Pierre-vh Pierre-vh deleted the fix-splitmodule-indirectcall-deps branch May 28, 2024 06:18
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request May 28, 2024
…g an indirect call (llvm#93480)

This is just something I noticed while going over this pass logic one
more time and didn't cause issues (yet). If we find an indirect call, we
stop looking assuming we added all functions to the list, but if not all
functions in the module were indirectly callable, some may still be
missing.

Just to be safe, keep looking until we did everything we could to find
dependencies, so we don't accidentally miss one.

Change-Id: I03cef564103e389d4f76d162bda10a0da0e1bff2
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