Skip to content

Commit 45a291b

Browse files
[Dominators] check indirect branches of callbr
This will be necessary to support outputs from asm goto along indirect edges. Test via: $ pushd llvm/build; ninja IRTests; popd $ ./llvm/build/unittests/IR/IRTests \ --gtest_filter=DominatorTree.CallBrDomination Also, return nullptr in Instruction::getInsertionPointAfterDef for CallBrInst as was recommened in https://reviews.llvm.org/D135997#3991427. The following phab review was folded into this commit: https://reviews.llvm.org/D140166 Link: Link: https://discourse.llvm.org/t/rfc-syncing-asm-goto-with-outputs-with-gcc/65453/8 Reviewed By: void, efriedma, ChuanqiXu, MaskRay Differential Revision: https://reviews.llvm.org/D135997
1 parent df277ec commit 45a291b

File tree

10 files changed

+63
-35
lines changed

10 files changed

+63
-35
lines changed

llvm/include/llvm/IR/Dominators.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ class DominatorTree : public DominatorTreeBase<BasicBlock, false> {
191191
/// * Non-instruction Defs dominate everything.
192192
/// * Def does not dominate a use in Def itself (outside of degenerate cases
193193
/// like unreachable code or trivial phi cycles).
194-
/// * Invoke/callbr Defs only dominate uses in their default destination.
194+
/// * Invoke Defs only dominate uses in their default destination.
195195
bool dominates(const Value *Def, const Use &U) const;
196196
/// Return true if value Def dominates all possible uses inside instruction
197197
/// User. Same comments as for the Use-based API apply.

llvm/lib/IR/Dominators.cpp

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,6 @@ bool DominatorTree::dominates(const Instruction *Def,
194194
return dominates(E, UseBB);
195195
}
196196

197-
// Callbr results are similarly only usable in the default destination.
198-
if (const auto *CBI = dyn_cast<CallBrInst>(Def)) {
199-
BasicBlock *NormalDest = CBI->getDefaultDest();
200-
BasicBlockEdge E(DefBB, NormalDest);
201-
return dominates(E, UseBB);
202-
}
203-
204197
return dominates(DefBB, UseBB);
205198
}
206199

@@ -311,13 +304,6 @@ bool DominatorTree::dominates(const Value *DefV, const Use &U) const {
311304
return dominates(E, U);
312305
}
313306

314-
// Callbr results are similarly only usable in the default destination.
315-
if (const auto *CBI = dyn_cast<CallBrInst>(Def)) {
316-
BasicBlock *NormalDest = CBI->getDefaultDest();
317-
BasicBlockEdge E(DefBB, NormalDest);
318-
return dominates(E, U);
319-
}
320-
321307
// If the def and use are in different blocks, do a simple CFG dominator
322308
// tree query.
323309
if (DefBB != UseBB)

llvm/lib/IR/Instruction.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,9 @@ Instruction *Instruction::getInsertionPointAfterDef() {
139139
InsertBB = II->getNormalDest();
140140
InsertPt = InsertBB->getFirstInsertionPt();
141141
} else if (auto *CB = dyn_cast<CallBrInst>(this)) {
142-
InsertBB = CB->getDefaultDest();
143-
InsertPt = InsertBB->getFirstInsertionPt();
142+
// Def is available in multiple successors, there's no single dominating
143+
// insertion point.
144+
return nullptr;
144145
} else {
145146
assert(!isTerminator() && "Only invoke/callbr terminators return value");
146147
InsertBB = getParent();

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3477,13 +3477,17 @@ Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) {
34773477
}
34783478

34793479
/// If the callee is a constexpr cast of a function, attempt to move the cast to
3480-
/// the arguments of the call/callbr/invoke.
3480+
/// the arguments of the call/invoke.
3481+
/// CallBrInst is not supported.
34813482
bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
34823483
auto *Callee =
34833484
dyn_cast<Function>(Call.getCalledOperand()->stripPointerCasts());
34843485
if (!Callee)
34853486
return false;
34863487

