[llvm] fc150ce - [SimplifyCFG] simplifyUnreachable(): erase instructions iff they are guaranteed to transfer execution to unreachable
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 3 00:46:24 PDT 2021
Author: Roman Lebedev
Date: 2021-07-03T10:45:44+03:00
New Revision: fc150cecd7484e47328fc78d4a3d597e2791b0fe
URL: https://github.com/llvm/llvm-project/commit/fc150cecd7484e47328fc78d4a3d597e2791b0fe
DIFF: https://github.com/llvm/llvm-project/commit/fc150cecd7484e47328fc78d4a3d597e2791b0fe.diff
LOG: [SimplifyCFG] simplifyUnreachable(): erase instructions iff they are guaranteed to transfer execution to unreachable
This replaces the current ad-hoc implementation,
by syncing the code from InstCombine's implementation in `InstCombinerImpl::visitUnreachableInst()`,
with one exception that here in SimplifyCFG we are allowed to remove EH instructions.
Effectively, this now allows SimplifyCFG to remove calls (iff they won't throw and will return),
arithmetic/logic operations, etc.
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D105374
Added:
Modified:
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
llvm/test/Transforms/SimplifyCFG/simplifyUnreachable-degenerate-conditional-branch-with-matching-destinations.ll
llvm/test/Transforms/SimplifyCFG/switch-range-to-icmp.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index e877fb1155f34..f5e320e94bdb0 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -26,6 +26,7 @@
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/AssumeBundleQueries.h"
#include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/EHPersonalities.h"
#include "llvm/Analysis/GuardUtils.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/Loads.h"
@@ -5273,6 +5274,18 @@ bool llvm::isGuaranteedToTransferExecutionToSuccessor(const Instruction *I) {
if (isa<UnreachableInst>(I))
return false;
+ if (isa<CatchPadInst>(I)) {
+ switch (classifyEHPersonality(I->getFunction()->getPersonalityFn())) {
+ default:
+ // A catchpad may invoke exception object constructors and such, which
+ // in some languages can be arbitrary code, so be conservative by default.
+ return false;
+ case EHPersonality::CoreCLR:
+ // For CoreCLR, it just involves a type test.
+ return true;
+ }
+ }
+
// An instruction that returns without throwing must transfer control flow
// to a successor.
return !I->mayThrow() && I->willReturn();
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 393180306ee8c..e00bcf8826d0d 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2870,6 +2870,7 @@ Instruction *InstCombinerImpl::visitReturnInst(ReturnInst &RI) {
return nullptr;
}
+// WARNING: keep in sync with SimplifyCFGOpt::simplifyUnreachable()!
Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) {
// Try to remove the previous instruction if it must lead to unreachable.
// This includes instructions like stores and "llvm.assume" that may not get
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index b8f6bf930fc93..f08ab18b15b20 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -4655,6 +4655,7 @@ bool SimplifyCFGOpt::simplifyReturn(ReturnInst *RI, IRBuilder<> &Builder) {
return false;
}
+// WARNING: keep in sync with InstCombinerImpl::visitUnreachableInst()!
bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
BasicBlock *BB = UI->getParent();
@@ -4665,39 +4666,24 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
while (UI->getIterator() != BB->begin()) {
BasicBlock::iterator BBI = UI->getIterator();
--BBI;
- // Do not delete instructions that can have side effects which might cause
- // the unreachable to not be reachable; specifically, calls and volatile
- // operations may have this effect.
- if (isa<CallInst>(BBI) && !isa<DbgInfoIntrinsic>(BBI))
- break;
- if (BBI->mayHaveSideEffects()) {
- if (auto *SI = dyn_cast<StoreInst>(BBI)) {
- // Temporarily disable removal of volatile stores preceding unreachable,
- // pending a potential LangRef change permitting volatile stores to
- // trap.
- // TODO: Either remove this code, or properly integrate the check into
- // isGuaranteedToTransferExecutionToSuccessor().
- if (SI->isVolatile())
- break;
- } else if (isa<CatchPadInst>(BBI)) {
- // A catchpad may invoke exception object constructors and such, which
- // in some languages can be arbitrary code, so be conservative by
- // default.
- // For CoreCLR, it just involves a type test, so can be removed.
- if (classifyEHPersonality(BB->getParent()->getPersonalityFn()) !=
- EHPersonality::CoreCLR)
- break;
- } else if (!isa<LoadInst>(BBI) && !isa<AtomicRMWInst>(BBI) &&
- !isa<AtomicCmpXchgInst>(BBI) && !isa<FenceInst>(BBI) &&
- !isa<VAArgInst>(BBI) && !isa<LandingPadInst>(BBI)) {
- break;
- }
- // Note that deleting LandingPad's here is in fact okay, although it
- // involves a bit of subtle reasoning. If this inst is a LandingPad,
- // all the predecessors of this block will be the unwind edges of Invokes,
- // and we can therefore guarantee this block will be erased.
- }
+ if (!isGuaranteedToTransferExecutionToSuccessor(&*BBI))
+ break; // Can not drop any more instructions. We're done here.
+ // Otherwise, this instruction can be freely erased,
+ // even if it is not side-effect free.
+
+ // Temporarily disable removal of volatile stores preceding unreachable,
+ // pending a potential LangRef change permitting volatile stores to trap.
+ // TODO: Either remove this code, or properly integrate the check into
+ // isGuaranteedToTransferExecutionToSuccessor().
+ if (auto *SI = dyn_cast<StoreInst>(&*BBI))
+ if (SI->isVolatile())
+ break; // Can not drop this instruction. We're done here.
+
+ // Note that deleting EH's here is in fact okay, although it involves a bit
+ // of subtle reasoning. If this inst is an EH, all the predecessors of this
+ // block will be the unwind edges of Invoke/CatchSwitch/CleanupReturn,
+ // and we can therefore guarantee this block will be erased.
// Delete this instruction (any uses are guaranteed to be dead)
BBI->replaceAllUsesWith(PoisonValue::get(BBI->getType()));
diff --git a/llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll b/llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
index 015017a5833d2..58f5c4d94e25d 100644
--- a/llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
+++ b/llvm/test/CodeGen/PowerPC/2007-11-16-landingpad-split.ll
@@ -74,9 +74,11 @@ define void @Bork(i64 %range.0.0, i64 %range.0.1, i64 %size) personality i32 (..
; CHECK-NEXT: .LBB0_6: # %unwind.loopexit
; CHECK-NEXT: .Ltmp5:
; CHECK-NEXT: .LBB0_7: # %unwind
-; CHECK-NEXT: ld 3, 0(1)
+; CHECK-NEXT: ld 4, 0(1)
; CHECK-NEXT: mr 1, 27
-; CHECK-NEXT: std 3, 0(1)
+; CHECK-NEXT: std 4, 0(1)
+; CHECK-NEXT: bl _Unwind_Resume
+; CHECK-NEXT: nop
entry:
%effectiveRange = alloca %struct.Range, align 8 ; <%struct.Range*> [#uses=2]
%tmp4 = call i8* @llvm.stacksave() ; <i8*> [#uses=1]
@@ -91,7 +93,7 @@ bb30.preheader: ; preds = %entry
unwind: ; preds = %cond_true, %entry
%exn = landingpad {i8*, i32}
- catch i8* null
+ cleanup
call void @llvm.stackrestore(i8* %tmp4)
resume { i8*, i32 } %exn
diff --git a/llvm/test/Transforms/SimplifyCFG/simplifyUnreachable-degenerate-conditional-branch-with-matching-destinations.ll b/llvm/test/Transforms/SimplifyCFG/simplifyUnreachable-degenerate-conditional-branch-with-matching-destinations.ll
index 66218a10521b0..77ffa9606fa89 100644
--- a/llvm/test/Transforms/SimplifyCFG/simplifyUnreachable-degenerate-conditional-branch-with-matching-destinations.ll
+++ b/llvm/test/Transforms/SimplifyCFG/simplifyUnreachable-degenerate-conditional-branch-with-matching-destinations.ll
@@ -8,9 +8,6 @@
define void @widget() {
; CHECK-LABEL: @widget(
; CHECK-NEXT: bb:
-; CHECK-NEXT: [[I:%.*]] = load i16, i16* @global, align 1
-; CHECK-NEXT: [[I13:%.*]] = icmp ne i16 [[I]], 0
-; CHECK-NEXT: call void @llvm.assume(i1 [[I13]])
; CHECK-NEXT: unreachable
;
bb:
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-range-to-icmp.ll b/llvm/test/Transforms/SimplifyCFG/switch-range-to-icmp.ll
index 89dafd75b27e7..56a6895f29b8f 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch-range-to-icmp.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch-range-to-icmp.ll
@@ -116,8 +116,6 @@ b:
define void @PR42737(i32* %a, i1 %c) {
; CHECK-LABEL: @PR42737(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[TMP0:%.*]] = xor i1 [[C:%.*]], true
-; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]])
; CHECK-NEXT: unreachable
;
entry:
More information about the llvm-commits
mailing list