[GVN] Check IndirectBr in Predecessor Terminators

[ParkHanbum]

Critical edges with an IndirectBr terminator cannot be split.
Add a check it to prevent assertion failures.

Fixes : #150229

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: hanbeom (ParkHanbum)

Changes

Critical edges with an IndirectBr terminator cannot be split.
Add a check it to prevent assertion failures.

Fixes : #150229

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+3-1)
• (modified) llvm/test/Transforms/GVN/cond_br.ll (+19)

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]

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.

[ParkHanbum]

@nikic at least no problem was found in the testcases when I modified SplitKnownCriticalEdge as you said. could I change it?

[nikic]

@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.

[nikic]

LGTM

[marcauberer]

thanks!