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,9 +326,17 @@ 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. -unsigned changeToUnreachable(Instruction *I, bool UseLLVMTrap, +/// The flag \p PreferToUseLLVMTrap is used as the default context choice for +/// `llvm::shouldUseTrapBeforeUnreachable` to determine if the unreachable +/// should be preceded by an `llvm.trap` or not. +unsigned changeToUnreachable(Instruction *I, bool PreferToUseLLVMTrap, bool PreserveLCSSA = false, DomTreeUpdater *DTU = nullptr, MemorySSAUpdater *MSSAU = 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, /* PreferToUseLLVMTrap */ false); } else { - changeToUnreachable(&I, /*UseLLVMTrap=*/false); + changeToUnreachable(&I, /* PreferToUseLLVMTrap */ 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, /* PreferToUseLLVMTrap */ 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, /* PreferToUseLLVMTrap */ 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, /* PreferToUseLLVMTrap */ 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, /* PreferToUseLLVMTrap */ 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(), + /* PreferToUseLLVMTrap */ 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 @@ -532,7 +532,7 @@ } if (!Solver.isBlockExecutable(&F.front())) NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHI(), - /*UseLLVMTrap=*/false, + /* PreferToUseLLVMTrap */ false, /*PreserveLCSSA=*/false, &DTU); for (BasicBlock &BB : F) 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, + /* PreferToUseLLVMTrap */ 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. // @@ -2096,7 +2128,7 @@ return {NumDeadInst, NumDeadDbgInst}; } -unsigned llvm::changeToUnreachable(Instruction *I, bool UseLLVMTrap, +unsigned llvm::changeToUnreachable(Instruction *I, bool PreferToUseLLVMTrap, bool PreserveLCSSA, DomTreeUpdater *DTU, MemorySSAUpdater *MSSAU) { BasicBlock *BB = I->getParent(); @@ -2106,6 +2138,8 @@ SmallSet UniqueSuccessors; + bool UseLLVMTrap = shouldUseTrapBeforeUnreachable(PreferToUseLLVMTrap); + // Loop over all of the successors, removing BB's entry from any PHI // nodes. for (BasicBlock *Successor : successors(BB)) { @@ -2257,7 +2291,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, /* PreferToUseLLVMTrap */ false, false, + DTU); Changed = true; break; } @@ -2273,8 +2308,9 @@ // still be useful for widening. if (match(CI->getArgOperand(0), m_Zero())) if (!isa(CI->getNextNode())) { - changeToUnreachable(CI->getNextNode(), /*UseLLVMTrap=*/false, - false, DTU); + changeToUnreachable(CI->getNextNode(), + /* PreferToUseLLVMTrap */ false, false, + DTU); Changed = true; break; } @@ -2282,7 +2318,7 @@ } else if ((isa(Callee) && !NullPointerIsDefined(CI->getFunction())) || isa(Callee)) { - changeToUnreachable(CI, /*UseLLVMTrap=*/false, false, DTU); + changeToUnreachable(CI, /* PreferToUseLLVMTrap */ false, false, DTU); Changed = true; break; } @@ -2292,7 +2328,8 @@ // 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(), + /* PreferToUseLLVMTrap */ false, false, DTU); Changed = true; } break; @@ -2311,7 +2348,7 @@ (isa(Ptr) && !NullPointerIsDefined(SI->getFunction(), SI->getPointerAddressSpace()))) { - changeToUnreachable(SI, true, false, DTU); + changeToUnreachable(SI, /* PreferToUseLLVMTrap */ true, false, DTU); Changed = true; break; } @@ -2325,7 +2362,7 @@ if ((isa(Callee) && !NullPointerIsDefined(BB->getParent())) || isa(Callee)) { - changeToUnreachable(II, true, false, DTU); + changeToUnreachable(II, /* PreferToUseLLVMTrap */ 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,7 @@ // Zap the dead pred's terminator and replace it with unreachable. Instruction *TI = P->getTerminator(); - changeToUnreachable(TI, /*UseLLVMTrap=*/false, PreserveLCSSA, + changeToUnreachable(TI, /* PreferToUseLLVMTrap */ 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,8 @@ // When completely unrolling, the last latch becomes unreachable. if (!LatchIsExiting && CompletelyUnroll) - changeToUnreachable(Latches.back()->getTerminator(), /* UseTrap */ false, - PreserveLCSSA, &DTU); + changeToUnreachable(Latches.back()->getTerminator(), + /* PreferToUseLLVMTrap */ 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(), + /* PreferToUseLLVMTrap */ 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 diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp --- a/llvm/unittests/Transforms/Utils/LocalTest.cpp +++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp @@ -623,7 +623,7 @@ ASSERT_TRUE(isa(&A)); // One instruction should be affected. - EXPECT_EQ(changeToUnreachable(&A, /*UseLLVMTrap*/false), 1U); + EXPECT_EQ(changeToUnreachable(&A, /*PreferToUseLLVMTrap*/ false), 1U); Instruction &B = BB.front();