Skip to content

[AMDGPU] Improve amdgcn_kill verification#208220

Open
ro-i wants to merge 2 commits into
mainfrom
users/ro-i/improve-kill-verifier
Open

[AMDGPU] Improve amdgcn_kill verification#208220
ro-i wants to merge 2 commits into
mainfrom
users/ro-i/improve-kill-verifier

Conversation

@ro-i

@ro-i ro-i commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@llvmorg-github-actions

llvmorg-github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-amdgpu

Author: Robert Imschweiler (ro-i)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/IR/VerifierAMDGPU.cpp (+6-1)
diff --git a/llvm/lib/IR/VerifierAMDGPU.cpp b/llvm/lib/IR/VerifierAMDGPU.cpp
index 04cb214ef2520..ab63307570b28 100644
--- a/llvm/lib/IR/VerifierAMDGPU.cpp
+++ b/llvm/lib/IR/VerifierAMDGPU.cpp
@@ -145,7 +145,12 @@ void llvm::verifyAMDGPUIntrinsicCall(VerifierSupport &VS, Intrinsic::ID ID,
       Check(CBI->getNumIndirectDests() == 1,
             "callbr amdgcn_kill only supports one indirect dest");
       bool Unreachable = isa<UnreachableInst>(CBI->getIndirectDest(0)->begin());
-      CallInst *CI = dyn_cast<CallInst>(CBI->getIndirectDest(0)->begin());
+      // We assume that amdgcn_unreachable is only introduced by
+      // AMDGPUUnifyDivergentExitNodes, which replaces the block's original
+      // unreachable terminator by a call to amdgcn_unreachable + a return.
+      const Instruction *Term = CBI->getIndirectDest(0)->getTerminator();
+      const CallInst *CI =
+          Term ? dyn_cast_if_present<CallInst>(Term->getPrevNode()) : nullptr;
       Check(Unreachable ||
                 (CI && CI->getIntrinsicID() == Intrinsic::amdgcn_unreachable),
             "callbr amdgcn_kill indirect dest needs to be unreachable");

@shiltian shiltian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need any test?

@ro-i

ro-i commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

verifier has only negative tests, iirc? Could probably make a test in CodeGen/

@shiltian

shiltian commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

verifier has only negative tests, iirc?

Yes. I thought this PR is to capture something that might be missing before?

@ro-i

ro-i commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

yeah, makes sense. This also led me to relax the unreachable verification, as well, although that wasn't my initial intention (but ig it makes more sense)

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.

2 participants