3488+
assert(!isa<CallBrInst>(Call) &&
3489+
"CallBr's don't have a single point after a def to insert at");
3490+
34873491
// If this is a call to a thunk function, don't remove the cast. Thunks are
34883492
// used to transparently forward all incoming parameters and outgoing return
34893493
// values, so it's important to leave the cast in place.
@@ -3529,16 +3533,14 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
35293533
return false; // Attribute not compatible with transformed value.
35303534
}
35313535

3532-
// If the callbase is an invoke/callbr instruction, and the return value is
3536+
// If the callbase is an invoke instruction, and the return value is
35333537
// used by a PHI node in a successor, we cannot change the return type of
35343538
// the call because there is no place to put the cast instruction (without
35353539
// breaking the critical edge). Bail out in this case.
35363540
if (!Caller->use_empty()) {
35373541
BasicBlock *PhisNotSupportedBlock = nullptr;
35383542
if (auto *II = dyn_cast<InvokeInst>(Caller))
35393543
PhisNotSupportedBlock = II->getNormalDest();
3540-
if (auto *CB = dyn_cast<CallBrInst>(Caller))
3541-
PhisNotSupportedBlock = CB->getDefaultDest();
35423544
if (PhisNotSupportedBlock)
35433545
for (User *U : Caller->users())
35443546
if (PHINode *PN = dyn_cast<PHINode>(U))
@@ -3722,9 +3724,6 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
37223724
if (InvokeInst *II = dyn_cast<InvokeInst>(Caller)) {
37233725
NewCall = Builder.CreateInvoke(Callee, II->getNormalDest(),
37243726
II->getUnwindDest(), Args, OpBundles);
3725-
} else if (CallBrInst *CBI = dyn_cast<CallBrInst>(Caller)) {
3726-
NewCall = Builder.CreateCallBr(Callee, CBI->getDefaultDest(),
3727-
CBI->getIndirectDests(), Args, OpBundles);
37283727
} else {
37293728
NewCall = Builder.CreateCall(Callee, Args, OpBundles);
37303729
cast<CallInst>(NewCall)->setTailCallKind(

llvm/test/Transforms/Coroutines/coro-debug.ll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ attributes #7 = { noduplicate }
193193
; CHECK: %[[CALLBR_RES:.+]] = callbr i32 asm
194194
; CHECK-NEXT: to label %[[DEFAULT_DEST:.+]] [label
195195
; CHECK: [[DEFAULT_DEST]]:
196-
; CHECK-NEXT: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
196+
; CHECK-NOT: {{.*}}:
197+
; CHECK: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
197198
; CHECK: define internal fastcc void @f.destroy(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
198199
; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
199200

llvm/test/Transforms/InstCombine/freeze.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,9 +453,9 @@ define i32 @freeze_callbr_use_after_phi(i1 %c) {
453453
; CHECK-NEXT: to label [[CALLBR_CONT:%.*]] []
454454
; CHECK: callbr.cont:
455455
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[X]], [[ENTRY:%.*]] ], [ 0, [[CALLBR_CONT]] ]
456+
; CHECK-NEXT: call void @use_i32(i32 [[X]])
456457
; CHECK-NEXT: [[FR:%.*]] = freeze i32 [[X]]
457458
; CHECK-NEXT: call void @use_i32(i32 [[FR]])
458-
; CHECK-NEXT: call void @use_i32(i32 [[FR]])
459459
; CHECK-NEXT: call void @use_i32(i32 [[PHI]])
460460
; CHECK-NEXT: br label [[CALLBR_CONT]]
461461
;

llvm/test/Transforms/Reassociate/callbr.ll

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ define i32 @test(i1 %b) {
66
; CHECK-NEXT: [[RES:%.*]] = callbr i32 asm "", "=r,!i"()
77
; CHECK-NEXT: to label [[NORMAL:%.*]] [label %abnormal]
88
; CHECK: normal:
9-
; CHECK-NEXT: [[FACTOR:%.*]] = mul i32 [[RES]], -2
10-
; CHECK-NEXT: [[SUB2:%.*]] = add i32 [[FACTOR]], 5
9+
; CHECK-NEXT: [[RES_NEG:%.*]] = sub i32 0, [[RES]]
10+
; CHECK-NEXT: [[SUB1:%.*]] = add i32 [[RES_NEG]], 5
11+
; CHECK-NEXT: [[RES_NEG1:%.*]] = sub i32 0, [[RES]]
12+
; CHECK-NEXT: [[SUB2:%.*]] = add i32 [[SUB1]], [[RES_NEG1]]
1113
; CHECK-NEXT: ret i32 [[SUB2]]
1214
; CHECK: abnormal:
1315
; CHECK-NEXT: ret i32 0

llvm/test/Verifier/callbr.ll

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ define void @callbr_without_label_constraint() {
5656
ret void
5757
}
5858

59-
;; Ensure you cannot use the return value of a callbr in indirect targets.
60-
; CHECK: Instruction does not dominate all uses!
61-
; CHECK-NEXT: #test4
59+
;; Ensure you can use the return value of a callbr in indirect targets.
60+
;; No issue!
6261
define i32 @test4(i1 %var) {
6362
entry:
6463
%ret = callbr i32 asm sideeffect "#test4", "=r,!i"() to label %normal [label %abnormal]

llvm/test/Verifier/dominates.ll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,12 @@ next:
6969
; CHECK-NEXT: %x = phi i32 [ %y, %entry ]
7070
}
7171

72+
;; No issue!
7273
define i32 @f6(i32 %x) {
7374
bb0:
7475
%y1 = callbr i32 asm "", "=r,!i"() to label %bb1 [label %bb2]
7576
bb1:
7677
ret i32 0
7778
bb2:
7879
ret i32 %y1
79-
; CHECK: Instruction does not dominate all uses!
80-
; CHECK-NEXT: %y1 = callbr i32 asm "", "=r,!i"()
81-
; CHECK-NEXT: to label %bb1 [label %bb2]
82-
; CHECK-NEXT: ret i32 %y1
8380
}

llvm/unittests/IR/DominatorTreeTest.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,3 +1100,46 @@ TEST(DominatorTree, ValueDomination) {
11001100
EXPECT_TRUE(DT->dominates(C, U));
11011101
});
11021102
}
1103+
TEST(DominatorTree, CallBrDomination) {
1104+
StringRef ModuleString = R"(
1105+
define void @x() {
1106+
%y = alloca i32
1107+
%w = callbr i32 asm "", "=r,!i"()
1108+
to label %asm.fallthrough [label %z]
1109+
1110+
asm.fallthrough:
1111+
br label %cleanup
1112+
1113+
z:
1114+
store i32 %w, ptr %y
1115+
br label %cleanup
1116+
1117+
cleanup:
1118+
ret void
1119+
})";
1120+
1121+
LLVMContext Context;
1122+
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
1123+
1124+
runWithDomTree(
1125+
*M, "x", [&](Function &F, DominatorTree *DT, PostDominatorTree *PDT) {
1126+
Function::iterator FI = F.begin();
1127+
1128+
BasicBlock *Entry = &*FI++;
1129+
BasicBlock *ASMFallthrough = &*FI++;
1130+
BasicBlock *Z = &*FI++;
1131+
1132+
EXPECT_TRUE(DT->dominates(Entry, ASMFallthrough));
1133+
EXPECT_TRUE(DT->dominates(Entry, Z));
1134+
1135+
BasicBlock::iterator BBI = Entry->begin();
1136+
++BBI;
1137+
Instruction &I = *BBI;
1138+
EXPECT_TRUE(isa<CallBrInst>(I));
1139+
EXPECT_TRUE(isa<Value>(I));
1140+
for (const User *U : I.users()) {
1141+
EXPECT_TRUE(isa<Instruction>(U));
1142+
EXPECT_TRUE(DT->dominates(cast<Value>(&I), cast<Instruction>(U)));
1143+
}
1144+
});
1145+
}

0 commit comments

Comments
 (0)