diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -326,8 +326,15 @@ std::pair removeAllNonTerminatorAndEHPadInstructions(BasicBlock *BB); +/// Return true if a new trap should be emitted before a new unreachable. The +/// \p DefaultContextChoice argument indicates if the context would by default +/// do so. The user can overwrite this choice via the command line though. +bool shouldUseTrapBeforeUnreachable(bool DefaultContextChoice); + /// Insert an unreachable instruction before the specified /// instruction, making it and the rest of the code in the block dead. +/// It is recommended to determe the \p UseLLVMTrap operand through a call to +/// `llvm::shouldUseTrapBeforeUnreachable`. unsigned changeToUnreachable(Instruction *I, bool UseLLVMTrap, bool PreserveLCSSA = false, DomTreeUpdater *DTU = nullptr, diff --git a/llvm/lib/CodeGen/WinEHPrepare.cpp b/llvm/lib/CodeGen/WinEHPrepare.cpp --- a/llvm/lib/CodeGen/WinEHPrepare.cpp +++ b/llvm/lib/CodeGen/WinEHPrepare.cpp @@ -984,9 +984,9 @@ BasicBlock::iterator CallI = std::prev(BB->getTerminator()->getIterator()); auto *CI = cast(&*CallI); - changeToUnreachable(CI, /*UseLLVMTrap=*/false); + changeToUnreachable(CI, shouldUseTrapBeforeUnreachable(false)); } else { - changeToUnreachable(&I, /*UseLLVMTrap=*/false); + changeToUnreachable(&I, shouldUseTrapBeforeUnreachable(false)); } // There are no more instructions in the block (except for unreachable), @@ -1007,7 +1007,7 @@ IsUnreachableCleanupret = CRI->getCleanupPad() != CleanupPad; if (IsUnreachableRet || IsUnreachableCatchret || IsUnreachableCleanupret) { - changeToUnreachable(TI, /*UseLLVMTrap=*/false); + changeToUnreachable(TI, shouldUseTrapBeforeUnreachable(false)); } else if (isa(TI)) { if (Personality == EHPersonality::MSVC_CXX && CleanupPad) { // Invokes within a cleanuppad for the MSVC++ personality never diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp --- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp @@ -955,7 +955,7 @@ case coro::ABI::RetconOnce: // Remove old returns. for (ReturnInst *Return : Returns) - changeToUnreachable(Return, /*UseLLVMTrap=*/false); + changeToUnreachable(Return, shouldUseTrapBeforeUnreachable(false)); break; // With multi-suspend continuations, we'll already have eliminated the diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp --- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp +++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp @@ -361,7 +361,7 @@ // Replace all coro.ends with unreachable instruction. for (AnyCoroEndInst *CE : CoroEnds) - changeToUnreachable(CE, /*UseLLVMTrap=*/false); + changeToUnreachable(CE, shouldUseTrapBeforeUnreachable(false)); return; } diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -1658,7 +1658,7 @@ if (!isRunOn(*I->getFunction())) continue; CGModifiedFunctions.insert(I->getFunction()); - changeToUnreachable(I, /* UseLLVMTrap */ false); + changeToUnreachable(I, shouldUseTrapBeforeUnreachable(false)); } for (auto &V : ToBeDeletedInsts) { diff --git a/llvm/lib/Transforms/IPO/PruneEH.cpp b/llvm/lib/Transforms/IPO/PruneEH.cpp --- a/llvm/lib/Transforms/IPO/PruneEH.cpp +++ b/llvm/lib/Transforms/IPO/PruneEH.cpp @@ -251,7 +251,8 @@ if (TokenInst) { if (!TokenInst->isTerminator()) - changeToUnreachable(TokenInst->getNextNode(), /*UseLLVMTrap=*/false); + changeToUnreachable(TokenInst->getNextNode(), + shouldUseTrapBeforeUnreachable(false)); } else { // Get the list of successors of this block. std::vector Succs(succ_begin(BB), succ_end(BB)); diff --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp --- a/llvm/lib/Transforms/Scalar/SCCP.cpp +++ b/llvm/lib/Transforms/Scalar/SCCP.cpp @@ -531,9 +531,9 @@ /*PreserveLCSSA=*/false, &DTU); } if (!Solver.isBlockExecutable(&F.front())) - NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHI(), - /*UseLLVMTrap=*/false, - /*PreserveLCSSA=*/false, &DTU); + NumInstRemoved += changeToUnreachable( + F.front().getFirstNonPHI(), shouldUseTrapBeforeUnreachable(false), + /*PreserveLCSSA=*/false, &DTU); for (BasicBlock &BB : F) MadeChanges |= removeNonFeasibleEdges(Solver, &BB, DTU); diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -2322,7 +2322,8 @@ // As such, we replace the cleanupret with unreachable. if (auto *CleanupRet = dyn_cast(BB->getTerminator())) if (CleanupRet->unwindsToCaller() && EHPadForCallUnwindsLocally) - changeToUnreachable(CleanupRet, /*UseLLVMTrap=*/false); + changeToUnreachable(CleanupRet, + shouldUseTrapBeforeUnreachable(false)); Instruction *I = BB->getFirstNonPHI(); if (!I->isEHPad()) diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -110,10 +110,42 @@ "When the basic block contains not more than this number of PHI nodes, " "perform a (faster!) exhaustive search instead of set-driven one.")); +enum class TrapBeforeUnreachableChoiceTy { + VARIABLE, + NEVER, + ALWAYS, +}; + +static cl::opt TrapBeforeUnreachableChoice( + "trap-before-unreachable", + cl::init(TrapBeforeUnreachableChoiceTy::VARIABLE), cl::Hidden, + cl::desc("Flag to indicate if a trap instruction should be placed before a " + "newly created unreachable or not."), + cl::values( + clEnumValN( + TrapBeforeUnreachableChoiceTy::VARIABLE, "variable", + "insert traps depending on the reason an unreachable was placed"), + clEnumValN(TrapBeforeUnreachableChoiceTy::NEVER, "never", + "never insert a trap before a new unreachable"), + clEnumValN(TrapBeforeUnreachableChoiceTy::ALWAYS, "always", + "awlays insert a trap before a new unreachable"))); + // Max recursion depth for collectBitParts used when detecting bswap and // bitreverse idioms. static const unsigned BitPartRecursionMaxDepth = 48; +bool llvm::shouldUseTrapBeforeUnreachable(bool DefaultContextChoice) { + switch (TrapBeforeUnreachableChoice) { + case TrapBeforeUnreachableChoiceTy::VARIABLE: + return DefaultContextChoice; + case TrapBeforeUnreachableChoiceTy::NEVER: + return false; + case TrapBeforeUnreachableChoiceTy::ALWAYS: + return true; + } + llvm_unreachable("Unknown trap-before-unreachable choice!"); +} + //===----------------------------------------------------------------------===// // Local constant propagation. // @@ -2257,7 +2289,8 @@ if (IntrinsicID == Intrinsic::assume) { if (match(CI->getArgOperand(0), m_CombineOr(m_Zero(), m_Undef()))) { // Don't insert a call to llvm.trap right before the unreachable. - changeToUnreachable(CI, false, false, DTU); + changeToUnreachable(CI, shouldUseTrapBeforeUnreachable(false), + false, DTU); Changed = true; break; } @@ -2273,7 +2306,8 @@ // still be useful for widening. if (match(CI->getArgOperand(0), m_Zero())) if (!isa(CI->getNextNode())) { - changeToUnreachable(CI->getNextNode(), /*UseLLVMTrap=*/false, + changeToUnreachable(CI->getNextNode(), + shouldUseTrapBeforeUnreachable(false), false, DTU); Changed = true; break; @@ -2282,7 +2316,8 @@ } else if ((isa(Callee) && !NullPointerIsDefined(CI->getFunction())) || isa(Callee)) { - changeToUnreachable(CI, /*UseLLVMTrap=*/false, false, DTU); + changeToUnreachable(CI, shouldUseTrapBeforeUnreachable(false), false, + DTU); Changed = true; break; } @@ -2292,7 +2327,9 @@ // though. if (!isa(CI->getNextNode())) { // Don't insert a call to llvm.trap right before the unreachable. - changeToUnreachable(CI->getNextNode(), false, false, DTU); + changeToUnreachable(CI->getNextNode(), + shouldUseTrapBeforeUnreachable(false), false, + DTU); Changed = true; } break; @@ -2311,7 +2348,8 @@ (isa(Ptr) && !NullPointerIsDefined(SI->getFunction(), SI->getPointerAddressSpace()))) { - changeToUnreachable(SI, true, false, DTU); + changeToUnreachable(SI, shouldUseTrapBeforeUnreachable(true), false, + DTU); Changed = true; break; } @@ -2325,7 +2363,8 @@ if ((isa(Callee) && !NullPointerIsDefined(BB->getParent())) || isa(Callee)) { - changeToUnreachable(II, true, false, DTU); + changeToUnreachable(II, shouldUseTrapBeforeUnreachable(true), false, + DTU); Changed = true; } else if (II->doesNotThrow() && canSimplifyInvokeNoUnwind(&F)) { if (II->use_empty() && II->onlyReadsMemory()) { diff --git a/llvm/lib/Transforms/Utils/LoopSimplify.cpp b/llvm/lib/Transforms/Utils/LoopSimplify.cpp --- a/llvm/lib/Transforms/Utils/LoopSimplify.cpp +++ b/llvm/lib/Transforms/Utils/LoopSimplify.cpp @@ -513,7 +513,8 @@ // Zap the dead pred's terminator and replace it with unreachable. Instruction *TI = P->getTerminator(); - changeToUnreachable(TI, /*UseLLVMTrap=*/false, PreserveLCSSA, + changeToUnreachable(TI, shouldUseTrapBeforeUnreachable(false), + PreserveLCSSA, /*DTU=*/nullptr, MSSAU); Changed = true; } diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp --- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp @@ -739,8 +739,9 @@ // When completely unrolling, the last latch becomes unreachable. if (!LatchIsExiting && CompletelyUnroll) - changeToUnreachable(Latches.back()->getTerminator(), /* UseTrap */ false, - PreserveLCSSA, &DTU); + changeToUnreachable(Latches.back()->getTerminator(), + shouldUseTrapBeforeUnreachable(false), PreserveLCSSA, + &DTU); // Merge adjacent basic blocks, if possible. for (BasicBlock *Latch : Latches) { diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp --- a/llvm/lib/Transforms/Utils/LoopUtils.cpp +++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp @@ -723,8 +723,9 @@ auto *BackedgeBB = SplitEdge(Latch, Header, &DT, &LI, MSSAU.get()); DomTreeUpdater DTU(&DT, DomTreeUpdater::UpdateStrategy::Eager); - (void)changeToUnreachable(BackedgeBB->getTerminator(), /*UseTrap*/false, - /*PreserveLCSSA*/true, &DTU, MSSAU.get()); + (void)changeToUnreachable(BackedgeBB->getTerminator(), + shouldUseTrapBeforeUnreachable(false), + /*PreserveLCSSA*/ true, &DTU, MSSAU.get()); // Erase (and destroy) this loop instance. Handles relinking sub-loops // and blocks within the loop as needed. diff --git a/llvm/test/Transforms/SimplifyCFG/assume.ll b/llvm/test/Transforms/SimplifyCFG/assume.ll --- a/llvm/test/Transforms/SimplifyCFG/assume.ll +++ b/llvm/test/Transforms/SimplifyCFG/assume.ll @@ -1,21 +1,31 @@ -; RUN: opt -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck %s +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -S -simplifycfg -simplifycfg-require-and-preserve-domtree=1 < %s | FileCheck %s --check-prefix=CTXTR +; RUN: opt -S -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -trap-before-unreachable=always < %s | FileCheck %s --check-prefix=TRAPS define void @test1() { - call void @llvm.assume(i1 0) - ret void +; CTXTR-LABEL: @test1( +; CTXTR-NEXT: unreachable +; +; TRAPS-LABEL: @test1( +; TRAPS-NEXT: call void @llvm.trap() +; TRAPS-NEXT: unreachable +; + call void @llvm.assume(i1 0) + ret void -; CHECK-LABEL: @test1 -; CHECK-NOT: llvm.assume -; CHECK: unreachable } define void @test2() { - call void @llvm.assume(i1 undef) - ret void +; CTXTR-LABEL: @test2( +; CTXTR-NEXT: unreachable +; +; TRAPS-LABEL: @test2( +; TRAPS-NEXT: call void @llvm.trap() +; TRAPS-NEXT: unreachable +; + call void @llvm.assume(i1 undef) + ret void -; CHECK-LABEL: @test2 -; CHECK-NOT: llvm.assume -; CHECK: unreachable } declare void @llvm.assume(i1) nounwind diff --git a/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll b/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll --- a/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll +++ b/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll @@ -1,5 +1,7 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py -; RUN: opt < %s -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s +; RUN: opt < %s -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s --check-prefixes=CHECK,CTXTR +; RUN: opt < %s -passes=simplifycfg -trap-before-unreachable=always -S | FileCheck %s --check-prefixes=CHECK,TRAPS +; RUN: opt < %s -passes=simplifycfg -trap-before-unreachable=never -S | FileCheck %s --check-prefixes=CHECK,NOTRP ; PR2967 target datalayout = @@ -50,10 +52,19 @@ ; rdar://7958343 define void @test2() nounwind { -; CHECK-LABEL: @test2( -; CHECK-NEXT: entry: -; CHECK-NEXT: call void @llvm.trap() -; CHECK-NEXT: unreachable +; CTXTR-LABEL: @test2( +; CTXTR-NEXT: entry: +; CTXTR-NEXT: call void @llvm.trap() +; CTXTR-NEXT: unreachable +; +; TRAPS-LABEL: @test2( +; TRAPS-NEXT: entry: +; TRAPS-NEXT: call void @llvm.trap() +; TRAPS-NEXT: unreachable +; +; NOTRP-LABEL: @test2( +; NOTRP-NEXT: entry: +; NOTRP-NEXT: unreachable ; entry: store i32 4,i32* null