[DebugInfo] Do not delete debug intrinsics with empty metadata operands

[Orlando]

Summary

A ValueAsMetadata may be replaced with nullptr for several reasons including deleting (certain) values and value remapping a use-before-def. In the case of a
MetadataAsValue user, handleChangedOperand intercepts and replaces the metadata with an empty tuple (!{}).

At the moment, an empty metadata operand in a debug intrinsics signals that it can be deleted.

Given that we end up with empty metadata operands in circumstances where the Value has been "lost" the current behaviour can lead to incorrect variable locations. Instead, we should treat empty metadata as meaning "there is no location for the variable" (the same as we currently treat undef operands).

This patch removes the deletion logic from wouldInstructionBeTriviallyDead.

Related to https://discourse.llvm.org/t/auto-undef-debug-uses-of-a-deleted-value

[StephenTozer]

Simple enough, LGTM (but since this is part of a more substantial patch stack, leave a little time for others to have a look).

[Orlando]

Thanks for the review. I factored out a part of this into D141140 so it can be committed before this patch (which I agree needs more time to receive others' reviews - I'll open a fresh discourse thread soon).

llvm/lib/IR/Verifier.cpp

6028–6032

Factored out this part into another patch that comes with a test: D141140

llvm/lib/IR/Verifier.cpp

CheckDI(isa<DILocalVariable>(DII.getRawVariable()),
           DII.getRawVariable());
   CheckDI(isa<DIExpression>(DII.getRawExpression()),
           "invalid llvm.dbg." + Kind + " intrinsic expression", &DII,
           DII.getRawExpression());
 
   if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(&DII)) {
     CheckDI(isa<DIAssignID>(DAI->getRawAssignID()),
             "invalid llvm.dbg.assign intrinsic DIAssignID", &DII,
             DAI->getRawAssignID());
-    CheckDI(isa<ValueAsMetadata>(DAI->getRawAddress()),
-            "invalid llvm.dbg.assign intrinsic address)", &DII,
-            DAI->getRawAddress());
+    const auto *RawAddr = DAI->getRawAddress();
+    CheckDI(
+        isa<ValueAsMetadata>(RawAddr) ||
+            (isa<MDNode>(RawAddr) && !cast<MDNode>(RawAddr)->getNumOperands()),
     CheckDI(isa<DIExpression>(DAI->getRawAddressExpression()),
             "invalid llvm.dbg.assign intrinsic address expression", &DII,
             DAI->getRawAddressExpression());
     // All of the linked instructions should be in the same function as DII.
     for (Instruction *I : at::getAssignmentInsts(DAI))
       CheckDI(DAI->getFunction() == I->getFunction(),
               "inst not in same function as dbg.assign", I, DAI);
   }

llvm/lib/Transforms/Utils/Local.cpp

bool llvm::wouldInstructionBeTriviallyDead(Instruction *I,
   if (I->isTerminator())
     return false;
 
   // We don't want the landingpad-like instructions removed by anything this
   // general.
   if (I->isEHPad())
     return false;
 
-  // We don't want debug info removed by anything this general, unless
-  // debug info is empty.
-  if (DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(I)) {
-    if (DDI->getAddress())
-      return false;
-    return true;
-  }
-  if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(I)) {
-    if (DVI->hasArgList() || DVI->getValue(0))
-      return false;
-    return true;
-  }
-  if (DbgLabelInst *DLI = dyn_cast<DbgLabelInst>(I)) {
-    if (DLI->getLabel())
-      return false;
-    return true;
-  }
+  // We don't want debug info removed by anything this general.
+  if (isa<DbgVariableIntrinsic>(I))
 
   if (auto *CB = dyn_cast<CallBase>(I))
     if (isRemovableAlloc(CB, TLI))
       return true;
 
   if (!I->willReturn()) {
     auto *II = dyn_cast<IntrinsicInst>(I);
     if (!II)

llvm/test/DebugInfo/Generic/empty-metadata.ll

+; RUN: opt -passes=dce -S %s -o - | FileCheck %s
+
+;; Check that DCE doesn't remove a dbg intrinsic that has an empty metadata
+;; operand.
+; CHECK: call void @llvm.dbg.declare(metadata !{},
+
+define dso_local void @fun() local_unnamed_addr #0 !dbg !9 {
+entry:
+  call void @llvm.dbg.declare(metadata !{}, metadata !13, metadata !DIExpression()), !dbg !15
+  ret void, !dbg !16
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 16.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.c", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{!"clang version 16.0.0"}
+!9 = distinct !DISubprogram(name: "fun", scope: !1, file: !1, line: 1, type: !10, scopeLine: 1, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12)
+!10 = !DISubroutineType(types: !11)
+!11 = !{null}
+!12 = !{!13}
+!13 = !DILocalVariable(name: "a", scope: !9, file: !1, line: 1, type: !14)
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!15 = !DILocation(line: 1, column: 18, scope: !9)
+!16 = !DILocation(line: 1, column: 21, scope: !9)
+

llvm/unittests/Transforms/Utils/LocalTest.cpp

TEST(Local, SimplifyVScaleWithRange) {
   auto *CI = CallInst::Create(VScale, {}, "vscale");
 
   // Test that simplifyCall won't try to query it's parent function for
   // vscale_range attributes in order to simplify llvm.vscale -> constant.
   EXPECT_EQ(simplifyCall(CI, SimplifyQuery(M.getDataLayout())), nullptr);
   delete CI;
 }
 
+TEST(Local, wouldInstructionBeTriviallyDead) {
+  LLVMContext Ctx;
+  std::unique_ptr<Module> M = parseIR(Ctx,
+                                      R"(
+    define dso_local void @fun() local_unnamed_addr #0 !dbg !9 {
+    entry:
+      call void @llvm.dbg.declare(metadata !{}, metadata !13, metadata !DIExpression()), !dbg !16
+      ret void, !dbg !16
+    }
+
+    declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!2, !3}
+    !llvm.ident = !{!8}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 16.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+    !1 = !DIFile(filename: "test.c", directory: "/")
+    !2 = !{i32 7, !"Dwarf Version", i32 5}
+    !3 = !{i32 2, !"Debug Info Version", i32 3}
+    !8 = !{!"clang version 16.0.0"}
+    !9 = distinct !DISubprogram(name: "fun", scope: !1, file: !1, line: 1, type: !10, scopeLine: 1, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12)
+    !10 = !DISubroutineType(types: !11)
+    !11 = !{null}
+    !12 = !{!13}
+    !13 = !DILocalVariable(name: "a", scope: !9, file: !1, line: 1, type: !14)
+    !14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+    !16 = !DILocation(line: 1, column: 21, scope: !9)
+    )");
+  bool BrokenDebugInfo = true;
+  verifyModule(*M, &errs(), &BrokenDebugInfo);
+  ASSERT_FALSE(BrokenDebugInfo);
+
+  // Get the dbg.declare.
+  Function &F = *cast<Function>(M->getNamedValue("fun"));
+  Instruction *DbgDeclare = &F.front().front();
+  ASSERT_TRUE(isa<DbgDeclareInst>(DbgDeclare));
+  // Debug intrinsics with empty metadata arguments are not dead.
+  EXPECT_FALSE(wouldInstructionBeTriviallyDead(DbgDeclare));
+}
+
 TEST(Local, ChangeToUnreachable) {
   LLVMContext Ctx;
 
   std::unique_ptr<Module> M = parseIR(Ctx,
                                       R"(
     define internal void @foo() !dbg !6 {
     entry:
       ret void, !dbg !8