[GVN] Check IndirectBr in Predecessor Terminators #151188
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-transforms Author: hanbeom (ParkHanbum) ChangesCritical edges with an IndirectBr terminator cannot be split. Fixes : #150229 Full diff: https://github.com/llvm/llvm-project/pull/151188.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index f6bf09d09433d..196f68e2d06d8 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -3297,8 +3297,10 @@ void GVNPass::addDeadBlock(BasicBlock *BB) {
if (!DeadBlocks.count(P))
continue;
+ auto PredTerminator = P->getTerminator();
if (is_contained(successors(P), B) &&
- isCriticalEdge(P->getTerminator(), B)) {
+ !isa<IndirectBrInst>(PredTerminator) &&
+ isCriticalEdge(PredTerminator, B)) {
if (BasicBlock *S = splitCriticalEdges(P, B))
DeadBlocks.insert(P = S);
}
diff --git a/llvm/test/Transforms/GVN/cond_br.ll b/llvm/test/Transforms/GVN/cond_br.ll
index 19166d17a8320..fb84b626c7455 100644
--- a/llvm/test/Transforms/GVN/cond_br.ll
+++ b/llvm/test/Transforms/GVN/cond_br.ll
@@ -53,3 +53,22 @@ if.end: ; preds = %if.else, %if.then
}
declare void @bar(i32)
+
+define void @indirectbr_could_not_split() {
+; CHECK-LABEL: define void @indirectbr_could_not_split() {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br i1 false, label %[[IBR:.*]], label %[[EXIT:.*]]
+; CHECK: [[IBR]]:
+; CHECK-NEXT: indirectbr ptr null, [label %[[EXIT]], label %exit]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 false, label %ibr, label %exit
+
+ibr:
+ indirectbr ptr null, [label %exit, label %exit]
+
+exit:
+ ret void
+}
|
nikic
reviewed
Jul 29, 2025
There was a problem hiding this comment.
This works, but I wonder whether it would be better to change SplitKnownCriticalEdge to convert the assertion into an if? SplitKnownCriticalEdge already has a bunch of other bailouts (e.g. for EH pads), so I'm not sure it makes sense to make this one thing an assert and force the caller to check it.
|
@nikic at least no problem was found in the testcases when I modified SplitKnownCriticalEdge as you said. could I change it? |
Yes, I think that would be best. |
Critical edges with an IndirectBr terminator cannot be split. We have already handled it with Assertion, but change it to checking whether block has IndirectBr terminator. Fixes : llvm#150229
|
/cherry-pick a750fcb |
|
/pull-request #152965 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/25185 Here is the relevant piece of the build log for the reference |
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 11, 2025
Critical edges with an IndirectBr terminator cannot be split. Add a check it to prevent assertion failures. Fixes: llvm#150229 (cherry picked from commit a750fcb)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Critical edges with an IndirectBr terminator cannot be split.
Add a check it to prevent assertion failures.
Fixes : #150229