[llvm] 45a291b - [Dominators] check indirect branches of callbr
Nick Desaulniers via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 16 18:03:53 PST 2023
Author: Nick Desaulniers
Date: 2023-02-16T17:58:33-08:00
New Revision: 45a291b5f609fc7edd8c526772e491d68b210dbe
URL: https://github.com/llvm/llvm-project/commit/45a291b5f609fc7edd8c526772e491d68b210dbe
DIFF: https://github.com/llvm/llvm-project/commit/45a291b5f609fc7edd8c526772e491d68b210dbe.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/IR/Dominators.h
llvm/lib/IR/Dominators.cpp
llvm/lib/IR/Instruction.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/test/Transforms/Coroutines/coro-debug.ll
llvm/test/Transforms/InstCombine/freeze.ll
llvm/test/Transforms/Reassociate/callbr.ll
llvm/test/Verifier/callbr.ll
llvm/test/Verifier/dominates.ll
llvm/unittests/IR/DominatorTreeTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Dominators.h b/llvm/include/llvm/IR/Dominators.h
index c2d080bc20047..6ceadbf30b895 100644
--- a/llvm/include/llvm/IR/Dominators.h
+++ b/llvm/include/llvm/IR/Dominators.h
@@ -191,7 +191,7 @@ class DominatorTree : public DominatorTreeBase<BasicBlock, false> {
/// * Non-instruction Defs dominate everything.
/// * Def does not dominate a use in Def itself (outside of degenerate cases
/// like unreachable code or trivial phi cycles).
- /// * Invoke/callbr Defs only dominate uses in their default destination.
+ /// * Invoke Defs only dominate uses in their default destination.
bool dominates(const Value *Def, const Use &U) const;
/// Return true if value Def dominates all possible uses inside instruction
/// User. Same comments as for the Use-based API apply.
diff --git a/llvm/lib/IR/Dominators.cpp b/llvm/lib/IR/Dominators.cpp
index 7c620c3a9331a..24cc9f46ff79f 100644
--- a/llvm/lib/IR/Dominators.cpp
+++ b/llvm/lib/IR/Dominators.cpp
@@ -194,13 +194,6 @@ bool DominatorTree::dominates(const Instruction *Def,
return dominates(E, UseBB);
}
- // Callbr results are similarly only usable in the default destination.
- if (const auto *CBI = dyn_cast<CallBrInst>(Def)) {
- BasicBlock *NormalDest = CBI->getDefaultDest();
- BasicBlockEdge E(DefBB, NormalDest);
- return dominates(E, UseBB);
- }
-
return dominates(DefBB, UseBB);
}
@@ -311,13 +304,6 @@ bool DominatorTree::dominates(const Value *DefV, const Use &U) const {
return dominates(E, U);
}
- // Callbr results are similarly only usable in the default destination.
- if (const auto *CBI = dyn_cast<CallBrInst>(Def)) {
- BasicBlock *NormalDest = CBI->getDefaultDest();
- BasicBlockEdge E(DefBB, NormalDest);
- return dominates(E, U);
- }
-
// If the def and use are in
diff erent blocks, do a simple CFG dominator
// tree query.
if (DefBB != UseBB)
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 9c88ca17ebde4..fc4e5d4433d1b 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -139,8 +139,9 @@ Instruction *Instruction::getInsertionPointAfterDef() {
InsertBB = II->getNormalDest();
InsertPt = InsertBB->getFirstInsertionPt();
} else if (auto *CB = dyn_cast<CallBrInst>(this)) {
- InsertBB = CB->getDefaultDest();
- InsertPt = InsertBB->getFirstInsertionPt();
+ // Def is available in multiple successors, there's no single dominating
+ // insertion point.
+ return nullptr;
} else {
assert(!isTerminator() && "Only invoke/callbr terminators return value");
InsertBB = getParent();
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index bc714e4a1bba7..f6ae6741a3171 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3477,13 +3477,17 @@ Instruction *InstCombinerImpl::visitCallBase(CallBase &Call) {
}
/// If the callee is a constexpr cast of a function, attempt to move the cast to
-/// the arguments of the call/callbr/invoke.
+/// the arguments of the call/invoke.
+/// CallBrInst is not supported.
bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
auto *Callee =
dyn_cast<Function>(Call.getCalledOperand()->stripPointerCasts());
if (!Callee)
return false;
+ assert(!isa<CallBrInst>(Call) &&
+ "CallBr's don't have a single point after a def to insert at");
+
// If this is a call to a thunk function, don't remove the cast. Thunks are
// used to transparently forward all incoming parameters and outgoing return
// values, so it's important to leave the cast in place.
@@ -3529,7 +3533,7 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
return false; // Attribute not compatible with transformed value.
}
- // If the callbase is an invoke/callbr instruction, and the return value is
+ // If the callbase is an invoke instruction, and the return value is
// used by a PHI node in a successor, we cannot change the return type of
// the call because there is no place to put the cast instruction (without
// breaking the critical edge). Bail out in this case.
@@ -3537,8 +3541,6 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
BasicBlock *PhisNotSupportedBlock = nullptr;
if (auto *II = dyn_cast<InvokeInst>(Caller))
PhisNotSupportedBlock = II->getNormalDest();
- if (auto *CB = dyn_cast<CallBrInst>(Caller))
- PhisNotSupportedBlock = CB->getDefaultDest();
if (PhisNotSupportedBlock)
for (User *U : Caller->users())
if (PHINode *PN = dyn_cast<PHINode>(U))
@@ -3722,9 +3724,6 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) {
if (InvokeInst *II = dyn_cast<InvokeInst>(Caller)) {
NewCall = Builder.CreateInvoke(Callee, II->getNormalDest(),
II->getUnwindDest(), Args, OpBundles);
- } else if (CallBrInst *CBI = dyn_cast<CallBrInst>(Caller)) {
- NewCall = Builder.CreateCallBr(Callee, CBI->getDefaultDest(),
- CBI->getIndirectDests(), Args, OpBundles);
} else {
NewCall = Builder.CreateCall(Callee, Args, OpBundles);
cast<CallInst>(NewCall)->setTailCallKind(
diff --git a/llvm/test/Transforms/Coroutines/coro-debug.ll b/llvm/test/Transforms/Coroutines/coro-debug.ll
index fc3ade55517fa..282969484be1a 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug.ll
@@ -193,7 +193,8 @@ attributes #7 = { noduplicate }
; CHECK: %[[CALLBR_RES:.+]] = callbr i32 asm
; CHECK-NEXT: to label %[[DEFAULT_DEST:.+]] [label
; CHECK: [[DEFAULT_DEST]]:
-; CHECK-NEXT: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
+; CHECK-NOT: {{.*}}:
+; CHECK: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
; CHECK: define internal fastcc void @f.destroy(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index 7bbc32f277398..d3274ef2d7596 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -453,9 +453,9 @@ define i32 @freeze_callbr_use_after_phi(i1 %c) {
; CHECK-NEXT: to label [[CALLBR_CONT:%.*]] []
; CHECK: callbr.cont:
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[X]], [[ENTRY:%.*]] ], [ 0, [[CALLBR_CONT]] ]
+; CHECK-NEXT: call void @use_i32(i32 [[X]])
; CHECK-NEXT: [[FR:%.*]] = freeze i32 [[X]]
; CHECK-NEXT: call void @use_i32(i32 [[FR]])
-; CHECK-NEXT: call void @use_i32(i32 [[FR]])
; CHECK-NEXT: call void @use_i32(i32 [[PHI]])
; CHECK-NEXT: br label [[CALLBR_CONT]]
;
diff --git a/llvm/test/Transforms/Reassociate/callbr.ll b/llvm/test/Transforms/Reassociate/callbr.ll
index 8b7be30b245d7..4b2323f93ddc8 100644
--- a/llvm/test/Transforms/Reassociate/callbr.ll
+++ b/llvm/test/Transforms/Reassociate/callbr.ll
@@ -6,8 +6,10 @@ define i32 @test(i1 %b) {
; CHECK-NEXT: [[RES:%.*]] = callbr i32 asm "", "=r,!i"()
; CHECK-NEXT: to label [[NORMAL:%.*]] [label %abnormal]
; CHECK: normal:
-; CHECK-NEXT: [[FACTOR:%.*]] = mul i32 [[RES]], -2
-; CHECK-NEXT: [[SUB2:%.*]] = add i32 [[FACTOR]], 5
+; CHECK-NEXT: [[RES_NEG:%.*]] = sub i32 0, [[RES]]
+; CHECK-NEXT: [[SUB1:%.*]] = add i32 [[RES_NEG]], 5
+; CHECK-NEXT: [[RES_NEG1:%.*]] = sub i32 0, [[RES]]
+; CHECK-NEXT: [[SUB2:%.*]] = add i32 [[SUB1]], [[RES_NEG1]]
; CHECK-NEXT: ret i32 [[SUB2]]
; CHECK: abnormal:
; CHECK-NEXT: ret i32 0
diff --git a/llvm/test/Verifier/callbr.ll b/llvm/test/Verifier/callbr.ll
index 2a347d20b9f28..a77b6fee037ab 100644
--- a/llvm/test/Verifier/callbr.ll
+++ b/llvm/test/Verifier/callbr.ll
@@ -56,9 +56,8 @@ define void @callbr_without_label_constraint() {
ret void
}
-;; Ensure you cannot use the return value of a callbr in indirect targets.
-; CHECK: Instruction does not dominate all uses!
-; CHECK-NEXT: #test4
+;; Ensure you can use the return value of a callbr in indirect targets.
+;; No issue!
define i32 @test4(i1 %var) {
entry:
%ret = callbr i32 asm sideeffect "#test4", "=r,!i"() to label %normal [label %abnormal]
diff --git a/llvm/test/Verifier/dominates.ll b/llvm/test/Verifier/dominates.ll
index d3697fd865943..9d40890238879 100644
--- a/llvm/test/Verifier/dominates.ll
+++ b/llvm/test/Verifier/dominates.ll
@@ -69,6 +69,7 @@ next:
; CHECK-NEXT: %x = phi i32 [ %y, %entry ]
}
+;; No issue!
define i32 @f6(i32 %x) {
bb0:
%y1 = callbr i32 asm "", "=r,!i"() to label %bb1 [label %bb2]
@@ -76,8 +77,4 @@ bb1:
ret i32 0
bb2:
ret i32 %y1
-; CHECK: Instruction does not dominate all uses!
-; CHECK-NEXT: %y1 = callbr i32 asm "", "=r,!i"()
-; CHECK-NEXT: to label %bb1 [label %bb2]
-; CHECK-NEXT: ret i32 %y1
}
diff --git a/llvm/unittests/IR/DominatorTreeTest.cpp b/llvm/unittests/IR/DominatorTreeTest.cpp
index 6260ce2a17ae4..44bde74ad350f 100644
--- a/llvm/unittests/IR/DominatorTreeTest.cpp
+++ b/llvm/unittests/IR/DominatorTreeTest.cpp
@@ -1100,3 +1100,46 @@ TEST(DominatorTree, ValueDomination) {
EXPECT_TRUE(DT->dominates(C, U));
});
}
+TEST(DominatorTree, CallBrDomination) {
+ StringRef ModuleString = R"(
+define void @x() {
+ %y = alloca i32
+ %w = callbr i32 asm "", "=r,!i"()
+ to label %asm.fallthrough [label %z]
+
+asm.fallthrough:
+ br label %cleanup
+
+z:
+ store i32 %w, ptr %y
+ br label %cleanup
+
+cleanup:
+ ret void
+})";
+
+ LLVMContext Context;
+ std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
+
+ runWithDomTree(
+ *M, "x", [&](Function &F, DominatorTree *DT, PostDominatorTree *PDT) {
+ Function::iterator FI = F.begin();
+
+ BasicBlock *Entry = &*FI++;
+ BasicBlock *ASMFallthrough = &*FI++;
+ BasicBlock *Z = &*FI++;
+
+ EXPECT_TRUE(DT->dominates(Entry, ASMFallthrough));
+ EXPECT_TRUE(DT->dominates(Entry, Z));
+
+ BasicBlock::iterator BBI = Entry->begin();
+ ++BBI;
+ Instruction &I = *BBI;
+ EXPECT_TRUE(isa<CallBrInst>(I));
+ EXPECT_TRUE(isa<Value>(I));
+ for (const User *U : I.users()) {
+ EXPECT_TRUE(isa<Instruction>(U));
+ EXPECT_TRUE(DT->dominates(cast<Value>(&I), cast<Instruction>(U)));
+ }
+ });
+}
More information about the llvm-commits
mailing list