Skip to content

Commit

Permalink
[BasicBlockUtils] Allow splitting predecessors with callbr terminators
Browse files Browse the repository at this point in the history 
SplitBlockPredecessors currently asserts if one of the predecessor
terminators is a callbr. This limitation was originally necessary,
because just like with indirectbr, it was not possible to replace
successors of a callbr. However, this is no longer the case since
D67252. As the requirement nowadays is that callbr must reference
all blockaddrs directly in the call arguments, and these get
automatically updated when setSuccessor() is called, we no longer
need this limitation.

The only thing we need to do here is use replaceSuccessorWith()
instead of replaceUsesOfWith(), because only the former does the
necessary blockaddr updating magic.

I believe there's other similar limitations that can be removed,
e.g. related to critical edge splitting.

Differential Revision: https://reviews.llvm.org/D129205
  • Loading branch information
@nikic
nikic committed Jul 7, 2022
1 parent 66e15d4 commit 40a4078
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 50 deletions.
14 changes: 5 additions & 9 deletions llvm/lib/Transforms/Scalar/JumpThreading.cpp
View file
Expand Up @@ -1459,9 +1459,7 @@ bool JumpThreadingPass::simplifyPartiallyRedundantLoad(LoadInst *LoadI) {
// Add all the unavailable predecessors to the PredsToSplit list.
for (BasicBlock *P : predecessors(LoadBB)) {
// If the predecessor is an indirect goto, we can't split the edge.
// Same for CallBr.
if (isa<IndirectBrInst>(P->getTerminator()) ||
isa<CallBrInst>(P->getTerminator()))
if (isa<IndirectBrInst>(P->getTerminator()))
return false;

if (!AvailablePredSet.count(P))
Expand Down Expand Up @@ -1685,9 +1683,8 @@ bool JumpThreadingPass::processThreadableEdges(Value *Cond, BasicBlock *BB,
}

// If the predecessor ends with an indirect goto, we can't change its
// destination. Same for CallBr.
if (isa<IndirectBrInst>(Pred->getTerminator()) ||
isa<CallBrInst>(Pred->getTerminator()))
// destination.
if (isa<IndirectBrInst>(Pred->getTerminator()))
continue;

PredToDestList.emplace_back(Pred, DestBB);
Expand Down Expand Up @@ -1924,10 +1921,9 @@ bool JumpThreadingPass::processBranchOnXOR(BinaryOperator *BO) {
}

// If any of predecessors end with an indirect goto, we can't change its
// destination. Same for CallBr.
// destination.
if (any_of(BlocksToFoldInto, [](BasicBlock *Pred) {
return isa<IndirectBrInst>(Pred->getTerminator()) ||
isa<CallBrInst>(Pred->getTerminator());
return isa<IndirectBrInst>(Pred->getTerminator());
}))
return false;

Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Transforms/Scalar/LICM.cpp
View file
Expand Up @@ -1508,8 +1508,7 @@ static bool canSplitPredecessors(PHINode *PN, LoopSafetyInfo *SafetyInfo) {
if (!SafetyInfo->getBlockColors().empty() && BB->getFirstNonPHI()->isEHPad())
return false;
for (BasicBlock *BBPred : predecessors(BB)) {
if (isa<IndirectBrInst>(BBPred->getTerminator()) ||
isa<CallBrInst>(BBPred->getTerminator()))
if (isa<IndirectBrInst>(BBPred->getTerminator()))
return false;
}
return true;
Expand Down
4 changes: 1 addition & 3 deletions llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
View file
Expand Up @@ -1132,9 +1132,7 @@ SplitBlockPredecessorsImpl(BasicBlock *BB, ArrayRef<BasicBlock *> Preds,
// all BlockAddress uses would need to be updated.
assert(!isa<IndirectBrInst>(Preds[i]->getTerminator()) &&
"Cannot split an edge from an IndirectBrInst");
assert(!isa<CallBrInst>(Preds[i]->getTerminator()) &&
"Cannot split an edge from a CallBrInst");
Preds[i]->getTerminator()->replaceUsesOfWith(BB, NewBB);
Preds[i]->getTerminator()->replaceSuccessorWith(BB, NewBB);
}

// Insert a new PHI node into NewBB for every PHI node in BB and that new PHI
Expand Down
3 changes: 0 additions & 3 deletions llvm/lib/Transforms/Utils/LoopUtils.cpp
View file
Expand Up @@ -75,9 +75,6 @@ bool llvm::formDedicatedExitBlocks(Loop *L, DominatorTree *DT, LoopInfo *LI,
if (isa<IndirectBrInst>(PredBB->getTerminator()))
// We cannot rewrite exiting edges from an indirectbr.
return false;
if (isa<CallBrInst>(PredBB->getTerminator()))
// We cannot rewrite exiting edges from a callbr.
return false;

InLoopPredecessors.push_back(PredBB);
} else {
Expand Down
3 changes: 1 addition & 2 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
View file
Expand Up @@ -3033,8 +3033,7 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,

// Skip if the predecessor's terminator is an indirect branch.
if (any_of(PredBBs, [](BasicBlock *PredBB) {
return isa<IndirectBrInst>(PredBB->getTerminator()) ||
isa<CallBrInst>(PredBB->getTerminator());
return isa<IndirectBrInst>(PredBB->getTerminator());
}))
continue;

Expand Down
18 changes: 9 additions & 9 deletions llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
View file
Expand Up @@ -8,18 +8,18 @@
define i32 @c() {
; CHECK-LABEL: @c(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* @a
; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* @a, align 4
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[TMP0]], 0
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
; CHECK: if.then:
; CHECK-NEXT: [[CALL:%.*]] = call i32 @b()
; CHECK-NEXT: [[PHITMP:%.*]] = icmp ne i32 [[CALL]], 0
; CHECK-NEXT: br i1 [[PHITMP]], label [[IF_THEN2:%.*]], label [[IF_END4:%.*]]
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
; CHECK: if.else:
; CHECK-NEXT: callbr void asm sideeffect "", "i"(i8* blockaddress(@c, [[IF_THEN2]]))
; CHECK-NEXT: to label [[IF_END_THREAD:%.*]] [label %if.then2]
; CHECK: if.end.thread:
; CHECK-NEXT: callbr void asm sideeffect "", "i"(i8* blockaddress(@c, [[IF_THEN2:%.*]]))
; CHECK-NEXT: to label [[NORMAL:%.*]] [label %if.then2]
; CHECK: normal:
; CHECK-NEXT: br label [[IF_THEN2]]
; CHECK: if.end:
; CHECK-NEXT: [[CALL:%.*]] = call i32 @b()
; CHECK-NEXT: [[PHITMP:%.*]] = icmp ne i32 [[CALL]], 0
; CHECK-NEXT: br i1 [[PHITMP]], label [[IF_THEN2]], label [[IF_END4:%.*]]
; CHECK: if.then2:
; CHECK-NEXT: [[CALL3:%.*]] = call i32 @b()
; CHECK-NEXT: br label [[IF_END4]]
Expand Down
23 changes: 12 additions & 11 deletions llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
View file
Expand Up @@ -6,21 +6,22 @@
define i1 @func(i1 %arg, i32 %arg1, i1 %arg2) {
; CHECK-LABEL: @func(
; CHECK-NEXT: bb:
; CHECK-NEXT: br i1 [[ARG:%.*]], label [[BB3:%.*]], label [[BB4:%.*]]
; CHECK: bb3:
; CHECK-NEXT: [[I:%.*]] = icmp eq i32 [[ARG1:%.*]], 0
; CHECK-NEXT: br label [[BB7:%.*]]
; CHECK-NEXT: br i1 [[ARG:%.*]], label [[BB7:%.*]], label [[BB4:%.*]]
; CHECK: bb4:
; CHECK-NEXT: callbr void asm sideeffect "", "i"(i8* blockaddress(@func, [[BB7]]))
; CHECK-NEXT: to label [[BB5:%.*]] [label %bb7]
; CHECK-NEXT: callbr void asm sideeffect "", "i"(i8* blockaddress(@func, [[BB7_THR_COMM:%.*]]))
; CHECK-NEXT: to label [[BB5:%.*]] [label %bb7.thr_comm]
; CHECK: bb5:
; CHECK-NEXT: br label [[BB7]]
; CHECK-NEXT: br label [[BB7_THR_COMM]]
; CHECK: bb7.thr_comm:
; CHECK-NEXT: [[I91:%.*]] = xor i1 [[ARG2:%.*]], [[ARG]]
; CHECK-NEXT: br i1 [[I91]], label [[BB11:%.*]], label [[BB11]]
; CHECK: bb7:
; CHECK-NEXT: [[I8:%.*]] = phi i1 [ [[I]], [[BB3]] ], [ [[ARG2:%.*]], [[BB5]] ], [ [[ARG2]], [[BB4]] ]
; CHECK-NEXT: [[I9:%.*]] = xor i1 [[I8]], [[ARG]]
; CHECK-NEXT: br i1 [[I9]], label [[BB11:%.*]], label [[BB11]]
; CHECK-NEXT: [[I:%.*]] = icmp eq i32 [[ARG1:%.*]], 0
; CHECK-NEXT: [[I9:%.*]] = xor i1 [[I]], [[ARG]]
; CHECK-NEXT: br i1 [[I9]], label [[BB11]], label [[BB11]]
; CHECK: bb11:
; CHECK-NEXT: ret i1 [[I9]]
; CHECK-NEXT: [[I93:%.*]] = phi i1 [ [[I91]], [[BB7_THR_COMM]] ], [ [[I9]], [[BB7]] ], [ [[I91]], [[BB7_THR_COMM]] ], [ [[I9]], [[BB7]] ]
; CHECK-NEXT: ret i1 [[I93]]
;
bb:
br i1 %arg, label %bb3, label %bb4
Expand Down
14 changes: 10 additions & 4 deletions llvm/test/Transforms/LICM/callbr-crash.ll
View file
Expand Up @@ -6,12 +6,18 @@ define i32 @j() {
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_COND:%.*]]
; CHECK: for.cond:
; CHECK-NEXT: callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@j, [[FOR_END:%.*]]))
; CHECK-NEXT: to label [[COND_TRUE_I:%.*]] [label %for.end]
; CHECK-NEXT: callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@j, [[FOR_END_SPLIT_LOOP_EXIT1:%.*]]))
; CHECK-NEXT: to label [[COND_TRUE_I:%.*]] [label %for.end.split.loop.exit1]
; CHECK: cond.true.i:
; CHECK-NEXT: br i1 true, label [[FOR_END]], label [[FOR_COND]]
; CHECK-NEXT: br i1 true, label [[FOR_END_SPLIT_LOOP_EXIT:%.*]], label [[FOR_COND]]
; CHECK: for.end.split.loop.exit:
; CHECK-NEXT: [[ASMRESULT1_I_I_LE:%.*]] = extractvalue { i8, i32 } zeroinitializer, 1
; CHECK-NEXT: br label [[FOR_END:%.*]]
; CHECK: for.end.split.loop.exit1:
; CHECK-NEXT: [[PHI_PH2:%.*]] = phi i32 [ undef, [[FOR_COND]] ]
; CHECK-NEXT: br label [[FOR_END]]
; CHECK: for.end:
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ 0, [[COND_TRUE_I]] ], [ undef, [[FOR_COND]] ]
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[ASMRESULT1_I_I_LE]], [[FOR_END_SPLIT_LOOP_EXIT]] ], [ [[PHI_PH2]], [[FOR_END_SPLIT_LOOP_EXIT1]] ]
; CHECK-NEXT: ret i32 [[PHI]]
;
entry:
Expand Down
8 changes: 1 addition & 7 deletions llvm/test/Transforms/SimplifyCFG/jump-threading.ll
View file
Expand Up @@ -425,14 +425,8 @@ define void @callbr() {
; CHECK-LABEL: @callbr(
; CHECK-NEXT: entry:
; CHECK-NEXT: callbr void asm sideeffect "", "i,~{dirflag},~{fpsr},~{flags}"(ptr blockaddress(@callbr, [[TARGET:%.*]]))
; CHECK-NEXT: to label [[JOIN:%.*]] [label %target]
; CHECK-NEXT: to label [[IF_END:%.*]] [label %target]
; CHECK: target:
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ false, [[TARGET]] ], [ false, [[ENTRY:%.*]] ]
; CHECK-NEXT: br i1 [[PHI]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: ret void
Expand Down

0 comments on commit 40a4078

Please sign in to comment.