[llvm] 4eafc9b - [IR] Treat callbr as special terminator (PR64215)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 25 00:20:26 PDT 2023
Author: Nikita Popov
Date: 2023-08-25T09:20:18+02:00
New Revision: 4eafc9b6ff4ae2bce82e9fdf0123b336825d931c
URL: https://github.com/llvm/llvm-project/commit/4eafc9b6ff4ae2bce82e9fdf0123b336825d931c
DIFF: https://github.com/llvm/llvm-project/commit/4eafc9b6ff4ae2bce82e9fdf0123b336825d931c.diff
LOG: [IR] Treat callbr as special terminator (PR64215)
isLegalToHoistInto() currently return true for callbr instructions.
That means that a callbr with one successor will be considered a
proper loop preheader, which may result in instructions that use
the callbr return value being hoisted past it.
Fix this by adding callbr to isExceptionTerminator (with a rename
to isSpecialTerminator), which also fixes similar assumptions in
other places.
Fixes https://github.com/llvm/llvm-project/issues/64215.
Differential Revision: https://reviews.llvm.org/D158609
Added:
llvm/test/Transforms/JumpThreading/callbr.ll
Modified:
llvm/include/llvm/IR/Instruction.h
llvm/lib/IR/BasicBlock.cpp
llvm/lib/Transforms/Coroutines/CoroElide.cpp
llvm/lib/Transforms/Scalar/GVN.cpp
llvm/lib/Transforms/Scalar/JumpThreading.cpp
llvm/lib/Transforms/Scalar/Sink.cpp
llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
llvm/lib/Transforms/Utils/SCCPSolver.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/LICM/callbr-crash.ll
Removed:
llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
################################################################################
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 5fd8b27447b77d..6ae66b0a7eaa4b 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -175,9 +175,7 @@ class Instruction : public User,
bool isShift() const { return isShift(getOpcode()); }
bool isCast() const { return isCast(getOpcode()); }
bool isFuncletPad() const { return isFuncletPad(getOpcode()); }
- bool isExceptionalTerminator() const {
- return isExceptionalTerminator(getOpcode());
- }
+ bool isSpecialTerminator() const { return isSpecialTerminator(getOpcode()); }
/// It checks if this instruction is the only user of at least one of
/// its operands.
@@ -235,14 +233,16 @@ class Instruction : public User,
return Opcode >= FuncletPadOpsBegin && Opcode < FuncletPadOpsEnd;
}
- /// Returns true if the Opcode is a terminator related to exception handling.
- static inline bool isExceptionalTerminator(unsigned Opcode) {
+ /// Returns true if the Opcode is a "special" terminator that does more than
+ /// branch to a successor (e.g. have a side effect or return a value).
+ static inline bool isSpecialTerminator(unsigned Opcode) {
switch (Opcode) {
case Instruction::CatchSwitch:
case Instruction::CatchRet:
case Instruction::CleanupRet:
case Instruction::Invoke:
case Instruction::Resume:
+ case Instruction::CallBr:
return true;
default:
return false;
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 14e1787c2b14b7..1081e03e0016f0 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -396,8 +396,9 @@ bool BasicBlock::isLegalToHoistInto() const {
// If the block has no successors, there can be no instructions to hoist.
assert(Term->getNumSuccessors() > 0);
- // Instructions should not be hoisted across exception handling boundaries.
- return !Term->isExceptionalTerminator();
+ // Instructions should not be hoisted across special terminators, which may
+ // have side effects or return values.
+ return !Term->isSpecialTerminator();
}
bool BasicBlock::isEntryBlock() const {
diff --git a/llvm/lib/Transforms/Coroutines/CoroElide.cpp b/llvm/lib/Transforms/Coroutines/CoroElide.cpp
index d0606c15f3d5b2..3310a0a7657f75 100644
--- a/llvm/lib/Transforms/Coroutines/CoroElide.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroElide.cpp
@@ -227,7 +227,7 @@ bool Lowerer::hasEscapePath(const CoroBeginInst *CB,
PotentiallyEscaped |= EscapingBBs.count(BB);
if (TIs.count(BB)) {
- if (!BB->getTerminator()->isExceptionalTerminator() || PotentiallyEscaped)
+ if (isa<ReturnInst>(BB->getTerminator()) || PotentiallyEscaped)
return true;
// If the function ends with the exceptional terminator, the memory used
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 8b9f89cfe755d6..635e56ec601229 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1368,7 +1368,7 @@ LoadInst *GVNPass::findLoadToHoistIntoPred(BasicBlock *Pred, BasicBlock *LoadBB,
LoadInst *Load) {
// For simplicity we handle a Pred has 2 successors only.
auto *Term = Pred->getTerminator();
- if (Term->getNumSuccessors() != 2 || Term->isExceptionalTerminator())
+ if (Term->getNumSuccessors() != 2 || Term->isSpecialTerminator())
return nullptr;
auto *SuccBB = Term->getSuccessor(0);
if (SuccBB == LoadBB)
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 24390f1b54f605..180e6b4c823880 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1899,7 +1899,7 @@ bool JumpThreadingPass::maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB) {
return false;
const Instruction *TI = SinglePred->getTerminator();
- if (TI->isExceptionalTerminator() || TI->getNumSuccessors() != 1 ||
+ if (TI->isSpecialTerminator() || TI->getNumSuccessors() != 1 ||
SinglePred == BB || hasAddressTakenAndUsed(BB))
return false;
diff --git a/llvm/lib/Transforms/Scalar/Sink.cpp b/llvm/lib/Transforms/Scalar/Sink.cpp
index 8b99f73b850b98..ed768deacd069b 100644
--- a/llvm/lib/Transforms/Scalar/Sink.cpp
+++ b/llvm/lib/Transforms/Scalar/Sink.cpp
@@ -67,9 +67,8 @@ static bool IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo,
assert(Inst && "Instruction to be sunk is null");
assert(SuccToSinkTo && "Candidate sink target is null");
- // It's never legal to sink an instruction into a block which terminates in an
- // EH-pad.
- if (SuccToSinkTo->getTerminator()->isExceptionalTerminator())
+ // It's never legal to sink an instruction into an EH-pad block.
+ if (SuccToSinkTo->isEHPad())
return false;
// If the block has multiple predecessors, this would introduce computation
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 6b956a193e6418..7edb4e91b935f5 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -194,7 +194,7 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
// Don't break unwinding instructions or terminators with other side-effects.
Instruction *PTI = PredBB->getTerminator();
- if (PTI->isExceptionalTerminator() || PTI->mayHaveSideEffects())
+ if (PTI->isSpecialTerminator() || PTI->mayHaveSideEffects())
return false;
// Can't merge if there are multiple distinct successors.
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 4821d344512b80..8a67fda7c98e78 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -1029,8 +1029,9 @@ void SCCPInstVisitor::getFeasibleSuccessors(Instruction &TI,
return;
}
- // Unwinding instructions successors are always executable.
- if (TI.isExceptionalTerminator()) {
+ // We cannot analyze special terminators, so consider all successors
+ // executable.
+ if (TI.isSpecialTerminator()) {
Succs.assign(TI.getNumSuccessors(), true);
return;
}
@@ -1098,13 +1099,6 @@ void SCCPInstVisitor::getFeasibleSuccessors(Instruction &TI,
return;
}
- // In case of callbr, we pessimistically assume that all successors are
- // feasible.
- if (isa<CallBrInst>(&TI)) {
- Succs.assign(TI.getNumSuccessors(), true);
- return;
- }
-
LLVM_DEBUG(dbgs() << "Unknown terminator instruction: " << TI << '\n');
llvm_unreachable("SCCP: Don't know how to handle this terminator!");
}
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index a0fab9d65ea583..7af366b47e9f56 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5676,7 +5676,7 @@ getCaseResults(SwitchInst *SI, ConstantInt *CaseVal, BasicBlock *CaseDest,
for (Instruction &I : CaseDest->instructionsWithoutDebug(false)) {
if (I.isTerminator()) {
// If the terminator is a simple branch, continue to the next block.
- if (I.getNumSuccessors() != 1 || I.isExceptionalTerminator())
+ if (I.getNumSuccessors() != 1 || I.isSpecialTerminator())
return false;
Pred = CaseDest;
CaseDest = I.getSuccessor(0);
diff --git a/llvm/test/Transforms/JumpThreading/pr46857-callbr.ll b/llvm/test/Transforms/JumpThreading/callbr.ll
similarity index 80%
rename from llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
rename to llvm/test/Transforms/JumpThreading/callbr.ll
index 1a1fe9a05d6ec1..ea1922f30ddfd9 100644
--- a/llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
+++ b/llvm/test/Transforms/JumpThreading/callbr.ll
@@ -47,3 +47,17 @@ bb10:
bb11:
ret i1 %i9
}
+
+define i32 @callbr_no_block_merge() {
+; CHECK-LABEL: @callbr_no_block_merge(
+; CHECK-NEXT: [[X:%.*]] = callbr i32 asm sideeffect "", "=r"()
+; CHECK-NEXT: to label [[BB:%.*]] []
+; CHECK: bb:
+; CHECK-NEXT: ret i32 [[X]]
+;
+ %x = callbr i32 asm sideeffect "", "=r"()
+ to label %bb []
+
+bb:
+ ret i32 %x
+}
diff --git a/llvm/test/Transforms/LICM/callbr-crash.ll b/llvm/test/Transforms/LICM/callbr-crash.ll
index cd7debf75aa5cd..60f7f6c7dd768d 100644
--- a/llvm/test/Transforms/LICM/callbr-crash.ll
+++ b/llvm/test/Transforms/LICM/callbr-crash.ll
@@ -35,3 +35,27 @@ for.end: ; preds = %cond.true.i, %for.c
%phi = phi i32 [ %asmresult1.i.i, %cond.true.i ], [ undef, %for.cond ]
ret i32 %phi
}
+
+declare void @use_i32(i32)
+
+define void @pr64215() {
+; CHECK-LABEL: @pr64215(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[X:%.*]] = callbr i32 asm sideeffect "", "=r"()
+; CHECK-NEXT: to label [[LOOP_PREHEADER:%.*]] []
+; CHECK: loop.preheader:
+; CHECK-NEXT: [[ADD:%.*]] = add i32 [[X]], 1
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: call void @use_i32(i32 [[ADD]])
+; CHECK-NEXT: br label [[LOOP]]
+;
+entry:
+ %x = callbr i32 asm sideeffect "", "=r"()
+ to label %loop []
+
+loop:
+ %add = add i32 %x, 1
+ call void @use_i32(i32 %add)
+ br label %loop
+}
More information about the llvm-commits
mailing list