Attempt to Salvage Debug Info or undef intrinsics when dropping trivially deletable instructions in reassociate pass

[TWeaver]

Summary

Fix for BZ38753 - https://bugs.llvm.org/show_bug.cgi?id=38753

Reassociate Expressions pass made no attempt to salvage or "fix up" debug info when dropping trivially delete-able instructions.

This was causing intrinsics that previously pointed to locations to have "dangling" locations that would be fixed up to empty metadata. Later passes would then remove these useless intrinsics. When we eventually reach the back-end, in LLC, LiveDebugValues would then propogate earlier intrinsics into as many blocks as possible. This would cause stale values to be seen in the debugger when debugging optmized code.

There's two test cases, one for undefs and another for a simple salvage case.

Thank you for your time reviewing this patch.

Tom W

[dstenb]

If D69809 lands before this patch, then you can use the helper function salvageDebugInfoOrMarkUndef() which the former introduces.

[TWeaver]

In D69943#1736998, @dstenb wrote:

If D69809 lands before this patch, then you can use the helper function salvageDebugInfoOrMarkUndef() which the former introduces.

Oh that'd be great, I'll hold off until that goes through then and touch this patch up to use the new helper. thanks for the shout out!

[vsk]

Looks great to me, assuming the plan is to use the 'salvageDebugInfoOrMarkUndef' helper (now available). Thanks!

[aprantl]

Nice!

[TWeaver]

updated to use newly added helper function for salvage or undeffing behaviour when dropping instructions in reassociate expressions pass.

[TWeaver]

Updated diff to correct revision for easing future historical searches

