[Local] Flag to decide if a trap should be placed before unreachable

[jdoerfert]

Summary

Before we used "custom heuristics" to determine if a newly placed
unreachable should be preceded by a trap (llvm.trap) or not. This was
undesirable for multiple reasons, mainly it causes traps where we might
really want to avoid them (GPU targets), and it didn't allow to insert
more traps with proper debug information for debugging. The new scheme
allows the user to overwrite the standard "context decision" and force
or disallow traps instead.

This patch should not change the behavior on it's own but just provide
more options. The plan is to update the default for OpenMP GPU targets.

[lebedev.ri]

I think TrapBeforeUnreachableChoice should be a TTI hook, overrided by GPU targets.
I think shouldUseTrapBeforeUnreachable() should be sunk into changeToUnreachable(), and UseLLVMTrap renamed into PreferToUseLLVMTrap, with an appropriate documentation opdate.

[jdoerfert]

In D106299#2888120, @lebedev.ri wrote:

I think TrapBeforeUnreachableChoice should be a TTI hook, overrided by GPU targets.

While I don't mind that in general, I figured "always" is a nice debugging option and should be exposed. Not sure what a nice way would be to get both things.

I think shouldUseTrapBeforeUnreachable() should be sunk into changeToUnreachable(), and UseLLVMTrap renamed into PreferToUseLLVMTrap, with an appropriate documentation opdate.

I can do that.

[lebedev.ri]

In D106299#2888142, @jdoerfert wrote:

In D106299#2888120, @lebedev.ri wrote:

I think TrapBeforeUnreachableChoice should be a TTI hook, overrided by GPU targets.

While I don't mind that in general, I figured "always" is a nice debugging option and should be exposed. Not sure what a nice way would be to get both things.

That's another reason why i'm not really in favor of this approach.
Even ignoring the subset of users that will want this for hardening reasons,
i'd really expect that always really means before each unreachable,
but clearly it won't appear before the already-unreachable terminators.

I think shouldUseTrapBeforeUnreachable() should be sunk into changeToUnreachable(), and UseLLVMTrap renamed into PreferToUseLLVMTrap, with an appropriate documentation opdate.

I can do that.

[jdoerfert]

Address comment and make it PreferToUseLLVMTrap

[JonChesterfield]

llvm/lib/Transforms/Utils/Local.cpp

131

s/awlays/always

[efriedma]

llvm/lib/Transforms/Utils/Local.cpp

2351

This (and the corresponding handling for calls, below) is the only place where we're passing PreferToUseLLVMTrap=true, right?

It looks like there was an attempt to remove the trap generation back in 2016, but it got reverted. Revert said it "it seems to break UBSan". (rGe14e7bc4b889dfaffb7180d176a03311df2d4ae6). Maybe worth attempting again; we really shouldn't need to insert a trap after non-volatile store to null.

(Note this has nothing to do with the backend converting an "unreachable" to a trap instruction. Some backends do this for various reasons. For example, on Windows, it's necessary to generate correct EH tables in some cases, I think.)

[jdoerfert]

llvm/lib/Transforms/Utils/Local.cpp

2351

This (and the corresponding handling for calls, below) is the only place where we're passing PreferToUseLLVMTrap=true, right?

It seems that way. I'm happy to remove the trap stuff and we wait and see if UBSan is unhappy or not.

[lebedev.ri]

llvm/lib/Transforms/Utils/Local.cpp

2351

SGTM

[jdoerfert]

Replaced by D106308

llvm/include/llvm/Transforms/Utils/Local.h

 /// Remove all instructions from a basic block other than its terminator
 /// and any present EH pad instructions. Returns a pair where the first element
 /// is the number of instructions (excluding debug info instrinsics) that have
 /// been removed, and the second element is the number of debug info intrinsics
 /// that have been removed.
 std::pair<unsigned, unsigned>
 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);
 
 /// Convert the CallInst to InvokeInst with the specified unwind edge basic
 /// block.  This also splits the basic block where CI is located, because
 /// InvokeInst is a terminator instruction.  Returns the newly split basic
 /// block.

llvm/lib/CodeGen/WinEHPrepare.cpp

