[Local] Add dbg location on unreachable inst in changeToUnreachable

[gramanas]

Summary

As show in https://bugs.llvm.org/show_bug.cgi?id=37960
it would be desirable to have debug location in the unreachable
instruction.

If the unreachable is lowered to llvm.trap() or similar,
it could be really useful to have line/scope information there
to make forensic debugging more pleasant.

I am not sure whether to add a unit test for this funtion.

[vsk]

Thanks for doing this! Yes, please add a unit test in unittests/Transforms/Utils/Local.cpp.

[gramanas]

Add unit test.

[vsk]

unittests/Transforms/Utils/Local.cpp

616

Why not just .front()?

620

nit, 1U is easier to read.

620

Please add a comment when using boolean literals as parameters. Instead of 'false', it's clearer to write /*UseLLVMTrap*/false.

626

This needs to check that the correct location is present, not just that the instruction has metadata.

[gramanas]

• update according to comments

[vsk]

LGTM, thanks!

unittests/Transforms/Utils/Local.cpp

627

Nit, this assertion is now redundant.

lib/Transforms/Utils/Local.cpp

unsigned llvm::changeToUnreachable(Instruction *I, bool UseLLVMTrap,
   // Insert a call to llvm.trap right before this.  This turns the undefined
   // behavior into a hard fail instead of falling through into random code.
   if (UseLLVMTrap) {
     Function *TrapFn =
       Intrinsic::getDeclaration(BB->getParent()->getParent(), Intrinsic::trap);
     CallInst *CallTrap = CallInst::Create(TrapFn, "", I);
     CallTrap->setDebugLoc(I->getDebugLoc());
   }
-  new UnreachableInst(I->getContext(), I);
+  auto *UI = new UnreachableInst(I->getContext(), I);
+  UI->setDebugLoc(I->getDebugLoc());
 
   // All instructions after this are dead.
   unsigned NumInstrsRemoved = 0;
   BasicBlock::iterator BBI = I->getIterator(), BBE = BB->end();
   while (BBI != BBE) {
     if (!BBI->use_empty())
       BBI->replaceAllUsesWith(UndefValue::get(BBI->getType()));
     BB->getInstList().erase(BBI++);

unittests/Transforms/Utils/Local.cpp

 TEST_F(SalvageDebugInfoTest, RecursiveBlockSimplification) {
   BasicBlock *BB = &F->front();
   ASSERT_TRUE(BB);
   bool Deleted = SimplifyInstructionsInBlock(BB);
   ASSERT_TRUE(Deleted);
   verifyDebugValuesAreSalvaged();
 }
 
+TEST(Local, ChangeToUnreachable) {
+  LLVMContext Ctx;
+
+  std::unique_ptr<Module> M = parseIR(Ctx,
+                                      R"(
+    define internal void @foo() !dbg !6 {
+    entry:
+      ret void, !dbg !8
+    }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.debugify = !{!3, !4}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "test.ll", directory: "/")
+    !2 = !{}
+    !3 = !{i32 1}
+    !4 = !{i32 0}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, isLocal: true, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !2)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !DILocation(line: 1, column: 1, scope: !6)
+  )");
+
+  bool BrokenDebugInfo = true;
+  verifyModule(*M, &errs(), &BrokenDebugInfo);
+  ASSERT_FALSE(BrokenDebugInfo);
+
+  Function &F = *cast<Function>(M->getNamedValue("foo"));
+
+  BasicBlock &BB = F.front();
+  Instruction *A = BB.getFirstNonPHI();
+
+  ASSERT_TRUE(isa<ReturnInst>(A));
+  // One instruction should be affected.
+  EXPECT_EQ(changeToUnreachable(A, false), (unsigned)1);
+
+  Instruction *B = BB.getFirstNonPHI();
+
+  // Should be an uncreachable instruction with metadata attached.
+  ASSERT_TRUE(isa<UnreachableInst>(B));
+  ASSERT_TRUE(B->hasMetadata());
+}
+
 TEST(Local, ReplaceAllDbgUsesWith) {
   using namespace llvm::dwarf;
 
   LLVMContext Ctx;
 
   // Note: The datalayout simulates Darwin/x86_64.
   std::unique_ptr<Module> M = parseIR(Ctx,
                                       R"(
TEST(Local, RemoveUnreachableBlocks) {
   auto checkRUBlocksRetVal = [&](Function &F, DominatorTree *DT) {
     DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
     EXPECT_TRUE(removeUnreachableBlocks(F, nullptr, &DTU));
     EXPECT_FALSE(removeUnreachableBlocks(F, nullptr, &DTU));
     EXPECT_TRUE(DTU.getDomTree().verify());
   };
 
   runWithDomTree(*M, "f", checkRUBlocksRetVal);
-}
+}