llvm/lib/Transforms/Scalar/Reassociate.cpp

 // Insts so they will be removed as well.
 void ReassociatePass::RecursivelyEraseDeadInsts(Instruction *I,
                                                 OrderedSet &Insts) {
   assert(isInstructionTriviallyDead(I) && "Trivially dead instructions only!");
   SmallVector<Value *, 4> Ops(I->op_begin(), I->op_end());
   ValueRankMap.erase(I);
   Insts.remove(I);
   RedoInsts.remove(I);
+  if (!llvm::salvageDebugInfo(*I))
+    llvm::replaceDbgUsesWithUndef(I);
   I->eraseFromParent();
   for (auto Op : Ops)
     if (Instruction *OpInst = dyn_cast<Instruction>(Op))
       if (OpInst->use_empty())
         Insts.insert(OpInst);
 }
 
 /// Zap the given instruction, adding interesting operands to the work list.
 void ReassociatePass::EraseInst(Instruction *I) {
   assert(isInstructionTriviallyDead(I) && "Trivially dead instructions only!");
   LLVM_DEBUG(dbgs() << "Erasing dead inst: "; I->dump());
 
   SmallVector<Value*, 8> Ops(I->op_begin(), I->op_end());
   // Erase the dead instruction.
   ValueRankMap.erase(I);
   RedoInsts.remove(I);
+  if (!llvm::salvageDebugInfo(*I))
+    llvm::replaceDbgUsesWithUndef(I);
   I->eraseFromParent();
   // Optimize its operands.
   SmallPtrSet<Instruction *, 8> Visited; // Detect self-referential nodes.
   for (unsigned i = 0, e = Ops.size(); i != e; ++i)
     if (Instruction *Op = dyn_cast<Instruction>(Ops[i])) {
       // If this is a node in an expression tree, climb to the expression root
       // and add that since that's where optimization actually happens.
       unsigned Opcode = Op->getOpcode();

llvm/test/Transforms/Reassociate/reassociate_salvages_debug_info.ll

+; RUN: opt < %s -reassociate -S | FileCheck %s
+
+; Check that reassociate pass now salvages debug info when dropping instructions.
+
+define hidden i32 @main(i32 %argc, i8** %argv) {
+entry:
+  ; CHECK: call void @llvm.dbg.value(metadata i32 %argc, metadata [[VAR_B:![0-9]+]], metadata !DIExpression(DW_OP_plus_uconst, 1, DW_OP_stack_value))
+  %add = add nsw i32 %argc, 1, !dbg !26
+  call void @llvm.dbg.value(metadata i32 %add, metadata !22, metadata !DIExpression()), !dbg !25
+  %add1 = add nsw i32 %argc, %add, !dbg !27
+  ret i32 %add1, !dbg !28
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "test2.cpp", directory: "C:\")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 10.0.0"}
+!8 = distinct !DISubprogram(name: "main", scope: !9, file: !9, line: 1, type: !10, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !18)
+!9 = !DIFile(filename: "./test2.cpp", directory: "C:\")
+!10 = !DISubroutineType(types: !11)
+!11 = !{!12, !13, !14}
+!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!13 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !12)
+!14 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !15, size: 64)
+!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !16, size: 64)
+!16 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !17)
+!17 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!18 = !{!19, !20, !21, !22, !23, !24}
+!19 = !DILocalVariable(name: "argc", arg: 1, scope: !8, file: !9, line: 1, type: !13)
+!20 = !DILocalVariable(name: "argv", arg: 2, scope: !8, file: !9, line: 1, type: !14)
+!21 = !DILocalVariable(name: "a", scope: !8, file: !9, line: 2, type: !12)
+; CHECK: [[VAR_B]] = !DILocalVariable(name: "b"
+!22 = !DILocalVariable(name: "b", scope: !8, file: !9, line: 3, type: !12)
+!23 = !DILocalVariable(name: "to_return", scope: !8, file: !9, line: 4, type: !12)
+!24 = !DILocalVariable(name: "result", scope: !8, file: !9, line: 5, type: !12)
+!25 = !DILocation(line: 0, scope: !8)
+!26 = !DILocation(line: 3, scope: !8)
+!27 = !DILocation(line: 4, scope: !8)
+!28 = !DILocation(line: 6, scope: !8)

llvm/test/Transforms/Reassociate/undef_intrinsics_when_deleting_instructions.ll

+; RUN: opt < %s -reassociate -S | FileCheck %s
+
+; Check that reassociate pass now undefs debug intrinsics that reference a value
+; that gets dropped and cannot be salvaged.
+
+define hidden i32 @main() local_unnamed_addr {
+entry:
+  %foo = alloca i32, align 4, !dbg !20
+  %foo.0.foo.0..sroa_cast = bitcast i32* %foo to i8*, !dbg !20
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %foo.0.foo.0..sroa_cast), !dbg !20
+  store volatile i32 4, i32* %foo, align 4, !dbg !20, !tbaa !21
+  %foo.0.foo.0. = load volatile i32, i32* %foo, align 4, !dbg !25, !tbaa !21
+  %foo.0.foo.0.15 = load volatile i32, i32* %foo, align 4, !dbg !27, !tbaa !21
+  %foo.0.foo.0.16 = load volatile i32, i32* %foo, align 4, !dbg !28, !tbaa !21
+  ; CHECK-NOT: %add = add nsw i32 %foo.0.foo.0., %foo.0.foo.0.15
+  %add = add nsw i32 %foo.0.foo.0., %foo.0.foo.0.15, !dbg !29
+  ; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[VAR_A:![0-9]+]], metadata !DIExpression())
+  call void @llvm.dbg.value(metadata i32 %add, metadata !19, metadata !DIExpression()), !dbg !26
+  %foo.0.foo.0.17 = load volatile i32, i32* %foo, align 4, !dbg !30, !tbaa !21
+  %cmp = icmp eq i32 %foo.0.foo.0.17, 4, !dbg !30
+  br i1 %cmp, label %if.then, label %if.end, !dbg !32
+
+  ; CHECK-LABEL: if.then:
+if.then:
+  ; CHECK-NOT: %add1 = add nsw i32 %add, %foo.0.foo.0.16
+  %add1 = add nsw i32 %add, %foo.0.foo.0.16, !dbg !33
+  ; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[VAR_A]], metadata !DIExpression())
+  call void @llvm.dbg.value(metadata i32 %add1, metadata !19, metadata !DIExpression()), !dbg !26
+  ; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata [[VAR_CHEESE:![0-9]+]], metadata !DIExpression())
+  call void @llvm.dbg.value(metadata i32 %add, metadata !18, metadata !DIExpression()), !dbg !26
+  %sub = add nsw i32 %add, -12, !dbg !34
+  %sub3 = sub nsw i32 %add1, %sub, !dbg !34
+  %mul = mul nsw i32 %sub3, 20, !dbg !36
+  %div = sdiv i32 %mul, 3, !dbg !37
+  br label %if.end, !dbg !38
+
+if.end:
+  %a.0 = phi i32 [ %div, %if.then ], [ 0, %entry ], !dbg !39
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %foo.0.foo.0..sroa_cast), !dbg !40
+  ret i32 %a.0, !dbg !41
+}
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #2
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #1
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "F:\")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 10.0.0"}
+!8 = distinct !DISubprogram(name: "main", scope: !9, file: !9, line: 1, type: !10, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !13)
+!9 = !DIFile(filename: "./test.cpp", directory: "F:\")
+!10 = !DISubroutineType(types: !11)
+!11 = !{!12}
+!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!13 = !{!14, !16, !17, !18, !19}
+!14 = !DILocalVariable(name: "foo", scope: !8, file: !9, line: 2, type: !15)
+!15 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !12)
+!16 = !DILocalVariable(name: "read1", scope: !8, file: !9, line: 3, type: !12)
+!17 = !DILocalVariable(name: "read2", scope: !8, file: !9, line: 4, type: !12)
+; CHECK: [[VAR_CHEESE]] = !DILocalVariable(name: "cheese"
+!18 = !DILocalVariable(name: "cheese", scope: !8, file: !9, line: 6, type: !12)
+; CHECK: [[VAR_A]] = !DILocalVariable(name: "a"
+!19 = !DILocalVariable(name: "a", scope: !8, file: !9, line: 7, type: !12)
+!20 = !DILocation(line: 2, scope: !8)
+!21 = !{!22, !22, i64 0}
+!22 = !{!"int", !23, i64 0}
+!23 = !{!"omnipotent char", !24, i64 0}
+!24 = !{!"Simple C++ TBAA"}
+!25 = !DILocation(line: 3, scope: !8)
+!26 = !DILocation(line: 0, scope: !8)
+!27 = !DILocation(line: 4, scope: !8)
+!28 = !DILocation(line: 6, scope: !8)
+!29 = !DILocation(line: 7, scope: !8)
+!30 = !DILocation(line: 10, scope: !31)
+!31 = distinct !DILexicalBlock(scope: !8, file: !9, line: 10)
+!32 = !DILocation(line: 10, scope: !8)
+!33 = !DILocation(line: 8, scope: !8)
+!34 = !DILocation(line: 12, scope: !35)
+!35 = distinct !DILexicalBlock(scope: !31, file: !9, line: 10)
+!36 = !DILocation(line: 13, scope: !35)
+!37 = !DILocation(line: 14, scope: !35)
+!38 = !DILocation(line: 15, scope: !35)
+!39 = !DILocation(line: 0, scope: !31)
+!40 = !DILocation(line: 20, scope: !8)
+!41 = !DILocation(line: 19, scope: !8)