[SelectionDAG] Transfer DbgValues when integer operations are promoted

[asmith]

Summary

DbgValue nodes were not transferred when integer DAG nodes were promoted. For example, if an i32 add node was promoted to an i64 add node by DAGTypeLegalizer::PromoteIntegerResult(), its DbgValue node was not transferred to the new node. The simple fix is to update SetPromotedInteger() to transfer DbgValues.

Add AArch64/dbg-value-i8.ll to test this change and fix ARM/debug-info-d16-reg.ll which had the wrong DILocalVariable nodes with arg numbers even though they are not for function parameters.

Patch by Se Jong Oh!

[aprantl]

Could you please add a test case for this?

[asmith]

This is to fix DebugInfo/Generic/linear-dbg-value.ll and won't repro on an in-tree target.

Is there a way to write a test that will force selection dag to promote an integer?

[vsk]

In D44546#1039654, @asmith wrote:

This is to fix DebugInfo/Generic/linear-dbg-value.ll and won't repro on an in-tree target.

Is there a way to write a test that will force selection dag to promote an integer?

We can probably find a test by adding an assertion that catches interesting input, and throwing lots of debug info at the backend. E.g:

diff --git a/include/llvm/CodeGen/SelectionDAG.h b/include/llvm/CodeGen/SelectionDAG.h
index 0df75cb0b1d..5c2fc3bee5e 100644
--- a/include/llvm/CodeGen/SelectionDAG.h
+++ b/include/llvm/CodeGen/SelectionDAG.h
@@ -1228,7 +1228,7 @@ public:
   /// Transfer debug values from one node to another, while optionally
   /// generating fragment expressions for split-up values. If \p InvalidateDbg
   /// is set, debug values are invalidated after they are transferred.