for (BasicBlock *BB : BlocksInFunclet) {
         // This call site was not part of this funclet, remove it.
         if (isa<InvokeInst>(CB)) {
           // Remove the unwind edge if it was an invoke.
           removeUnwindEdge(BB);
           // Get a pointer to the new call.
           BasicBlock::iterator CallI =
               std::prev(BB->getTerminator()->getIterator());
           auto *CI = cast<CallInst>(&*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),
         // we are done.
         break;
       }
 
       Instruction *TI = BB->getTerminator();
       // CatchPadInst and CleanupPadInst can't transfer control to a ReturnInst.
       bool IsUnreachableRet = isa<ReturnInst>(TI) && FuncletPad;
       // The token consumed by a CatchReturnInst must match the funclet token.
       bool IsUnreachableCatchret = false;
       if (auto *CRI = dyn_cast<CatchReturnInst>(TI))
         IsUnreachableCatchret = CRI->getCatchPad() != CatchPad;
       // The token consumed by a CleanupReturnInst must match the funclet token.
       bool IsUnreachableCleanupret = false;
       if (auto *CRI = dyn_cast<CleanupReturnInst>(TI))
         IsUnreachableCleanupret = CRI->getCleanupPad() != CleanupPad;
       if (IsUnreachableRet || IsUnreachableCatchret ||
           IsUnreachableCleanupret) {
-        changeToUnreachable(TI, /*UseLLVMTrap=*/false);
+        changeToUnreachable(TI, /* PreferToUseLLVMTrap */ false);
       } else if (isa<InvokeInst>(TI)) {
         if (Personality == EHPersonality::MSVC_CXX && CleanupPad) {
           // Invokes within a cleanuppad for the MSVC++ personality never
           // transfer control to their unwind edge: the personality will
           // terminate the program.
           removeUnwindEdge(BB);
         }
       }

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

void CoroCloner::create() {
   // In these ABIs, the cloned functions always return 'void', and the
   // existing return sites are meaningless.  Note that for unique
   // continuations, this includes the returns associated with suspends;
   // this is fine because we can't suspend twice.
   case coro::ABI::Switch:
   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
   // original returns and inserted returns before all the suspend points,
   // so we want to leave any returns in place.
   case coro::ABI::Retcon:
     break;
   // Async lowering will insert musttail call functions at all suspend points

llvm/lib/Transforms/Coroutines/Coroutines.cpp

for (AnyCoroSuspendInst *CS : CoroSuspends) {
       CS->replaceAllUsesWith(UndefValue::get(CS->getType()));
       CS->eraseFromParent();
       if (auto *CoroSave = CS->getCoroSave())
         CoroSave->eraseFromParent();
     }
 
     // Replace all coro.ends with unreachable instruction.
     for (AnyCoroEndInst *CE : CoroEnds)
-      changeToUnreachable(CE, /*UseLLVMTrap=*/false);
+      changeToUnreachable(CE, /* PreferToUseLLVMTrap */ false);
 
     return;
   }
 
   auto Id = CoroBegin->getId();
   switch (auto IdIntrinsic = Id->getIntrinsicID()) {
   case Intrinsic::coro_id: {
     auto SwitchId = cast<CoroIdInst>(Id);

llvm/lib/Transforms/IPO/Attributor.cpp

for (Instruction *I : TerminatorsToFold) {
     CGModifiedFunctions.insert(I->getFunction());
     ConstantFoldTerminator(I->getParent());
   }
   for (auto &V : ToBeChangedToUnreachableInsts)
     if (Instruction *I = dyn_cast_or_null<Instruction>(V)) {
       if (!isRunOn(*I->getFunction()))
         continue;
       CGModifiedFunctions.insert(I->getFunction());
-      changeToUnreachable(I, /* UseLLVMTrap */ false);
+      changeToUnreachable(I, /* PreferToUseLLVMTrap */ false);
     }
 
   for (auto &V : ToBeDeletedInsts) {
     if (Instruction *I = dyn_cast_or_null<Instruction>(V)) {
       if (auto *CB = dyn_cast<CallBase>(I)) {
         if (!isRunOn(*I->getFunction()))
           continue;
         if (!isa<IntrinsicInst>(CB))

llvm/lib/Transforms/IPO/PruneEH.cpp

for (BasicBlock::iterator I = BB->end(), E = BB->begin(); I != E; ) {
     }
 
     if (!I->use_empty())
       I->replaceAllUsesWith(UndefValue::get(I->getType()));
   }
 
   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<BasicBlock *> Succs(succ_begin(BB), succ_end(BB));
 
     for (unsigned i = 0, e = Succs.size(); i != e; ++i)
       Succs[i]->removePredecessor(BB);
 
     BB->eraseFromParent();
   }
 }

llvm/lib/Transforms/Scalar/SCCP.cpp

for (Function &F : M) {
     // block is not part of BlocksToErase, so we have to handle it separately.
     for (BasicBlock *BB : BlocksToErase) {
       NumInstRemoved +=
           changeToUnreachable(BB->getFirstNonPHI(), /*UseLLVMTrap=*/false,
                               /*PreserveLCSSA=*/false, &DTU);
     }
     if (!Solver.isBlockExecutable(&F.front()))
       NumInstRemoved += changeToUnreachable(F.front().getFirstNonPHI(),
-                                            /*UseLLVMTrap=*/false,
+                                            /* PreferToUseLLVMTrap */ false,
                                             /*PreserveLCSSA=*/false, &DTU);
 
     for (BasicBlock &BB : F)
       MadeChanges |= removeNonFeasibleEdges(Solver, &BB, DTU);
 
     for (BasicBlock *DeadBB : BlocksToErase)
       DTU.deleteBB(DeadBB);
 

llvm/lib/Transforms/Utils/InlineFunction.cpp

for (Function::iterator BB = FirstNewBlock->getIterator(),
       }
 
       // It is problematic if the inlinee has a cleanupret which unwinds to
       // caller and we inline it into a call site which doesn't unwind but into
       // an EH pad that does.  Such an edge must be dynamically unreachable.
       // As such, we replace the cleanupret with unreachable.
       if (auto *CleanupRet = dyn_cast<CleanupReturnInst>(BB->getTerminator()))
         if (CleanupRet->unwindsToCaller() && EHPadForCallUnwindsLocally)
-          changeToUnreachable(CleanupRet, /*UseLLVMTrap=*/false);
+          changeToUnreachable(CleanupRet,
+                              /* PreferToUseLLVMTrap */ false);
 
       Instruction *I = BB->getFirstNonPHI();
       if (!I->isEHPad())
         continue;
 
       if (auto *CatchSwitch = dyn_cast<CatchSwitchInst>(I)) {
         if (isa<ConstantTokenNone>(CatchSwitch->getParentPad()))
           CatchSwitch->setParentPad(CallSiteEHPad);

llvm/lib/Transforms/Utils/Local.cpp

cl::desc("Perform extra assertion checking to verify that PHINodes's hash "
              "function is well-behaved w.r.t. its isEqual predicate"));
 
 static cl::opt<unsigned> PHICSENumPHISmallSize(
     "phicse-num-phi-smallsize", cl::init(32), cl::Hidden,
     cl::desc(
         "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<TrapBeforeUnreachableChoiceTy> 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.
 //
 
 /// ConstantFoldTerminator - If a terminator instruction is predicated on a
 /// constant value, convert it into an unconditional branch to the constant
 /// destination.  This is a nontrivial operation because the successors of this
 /// basic block must have their PHI nodes updated.
if (isa<DbgInfoIntrinsic>(Inst))
       ++NumDeadDbgInst;
     else
       ++NumDeadInst;
     Inst->eraseFromParent();
   }
   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();
 
   if (MSSAU)
     MSSAU->changeToUnreachable(I);
 
   SmallSet<BasicBlock *, 8> UniqueSuccessors;
 
+  bool UseLLVMTrap = shouldUseTrapBeforeUnreachable(PreferToUseLLVMTrap);
+
   // Loop over all of the successors, removing BB's entry from any PHI
   // nodes.
   for (BasicBlock *Successor : successors(BB)) {
     Successor->removePredecessor(BB, PreserveLCSSA);
     if (DTU)
       UniqueSuccessors.insert(Successor);
   }
   // Insert a call to llvm.trap right before this.  This turns the undefined
for (Instruction &I : *BB) {
           auto IntrinsicID = F->getIntrinsicID();
           // Assumptions that are known to be false are equivalent to
           // unreachable. Also, if the condition is undefined, then we make the
           // choice most beneficial to the optimizer, and choose that to also be
           // unreachable.
           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;
             }
           } else if (IntrinsicID == Intrinsic::experimental_guard) {
             // A call to the guard intrinsic bails out of the current
             // compilation unit if the predicate passed to it is false. If the
             // predicate is a constant false, then we know the guard will bail
             // out of the current compile unconditionally, so all code following
             // it is dead.
             //
             // Note: unlike in llvm.assume, it is not "obviously profitable" for
             // guards to treat `undef` as `false` since a guard on `undef` can
             // still be useful for widening.
             if (match(CI->getArgOperand(0), m_Zero()))
               if (!isa<UnreachableInst>(CI->getNextNode())) {
-                changeToUnreachable(CI->getNextNode(), /*UseLLVMTrap=*/false,
-                                    false, DTU);
+                changeToUnreachable(CI->getNextNode(),
+                                    /* PreferToUseLLVMTrap */ false, false,
+                                    DTU);
                 Changed = true;
                 break;
               }
           }
         } else if ((isa<ConstantPointerNull>(Callee) &&
                     !NullPointerIsDefined(CI->getFunction())) ||
                    isa<UndefValue>(Callee)) {
-          changeToUnreachable(CI, /*UseLLVMTrap=*/false, false, DTU);
+          changeToUnreachable(CI, /* PreferToUseLLVMTrap */ false, false, DTU);
           Changed = true;
           break;
         }
         if (CI->doesNotReturn() && !CI->isMustTailCall()) {
           // If we found a call to a no-return function, insert an unreachable
           // instruction after it.  Make sure there isn't *already* one there
           // though.
           if (!isa<UnreachableInst>(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;
         }
       } else if (auto *SI = dyn_cast<StoreInst>(&I)) {
         // Store to undef and store to null are undefined and used to signal
         // that they should be changed to unreachable by passes that can't
         // modify the CFG.
 
         // Don't touch volatile stores.
         if (SI->isVolatile()) continue;
 
         Value *Ptr = SI->getOperand(1);
 
         if (isa<UndefValue>(Ptr) ||
             (isa<ConstantPointerNull>(Ptr) &&
              !NullPointerIsDefined(SI->getFunction(),
                                    SI->getPointerAddressSpace()))) {
-          changeToUnreachable(SI, true, false, DTU);
+          changeToUnreachable(SI, /* PreferToUseLLVMTrap */ true, false, DTU);
           Changed = true;
           break;
         }
       }
     }
 
     Instruction *Terminator = BB->getTerminator();
     if (auto *II = dyn_cast<InvokeInst>(Terminator)) {
       // Turn invokes that call 'nounwind' functions into ordinary calls.
       Value *Callee = II->getCalledOperand();
       if ((isa<ConstantPointerNull>(Callee) &&
            !NullPointerIsDefined(BB->getParent())) ||
           isa<UndefValue>(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()) {
           // jump to the normal destination branch.
           BasicBlock *NormalDestBB = II->getNormalDest();
           BasicBlock *UnwindDestBB = II->getUnwindDest();
           BranchInst::Create(NormalDestBB, II);
           UnwindDestBB->removePredecessor(II->getParent());

llvm/lib/Transforms/Utils/LoopSimplify.cpp

for (Loop::block_iterator BB = L->block_begin(), E = L->block_end();
     // Delete each unique out-of-loop (and thus dead) predecessor.
     for (BasicBlock *P : BadPreds) {
 
       LLVM_DEBUG(dbgs() << "LoopSimplify: Deleting edge from dead predecessor "
                         << P->getName() << "\n");
 
       // 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;
     }
   }
 
   if (MSSAU && VerifyMemorySSA)
     MSSAU->getMemorySSA()->verifyMemorySSA();
 

llvm/lib/Transforms/Utils/LoopUnroll.cpp

for (unsigned i = 0, e = Info.ExitingBlocks.size(); i != e; ++i) {
         continue;
 
       SetDest(Info.ExitingBlocks[i], *KnownWillExit, Info.ExitOnTrue);
     }
   }
 
   // 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) {
     BranchInst *Term = dyn_cast<BranchInst>(Latch->getTerminator());
     assert((Term ||
             (CompletelyUnroll && !LatchIsExiting && Latch == Latches.back())) &&
            "Need a branch as terminator, except when fully unrolling with "
            "unconditional latch");

llvm/lib/Transforms/Utils/LoopUtils.cpp

void llvm::breakLoopBackedge(Loop *L, DominatorTree &DT, ScalarEvolution &SE,
 
   std::unique_ptr<MemorySSAUpdater> MSSAU;
   if (MSSA)
     MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
 
   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.
   LI.erase(L);
 
   // If the loop we broke had a parent, then changeToUnreachable might have
   // caused a block to be removed from the parent loop (see loop_nest_lcssa
   // test case in zero-btc.ll for an example), thus changing the parent's

llvm/test/Transforms/SimplifyCFG/assume.ll

-; 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
+;
 
-; 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
+;
 
-; CHECK-LABEL: @test2
-; CHECK-NOT: llvm.assume
-; CHECK: unreachable
 }
 
 declare void @llvm.assume(i1) nounwind
 

llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll

 ; 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 =
 "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32"
 target triple = "i386-pc-linux-gnu"
 
 define void @test1(i32 %x) nounwind {
 ; CHECK-LABEL: @test1(
bb:             ; preds = %entry
 
   br label %return
 return:         ; preds = %entry
   ret void
 }
 
 ; 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
   ret void
 
 }
 
 define void @test2_no_null_opt() nounwind #0 {

llvm/unittests/Transforms/Utils/LocalTest.cpp

TEST(Local, ChangeToUnreachable) {
   Function &F = *cast<Function>(M->getNamedValue("foo"));
 
   BasicBlock &BB = F.front();
   Instruction &A = BB.front();
   DebugLoc DLA = A.getDebugLoc();
 
   ASSERT_TRUE(isa<ReturnInst>(&A));
   // One instruction should be affected.
-  EXPECT_EQ(changeToUnreachable(&A, /*UseLLVMTrap*/false), 1U);
+  EXPECT_EQ(changeToUnreachable(&A, /*PreferToUseLLVMTrap*/ false), 1U);
 
   Instruction &B = BB.front();
 
   // There should be an uncreachable instruction.
   ASSERT_TRUE(isa<UnreachableInst>(&B));
 
   DebugLoc DLB = B.getDebugLoc();
   EXPECT_EQ(DLA, DLB);