[llvm] [DebugInfo][RemoveDIs] Final cleanup for enabling non-instr-debuginfo (PR #74497)

[Jeremy Morse via llvm-commits]

https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/74497

>From c68790766722ec05f3cd65c04cb07324260748d6 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Tue, 5 Dec 2023 17:00:38 +0000
Subject: [PATCH 1/2] [DebugInfo][RemoveDIs] Final cleanup for enabling
 non-instr-debuginfo

Some final errors have turned up when doing stage2clang builds:
 * We can insert before end(), which won't have an attached DPMarker, thus
   we can have a nullptr DPMarker in Instruction::insertBefore. Add a
   null check there.
 * We need to use the iterator-returning form of getFirstNonPHI to ensure
   we don't insert any PHIs behind debug-info at the start of a block.
---
 llvm/lib/IR/Instruction.cpp         | 2 +-
 llvm/lib/Transforms/Utils/Local.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 920a2ecfb07ee..717e33f1857b8 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -218,7 +218,7 @@ void Instruction::moveBeforeImpl(BasicBlock &BB, InstListType::iterator I,
 
     // If we're inserting at point I, and not in front of the DPValues attached
     // there, then we should absorb the DPValues attached to I.
-    if (!InsertAtHead)
+    if (NextMarker && !InsertAtHead)
       DbgMarker->absorbDebugValues(*NextMarker, false);
   }
 
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index dfc78aa589ef8..51f39e0ba0cce 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1295,7 +1295,7 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
     // the same predecessors BB had.
     // Copy over any phi, debug or lifetime instruction.
     BB->getTerminator()->eraseFromParent();
-    Succ->splice(Succ->getFirstNonPHI()->getIterator(), BB);
+    Succ->splice(Succ->getFirstNonPHIIt(), BB);
   } else {
     while (PHINode *PN = dyn_cast<PHINode>(&BB->front())) {
       // We explicitly check for such uses for merging phis.

>From 0b330996b1d4486fd15d3daedff15d7aab585eb3 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Tue, 5 Dec 2023 17:14:08 +0000
Subject: [PATCH 2/2] Add a unit tests

---
 llvm/unittests/IR/BasicBlockDbgInfoTest.cpp | 53 +++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index 46b42a257f588..fb4847fc0a826 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -1481,5 +1481,58 @@ TEST(BasicBlockDbgInfoTest, DbgSpliceToEmpty2) {
 
   UseNewDbgInfoFormat = false;
 }
+
+// What if we moveBefore end() -- there might be no debug-info there, in which
+// case we shouldn't crash.
+TEST(BasicBlockDbgInfoTest, DbgMoveToEnd) {
+  LLVMContext C;
+  UseNewDbgInfoFormat = true;
+
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+    entry:
+      br label %exit
+
+    exit:
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+    attributes #0 = { nounwind readnone speculatable willreturn }
+
+    !llvm.dbg.cu = !{!0}
+    !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: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+  Function &F = *M->getFunction("f");
+  BasicBlock &Entry = F.getEntryBlock();
+  BasicBlock &Exit = *Entry.getNextNode();
+  M->convertToNewDbgValues();
+
+  // Move the return to the end of the entry block.
+  Instruction *Br = Entry.getTerminator();
+  Instruction *Ret = Exit.getTerminator();
+  EXPECT_EQ(Entry.getTrailingDPValues(), nullptr);
+  Ret->moveBefore(Entry, Entry.end());
+  Br->eraseFromParent();
+
+  // There should continue to not be any debug-info anywhere.
+  EXPECT_EQ(Entry.getTrailingDPValues(), nullptr);
+  EXPECT_EQ(Exit.getTrailingDPValues(), nullptr);
+  EXPECT_FALSE(Ret->hasDbgValues());
+
+  UseNewDbgInfoFormat = false;
+}
+
 } // End anonymous namespace.
 #endif // EXPERIMENTAL_DEBUGINFO_ITERATORS