[DebugInfo@O2][Utils] Undef instead of delete dbg.values in helper func

Summary:
Related bug: https://bugs.llvm.org/show_bug.cgi?id=40648

Static helper function rewriteDebugUsers in Local.cpp deletes dbg.value
intrinsics when it cannot move or rewrite them, or salvage the deleted
instruction's value. It should instead undef them in this case.

This patch fixes that and I've added a test which covers the failing test
case in bz40648. I've updated the unit test Local.ReplaceAllDbgUsesWith
to check for this behaviour (and fixed a typo in the test which would
cause the old test to always pass).

Reviewers: aprantl, vsk, djtodoro, probinson

Reviewed By: vsk

Subscribers: hiraditya, llvm-commits

Tags: #debug-info, #llvm

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

3 files changed, 88 insertions, 17 deletions

diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index aa1341eea05..eaccaa1c4ac 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1732,7 +1732,7 @@ DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
 using DbgValReplacement = Optional<DIExpression *>;
 
 /// Point debug users of \p From to \p To using exprs given by \p RewriteExpr,
-/// possibly moving/deleting users to prevent use-before-def. Returns true if
+/// possibly moving/undefing users to prevent use-before-def. Returns true if
 /// changes are made.
 static bool rewriteDebugUsers(
     Instruction &From, Value &To, Instruction &DomPoint, DominatorTree &DT,
@@ -1745,7 +1745,7 @@ static bool rewriteDebugUsers(
 
   // Prevent use-before-def of To.
   bool Changed = false;
-  SmallPtrSet<DbgVariableIntrinsic *, 1> DeleteOrSalvage;
+  SmallPtrSet<DbgVariableIntrinsic *, 1> UndefOrSalvage;
   if (isa<Instruction>(&To)) {
     bool DomPointAfterFrom = From.getNextNonDebugInstruction() == &DomPoint;
 
@@ -1760,14 +1760,14 @@ static bool rewriteDebugUsers(
       // Users which otherwise aren't dominated by the replacement value must
       // be salvaged or deleted.
       } else if (!DT.dominates(&DomPoint, DII)) {
-        DeleteOrSalvage.insert(DII);
+        UndefOrSalvage.insert(DII);
       }
     }
   }
 
   // Update debug users without use-before-def risk.
   for (auto *DII : Users) {
-    if (DeleteOrSalvage.count(DII))
+    if (UndefOrSalvage.count(DII))
       continue;
 
     LLVMContext &Ctx = DII->getContext();
@@ -1781,18 +1781,10 @@ static bool rewriteDebugUsers(
     Changed = true;
   }
 
-  if (!DeleteOrSalvage.empty()) {
+  if (!UndefOrSalvage.empty()) {
     // Try to salvage the remaining debug users.
-    Changed |= salvageDebugInfo(From);
-
-    // Delete the debug users which weren't salvaged.
-    for (auto *DII : DeleteOrSalvage) {
-      if (DII->getVariableLocation() == &From) {
-        LLVM_DEBUG(dbgs() << "Erased UseBeforeDef:  " << *DII << '\n');
-        DII->eraseFromParent();
-        Changed = true;
-      }
-    }
+    salvageDebugInfoOrMarkUndef(From);
+    Changed = true;
   }
 
   return Changed;
diff --git a/llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll b/llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll
new file mode 100644
index 00000000000..18e51b6fabd
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/dbg-value-dropped-instcombine.ll
@@ -0,0 +1,76 @@
+; RUN: opt -instcombine -S %s -o - | FileCheck %s
+
+; In pr40648 one of two dbg.values used to describe the variable bumble was
+; being dropped by instcombine. Test that both dbg.values survive instcombine.
+
+; $ clang -O0 -Xclang -disable-O0-optnone -g bees.c -emit-llvm -S -o - \
+;   | opt -opt-bisect-limit=10 -O2 -o -
+; $ cat bees.c
+; struct bees {
+;  int a;
+;  int b;
+; }
+
+; int global = 0;
+
+; struct bees foo(struct bees bumble)
+; {
+;   global = 1;
+;   int temp = bumble.a + bumble.b;
+;   return bumble;
+; }
+
+; CHECK: define dso_local i64 @foo
+; CHECK: @llvm.dbg.value({{.*}}, metadata ![[BEE:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)),
+; CHECK: @llvm.dbg.value(metadata i32 undef, metadata ![[BEE]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)),
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-unknown"
+
+@global = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0
+
+define dso_local i64 @foo(i64 %bumble.coerce) local_unnamed_addr !dbg !11 {
+entry:
+  %bumble.sroa.0.0.extract.trunc = trunc i64 %bumble.coerce to i32
+  call void @llvm.dbg.value(metadata i32 %bumble.sroa.0.0.extract.trunc, metadata !18, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !19
+  %bumble.sroa.3.0.extract.shift = lshr i64 %bumble.coerce, 32
+  %bumble.sroa.3.0.extract.trunc = trunc i64 %bumble.sroa.3.0.extract.shift to i32
+  call void @llvm.dbg.value(metadata i32 %bumble.sroa.3.0.extract.trunc, metadata !18, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !19
+  store i32 1, i32* @global, align 4, !dbg !20
+  call void @llvm.dbg.value(metadata i32 undef, metadata !21, metadata !DIExpression()), !dbg !19
+  %retval.sroa.2.0.insert.ext = zext i32 %bumble.sroa.3.0.extract.trunc to i64, !dbg !22
+  %retval.sroa.2.0.insert.shift = shl i64 %retval.sroa.2.0.insert.ext, 32, !dbg !22
+  %retval.sroa.0.0.insert.ext = zext i32 %bumble.sroa.0.0.extract.trunc to i64, !dbg !22
+  %retval.sroa.0.0.insert.insert = or i64 %retval.sroa.2.0.insert.shift, %retval.sroa.0.0.insert.ext, !dbg !22
+  ret i64 %retval.sroa.0.0.insert.insert, !dbg !22
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8, !9}
+!llvm.ident = !{!10}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "global", scope: !2, file: !3, line: 6, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
+!3 = !DIFile(filename: "bees.c", directory: "/")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 2, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+!9 = !{i32 1, !"wchar_size", i32 4}
+!10 = !{!"clang version 10.0.0"}
+!11 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 8, type: !12, scopeLine: 9, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4)
+!12 = !DISubroutineType(types: !13)
+!13 = !{!14, !14}
+!14 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "bees", file: !3, line: 1, size: 64, elements: !15)
+!15 = !{!16, !17}
+!16 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !14, file: !3, line: 2, baseType: !6, size: 32)
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !14, file: !3, line: 3, baseType: !6, size: 32, offset: 32)
+!18 = !DILocalVariable(name: "bumble", arg: 1, scope: !11, file: !3, line: 8, type: !14)
+!19 = !DILocation(line: 0, scope: !11)
+!20 = !DILocation(line: 10, column: 10, scope: !11)
+!21 = !DILocalVariable(name: "temp", scope: !11, file: !3, line: 11, type: !6)
+!22 = !DILocation(line: 12, column: 3, scope: !11)
diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp
index 1f67a1ec84c..99713d5817f 100644
--- a/llvm/unittests/Transforms/Utils/LocalTest.cpp
+++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp
@@ -759,11 +759,14 @@ TEST(Local, ReplaceAllDbgUsesWith) {
   auto *ADbgVal = cast<DbgValueInst>(A.getNextNode());
   EXPECT_EQ(ConstantInt::get(A.getType(), 0), ADbgVal->getVariableLocation());
 
-  // Introduce a use-before-def. Check that the dbg.values for %f are deleted.
+  // Introduce a use-before-def. Check that the dbg.values for %f become undef.
   EXPECT_TRUE(replaceAllDbgUsesWith(F_, G, G, DT));
 
+  auto *FDbgVal = cast<DbgValueInst>(F_.getNextNode());
+  EXPECT_TRUE(isa<UndefValue>(FDbgVal->getVariableLocation()));
+
   SmallVector<DbgValueInst *, 1> FDbgVals;
-  findDbgValues(FDbgVals, &F);
+  findDbgValues(FDbgVals, &F_);
   EXPECT_EQ(0U, FDbgVals.size());
 
   // Simulate i32 -> i64 conversion to test sign-extension. Here are some