-  void transferDbgValues(SDValue From, SDValue To, unsigned OffsetInBits = 0,
+  bool transferDbgValues(SDValue From, SDValue To, unsigned OffsetInBits = 0,
                          unsigned SizeInBits = 0, bool InvalidateDbg = true);
 
   /// Remove the specified node from the system. If any of its
diff --git a/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp b/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
index 4438ee7878b..03eab5cd94e 100644
--- a/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
+++ b/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
@@ -776,6 +776,9 @@ void DAGTypeLegalizer::SetPromotedInteger(SDValue Op, SDValue Result) {
   SDValue &OpEntry = PromotedIntegers[Op];
   assert(!OpEntry.getNode() && "Node is already promoted!");
   OpEntry = Result;
+
+  bool FoundIt = DAG.transferDbgValues(Op, Result);
+  assert(!FoundIt && "Found a test case");
 }
 
 void DAGTypeLegalizer::SetSoftenedFloat(SDValue Op, SDValue Result) {
diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index aac79075e6e..ea6e151dad2 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -7187,7 +7187,7 @@ SDDbgValue *SelectionDAG::getFrameIndexDbgValue(DIVariable *Var,
   return new (DbgInfo->getAlloc()) SDDbgValue(Var, Expr, FI, DL, O);
 }
 
-void SelectionDAG::transferDbgValues(SDValue From, SDValue To,
+bool SelectionDAG::transferDbgValues(SDValue From, SDValue To,
                                      unsigned OffsetInBits, unsigned SizeInBits,
                                      bool InvalidateDbg) {
   SDNode *FromNode = From.getNode();
@@ -7198,10 +7198,10 @@ void SelectionDAG::transferDbgValues(SDValue From, SDValue To,
   // TODO: assert(From != To && "Redundant dbg value transfer");
   // TODO: assert(FromNode != ToNode && "Intranode dbg value transfer");
   if (From == To || FromNode == ToNode)
-    return;
+    return false;
 
   if (!FromNode->getHasDebugValue())
-    return;
+    return false;
 
   SmallVector<SDDbgValue *, 2> ClonedDVs;
   for (SDDbgValue *Dbg : GetDbgValues(FromNode)) {
@@ -7242,6 +7242,7 @@ void SelectionDAG::transferDbgValues(SDValue From, SDValue To,
 
   for (SDDbgValue *Dbg : ClonedDVs)
     AddDbgValue(Dbg, ToNode, false);
+  return ClonedDVs.size();
 }
 
 void SelectionDAG::salvageDebugInfo(SDNode &N) {

Here's a test that trips the assert:

$ ./bin/llvm-lit /Users/vsk/src/llvm.org-debugify/llvm/test/CodeGen/ARM/debug-info-d16-reg.ll -a
Assertion failed: (!FoundIt && "Found a test case"), function SetPromotedInteger

You could probably modify this for your purposes.

[asmith]

Thanks, that's helpful.

[aprantl]

test/DebugInfo/AArch64/dbg-value-i8.ll

8 ↗

(On Diff #138790)

Can you run llc as llc -stop-after=livedebugvariables and check for a DBG_VALUE instead? This should make the test more robust.

29 ↗

(On Diff #138790)

Please remove all attributes that are not necessary to reproduce the test. (Usually everything in quotes).

[JDevlieghere]

Unless Adrian has any more remarks, this LGTM. Thank you!

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

void DAGTypeLegalizer::SetPromotedInteger(SDValue Op, SDValue Result) {
   assert(Result.getValueType() ==
          TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
          "Invalid type for promoted integer");
   AnalyzeNewValue(Result);
 
   SDValue &OpEntry = PromotedIntegers[Op];
   assert(!OpEntry.getNode() && "Node is already promoted!");
   OpEntry = Result;
+
+  DAG.transferDbgValues(Op, Result);
 }
 
 void DAGTypeLegalizer::SetSoftenedFloat(SDValue Op, SDValue Result) {
   // f128 of x86_64 could be kept in SSE registers,
   // but sometimes softened to i128.
   assert((Result.getValueType() ==
           TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) ||
           Op.getValueType() ==

llvm/trunk/test/CodeGen/ARM/debug-info-d16-reg.ll

 !15 = !DIBasicType(tag: DW_TAG_base_type, name: "char", size: 8, align: 8, encoding: DW_ATE_signed_char)
 !16 = !DILocalVariable(name: "ptr", line: 11, arg: 1, scope: !0, file: !1, type: !6)
 !17 = !DILocalVariable(name: "val", line: 11, arg: 2, scope: !0, file: !1, type: !7)
 !18 = !DILocalVariable(name: "c", line: 11, arg: 3, scope: !0, file: !1, type: !8)
 !19 = !DILocalVariable(name: "ptr", line: 4, arg: 1, scope: !9, file: !1, type: !6)
 !20 = !DILocalVariable(name: "val", line: 4, arg: 2, scope: !9, file: !1, type: !7)
 !21 = !DILocalVariable(name: "c", line: 4, arg: 3, scope: !9, file: !1, type: !8)
 
-!49 = !DILocalVariable(name: "ptr", line: 4, arg: 1, scope: !9, file: !1, type: !6)
-!50 = !DILocalVariable(name: "val", line: 4, arg: 2, scope: !9, file: !1, type: !7)
-!51 = !DILocalVariable(name: "c", line: 4, arg: 2, scope: !9, file: !1, type: !8)
+!49 = !DILocalVariable(name: "ptr", line: 4, scope: !10, file: !1, type: !6)
+!50 = !DILocalVariable(name: "val", line: 4, scope: !10, file: !1, type: !7)
+!51 = !DILocalVariable(name: "c", line: 4, scope: !10, file: !1, type: !8)
 
 !22 = !DILocalVariable(name: "argc", line: 17, arg: 1, scope: !10, file: !1, type: !5)
 !23 = !DILocalVariable(name: "argv", line: 17, arg: 2, scope: !10, file: !1, type: !13)
 !24 = !DILocalVariable(name: "dval", line: 19, scope: !25, file: !1, type: !7)
 !25 = distinct !DILexicalBlock(line: 18, column: 0, file: !46, scope: !10)
 !26 = !DILocation(line: 4, scope: !9)
 !27 = !DILocation(line: 6, scope: !28)
 !28 = distinct !DILexicalBlock(line: 5, column: 0, file: !46, scope: !9)
 !29 = !DILocation(line: 7, scope: !28)
 !30 = !DILocation(line: 11, scope: !0)
 !31 = !DILocation(line: 13, scope: !32)
 !32 = distinct !DILexicalBlock(line: 12, column: 0, file: !46, scope: !0)
 !33 = !DILocation(line: 14, scope: !32)
 !34 = !DILocation(line: 17, scope: !10)
 !35 = !DILocation(line: 19, scope: !25)
 !36 = !DILocation(line: 20, scope: !25)
 !37 = !DILocation(line: 21, scope: !25)
-!38 = !DILocation(line: 4, scope: !9, inlinedAt: !37)
+!38 = !DILocation(line: 4, scope: !10, inlinedAt: !37)
 !39 = !DILocation(line: 6, scope: !28, inlinedAt: !37)
 !40 = !DILocation(line: 22, scope: !25)
 !41 = !DILocation(line: 23, scope: !25)
 !43 = !{!16, !17, !18}
 !44 = !{!19, !20, !21}
 !45 = !{!22, !23, !24}
 !46 = !DIFile(filename: "a.c", directory: "/tmp/")
 !47 = !{}
 !48 = !{i32 1, !"Debug Info Version", i32 3}

llvm/trunk/test/DebugInfo/AArch64/dbg-value-i8.ll

+; RUN: llc -stop-after=livedebugvars < %s | FileCheck %s
+; ModuleID = 'debug_value-i8.bc'
+source_filename = "debug_value.c"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64"
+
+; When ISel promotes the result of i8 add, the attached dbg.value shouldn't be dropped.
+; CHECK: DBG_VALUE
+; Function Attrs: nounwind
+define dso_local i32 @bar(i32*, i32) local_unnamed_addr #0 !dbg !10 {
+  %3 = trunc i32 %1 to i8, !dbg !20
+  %4 = add i8 %3, 97, !dbg !20
+  call void @llvm.dbg.value(metadata i8 %4, metadata !17, metadata !DIExpression()), !dbg !21
+  %5 = zext i8 %4 to i32, !dbg !22
+  tail call void @foo1(i32* %0, i32 %5) #2, !dbg !23
+  tail call void @foo2(i32* %0, i8 %4) #2, !dbg !24
+  ret i32 undef, !dbg !25
+}
+
+declare dso_local void @foo1(i32*, i32) local_unnamed_addr 
+
+declare dso_local void @foo2(i32*, i8) local_unnamed_addr 
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind readnone speculatable }
+attributes #2 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 7.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3)
+!1 = !DIFile(filename: "debug_value.c", directory: "./")
+!2 = !{}
+!3 = !{!4, !5}
+!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_unsigned_char)
+!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!6 = !{i32 2, !"Dwarf Version", i32 4}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{i32 1, !"wchar_size", i32 4}
+!9 = !{!"clang version 7.0.0"}
+!10 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 16, type: !11, isLocal: false, isDefinition: true, scopeLine: 16, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!5, !13, !5}
+!13 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !5, size: 64)
+!14 = !{!15, !16, !17}
+!15 = !DILocalVariable(name: "a", arg: 1, scope: !10, file: !1, line: 16, type: !13)
+!16 = !DILocalVariable(name: "b", arg: 2, scope: !10, file: !1, line: 16, type: !5)
+!17 = !DILocalVariable(name: "t", scope: !10, file: !1, line: 17, type: !4)
+!18 = !DILocation(line: 16, column: 14, scope: !10)
+!19 = !DILocation(line: 16, column: 21, scope: !10)
+!20 = !DILocation(line: 17, column: 11, scope: !10)
+!21 = !DILocation(line: 17, column: 7, scope: !10)
+!22 = !DILocation(line: 18, column: 10, scope: !10)
+!23 = !DILocation(line: 18, column: 2, scope: !10)
+!24 = !DILocation(line: 19, column: 2, scope: !10)
+!25 = !DILocation(line: 20, column: 1, scope: !10)