[llvm] ee371b2 - [DebugInfo] Treat empty metadata operands the same as undef operands

[via llvm-commits]

Author: OCHyams
Date: 2023-04-25T17:17:45+01:00
New Revision: ee371b2566eef7178b1e568a63bc2f961684581b

URL: https://github.com/llvm/llvm-project/commit/ee371b2566eef7178b1e568a63bc2f961684581b
DIFF: https://github.com/llvm/llvm-project/commit/ee371b2566eef7178b1e568a63bc2f961684581b.diff

LOG: [DebugInfo] Treat empty metadata operands the same as undef operands

A `ValueAsMetadata` may be replaced with nullptr for several reasons including
deleting 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 changes `isKillLocation` to take this into account.

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

Reviewed By: StephenTozer

Differential Revision: https://reviews.llvm.org/D140902

Added: 
    

Modified: 
    llvm/include/llvm/IR/IntrinsicInst.h
    llvm/unittests/IR/DebugInfoTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index b51416b5c17df..62bd833198f02 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -251,8 +251,16 @@ class RawLocationWrapper {
   }
   bool hasArgList() const { return isa<DIArgList>(getRawLocation()); }
   bool isKillLocation(const DIExpression *Expression) const {
-    return (getNumVariableLocationOps() == 0 && !Expression->isComplex()) ||
-           any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
+    // Check for "kill" sentinel values.
+    // Non-variadic: empty metadata.
+    if (!hasArgList() && isa<MDNode>(getRawLocation()))
+      return true;
+    // Variadic: empty DIArgList with empty expression.
+    if (getNumVariableLocationOps() == 0 && !Expression->isComplex())
+      return true;
+    // Variadic and non-variadic: Interpret expressions using undef or poison
+    // values as kills.
+    return any_of(location_ops(), [](Value *V) { return isa<UndefValue>(V); });
   }
 
   friend bool operator==(const RawLocationWrapper &A,

diff  --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index f038ff2fc7f31..a22c7be73f49f 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -190,6 +190,47 @@ TEST(MetadataTest, DeleteInstUsedByDbgValue) {
   EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue(0)));
 }
 
+TEST(DbgVariableIntrinsic, EmptyMDIsKillLocation) {
+  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"));
+  DbgVariableIntrinsic *DbgDeclare =
+      cast<DbgVariableIntrinsic>(&F.front().front());
+  // Check that this form counts as a "no location" marker.
+  EXPECT_TRUE(DbgDeclare->isKillLocation());
+}
+
 TEST(DIBuiler, CreateFile) {
   LLVMContext Ctx;
   std::unique_ptr<Module> M(new Module("MyModule", Ctx));