Debug info: Teach the SDag type legalizer how to split up debug info for integer values that are split into a hi and lo part.

[aprantl]

Summary

Teach the SDag type legalizer how to split up debug info for integer values that are split into a hi and lo part.

This is yet another a split-off from http://reviews.llvm.org/D2680

[aprantl]

Closing in favor of D4831.

[dblaikie]

Closed the wrong one :) Hopefully you can reopen this one. Sorry for
all the churn/confusion/annoyance... :/

[dblaikie]

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

675

Not knowing much about any of this SelDAG code, I don't understand how this change is related to the rest of the patch - could you explain it?

test/DebugInfo/X86/sdagsplit-1.ll

2

Same feedback as the other test case's RUN line.

35

I'm a little confused as to how the use of %add comes before its definition... - but I guess that's working as intended somehow?

test/DebugInfo/X86/sdagsplit-2.ll

1

Not sure about your preference, but I usually avoid the temporary file and just pipe from one to the next:

; RUN: %llc_dwarf -filetype=obj < %s | FileCheck %s

Note also the use of %llc_dwarf (I keep forgetting why we need that, and why it's not done more consistently - but something to do with Windows support... Takumi knows more about this) and < %s, so that the output doesn't risk containing the filename of the input (which might vary depending on test configuration/directory hierarchy, and thus fail on some machines and pass on others).

2

Why the custom check prefix rather than just the default CHECK?

10

Any particular reason this needs 3 parameters and a return value? (are they testing different codepaths? If they're all testing the same one, could we just use a single parameter?)

15

I see the return value is the only difference between these two test cases. Is it relevant? (I assume so, - guess that's how you ended up with two test cases) Could it be documented in the test better?

Is the functionality that makes this test pass separate from the functionality that makes the first test pass - if so, perhaps it could be separated further. I'm still muddling through the actual source changes & trying to figure out how they're all dependent/related.

[aprantl]

I _think_ I just messed up the comment.
I closed http://reviews.llvm.org/D4831 and http://reviews.llvm.org/D4832 is still open.

[dblaikie]

Ah, right -it's all getting jumbled up in my email because it has the
same subject line (Go GMail!)

I sent review to the wrong one - will rewrite/repost shortly. Blagle.

[aprantl]

Is there any way to close a phabricator revision without accepting it?

[dblaikie]

Yep, you "Abandon" it (if you own the CR you can choose "Abandon" as one of
the actions).

[aprantl]

Yes, but now that I abandoned it, it's still lying around. Can't I close or delete it now?

[dblaikie]

Ah. Not sure - it can be "closed". I'm not sure if "closed" implies
successful resolution or not... doesn't seem like a bad thing to close a
revision even if it was never committed, but I don't really know.

[probinson]

Yes, but now that I abandoned it, it's still lying around. Can't I close
or delete it now?

http://reviews.llvm.org/D4831

When you figure this out, please make an appropriate note in the Phab doc.
http://llvm.org/docs/Phabricator.html

Thanks,
--paulr
who learned that Phab hated him long ago, but would like to get along with it better

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

     DenseMap<SDValue, unsigned>::iterator I = VRBaseMap.find(Op);
     if (I==VRBaseMap.end())
       MIB.addReg(0U);       // undef
+    else if (TargetRegisterInfo::isPhysicalRegister(I->second))
+      MIB.addReg(I->second, RegState::Debug);
     else
       AddOperand(MIB, Op, (*MIB).getNumOperands(), &II, VRBaseMap,
                  /*IsDebug=*/true, /*IsClone=*/false, /*IsCloned=*/false);

lib/CodeGen/SelectionDAG/LegalizeTypes.cpp

 //===----------------------------------------------------------------------===//
 
 #include "LegalizeTypes.h"
+#include "SDNodeDbgValue.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/IR/CallingConv.h"
+#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"

   Hi = Entry.second;
 }
 
+/// TransferDbgValues - Transfer SDDbgValues.
+static void TransferDbgValues(SelectionDAG &DAG, DIBuilder &DIB,
+                              SDValue From, SDValue To,
+                              unsigned OffsetInBits) {
+  SDNode *FromNode = From.getNode();
+  SDNode *ToNode = To.getNode();
+  assert(FromNode != ToNode);
+  ArrayRef<SDDbgValue *> DVs = DAG.GetDbgValues(FromNode);
+  for (ArrayRef<SDDbgValue *>::iterator I = DVs.begin(), E = DVs.end();
+       I != E; ++I) {
+    SDDbgValue *Dbg = *I;
+
+    if (Dbg->getKind() == SDDbgValue::SDNODE) {
+      DIVariable Var(Dbg->getMDPtr());
+      const unsigned SizeOfByte = 8;
+      unsigned PieceSizeInBits = To.getValueSizeInBits();
+      assert(OffsetInBits % SizeOfByte == 0
+             && "sub-byte types are not supported");
+      assert(PieceSizeInBits % SizeOfByte == 0
+             && "sub-byte types are not supported");
+      DIVariable Piece = DIB.createVariablePiece(Var, OffsetInBits/SizeOfByte,
+                                                 PieceSizeInBits/SizeOfByte);
+      SDDbgValue *Clone = DAG.getDbgValue(&*Piece, ToNode, To.getResNo(),
+                                          Dbg->isIndirect(),
+                                          Dbg->getOffset(), Dbg->getDebugLoc(),
+                                          Dbg->getOrder());
+      Dbg->setIsInvalidated();
+      DAG.AddDbgValue(Clone, ToNode, false);
+    }
+  }
+}
+
 void DAGTypeLegalizer::SetExpandedInteger(SDValue Op, SDValue Lo,
                                           SDValue Hi) {
   assert(Lo.getValueType() ==

   AnalyzeNewValue(Lo);
   AnalyzeNewValue(Hi);
 
+  // Transfer debug values.
+  const Module *M = DAG.getMachineFunction().getMMI().getModule();
+  DIBuilder DIB(*const_cast<Module*>(M));
+  TransferDbgValues(DAG, DIB, Op, Lo, 0);
+  TransferDbgValues(DAG, DIB, Op, Hi, Lo.getValueSizeInBits());
+
   // Remember that this is the result of the node.
   std::pair<SDValue, SDValue> &Entry = ExpandedIntegers[Op];
   assert(!Entry.first.getNode() && "Node already expanded");

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

        I != E; ++I) {
     SDDbgValue *Dbg = *I;
     if (Dbg->getKind() == SDDbgValue::SDNODE) {
-      SDDbgValue *Clone = getDbgValue(Dbg->getMDPtr(), ToNode, To.getResNo(),
+      SDDbgValue *Clone = getDbgValue(Dbg->getMDPtr(), ToNode, Dbg->getResNo(),
               Dbg->isIndirect(),
                                       Dbg->getOffset(), Dbg->getDebugLoc(),
                                       Dbg->getOrder());

test/DebugInfo/X86/sdagsplit-1.ll

+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump %t.o | FileCheck --check-prefix=CHECK-DWARF %s
+;
+; rdar://problem/15928306
+;
+; Test that we can emit debug info for large values that are split
+; up across multiple registers by the SelectionDAG type legalizer.
+;
+;    // Compile with -O1 -m32.
+;    long long foo (long long a, long long b, long long c)
+;    {
+;      long long res = c+1;
+;      if ( a == b )
+;        return res;
+;      return 0;
+;    }
+;
+; CHECK-DWARF: DW_AT_location [DW_FORM_data4]        ([[LOC:.*]])
+; CHECK-DWARF-NEXT:  DW_AT_name {{.*}}"res"
+; CHECK-DWARF: .debug_loc
+; CHECK-DWARF: [[LOC]]:
+; eax, piece 0x00000004, edx, piece 0x00000004
+; CHECK-DWARF:   Location description: 50 93 04 52 93 04
+
+; ModuleID = 'sdagsplit-1.c'
+target datalayout = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128"
+target triple = "i386-apple-macosx10.9.0"
+
+; Function Attrs: nounwind readnone ssp
+define i64 @foo(i64 %a, i64 %b, i64 %c) #0 {
+entry:
+  tail call void @llvm.dbg.value(metadata !{i64 %a}, i64 0, metadata !10), !dbg !17
+  tail call void @llvm.dbg.value(metadata !{i64 %b}, i64 0, metadata !11), !dbg !18
+  tail call void @llvm.dbg.value(metadata !{i64 %c}, i64 0, metadata !12), !dbg !19
+  tail call void @llvm.dbg.value(metadata !{i64 %add}, i64 0, metadata !13), !dbg !20
+  %cmp = icmp eq i64 %a, %b, !dbg !21
+  %add = add nsw i64 %c, 1, !dbg !23
+  %retval.0 = select i1 %cmp, i64 %add, i64 0, !dbg !21
+  ret i64 %retval.0, !dbg !24
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata) #1
+
+attributes #0 = { nounwind readnone ssp }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!14, !15}
+!llvm.ident = !{!16}
+
+!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.6.0 ", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [/sdagsplit-1.c] [DW_LANG_C99]
+!1 = metadata !{metadata !"sdagsplit-1.c", metadata !""}
+!2 = metadata !{}
+!3 = metadata !{metadata !4}
+!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"foo", metadata !"foo", metadata !"", i32 2, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, i64 (i64, i64, i64)* @foo, null, null, metadata !9, i32 3} ; [ DW_TAG_subprogram ] [line 2] [def] [scope 3] [foo]
+!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [/sdagsplit-1.c]
+!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = metadata !{metadata !8, metadata !8, metadata !8, metadata !8}
+!8 = metadata !{i32 786468, null, null, metadata !"long long int", i32 0, i64 64, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [long long int] [line 0, size 64, align 32, offset 0, enc DW_ATE_signed]
+!9 = metadata !{metadata !10, metadata !11, metadata !12, metadata !13}
+!10 = metadata !{i32 786689, metadata !4, metadata !"a", metadata !5, i32 16777218, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [a] [line 2]
+!11 = metadata !{i32 786689, metadata !4, metadata !"b", metadata !5, i32 33554434, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [b] [line 2]
+!12 = metadata !{i32 786689, metadata !4, metadata !"c", metadata !5, i32 50331650, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [c] [line 2]
+!13 = metadata !{i32 786688, metadata !4, metadata !"res", metadata !5, i32 4, metadata !8, i32 0, i32 0} ; [ DW_TAG_auto_variable ] [res] [line 4]
+!14 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}
+!15 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}
+!16 = metadata !{metadata !"clang version 3.6.0 "}
+!17 = metadata !{i32 2, i32 26, metadata !4, null}
+!18 = metadata !{i32 2, i32 39, metadata !4, null}
+!19 = metadata !{i32 2, i32 52, metadata !4, null}
+!20 = metadata !{i32 4, i32 13, metadata !4, null}
+!21 = metadata !{i32 5, i32 8, metadata !22, null}
+!22 = metadata !{i32 786443, metadata !1, metadata !4, i32 5, i32 8, i32 0, i32 0} ; [ DW_TAG_lexical_block ] [/sdagsplit-1.c]
+!23 = metadata !{i32 4, i32 3, metadata !4, null}
+!24 = metadata !{i32 8, i32 1, metadata !4, null} ; [ DW_TAG_imported_declaration ]

test/DebugInfo/X86/sdagsplit-2.ll

+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump %t.o | FileCheck --check-prefix=CHECK-DWARF %s
+;
+; rdar://problem/15928306
+;
+; Test that we can emit debug info for large values that are split
+; up across multiple registers by the SelectionDAG type legalizer.
+;
+;    // Compile with -O1 -m32.
+;    long long foo (long long a, long long b, long long c)
+;    {
+;      long long res = c+1;
+;      if ( a == b )
+;        return res;
+;      return res+b;
+;    }
+;
+; CHECK-DWARF:  "res"
+;  piece 0x00000004, edx, piece 0x00000004
+; CHECK-DWARF:  93 04 52 93 04
+
+; ModuleID = 'sdagsplit-2.c'
+target datalayout = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128"
+target triple = "i386-apple-macosx10.9.0"
+
+; Function Attrs: nounwind readnone ssp
+define i64 @foo(i64 %a, i64 %b, i64 %c) #0 {
+entry:
+  tail call void @llvm.dbg.value(metadata !{i64 %a}, i64 0, metadata !10), !dbg !17
+  tail call void @llvm.dbg.value(metadata !{i64 %b}, i64 0, metadata !11), !dbg !18
+  tail call void @llvm.dbg.value(metadata !{i64 %c}, i64 0, metadata !12), !dbg !19
+  %add = add nsw i64 %c, 1, !dbg !20
+  tail call void @llvm.dbg.value(metadata !{i64 %add}, i64 0, metadata !13), !dbg !21
+  %cmp = icmp eq i64 %a, %b, !dbg !22
+  %add1 = select i1 %cmp, i64 0, i64 %b, !dbg !22
+  %retval.0 = add nsw i64 %add, %add1, !dbg !22
+  ret i64 %retval.0, !dbg !24
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata) #1
+
+attributes #0 = { nounwind readnone ssp }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!14, !15}
+!llvm.ident = !{!16}
+
+!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.6.0 ", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [/sdagsplit-2.c] [DW_LANG_C99]
+!1 = metadata !{metadata !"sdagsplit-2.c", metadata !""}
+!2 = metadata !{}
+!3 = metadata !{metadata !4}
+!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"foo", metadata !"foo", metadata !"", i32 2, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, i64 (i64, i64, i64)* @foo, null, null, metadata !9, i32 3} ; [ DW_TAG_subprogram ] [line 2] [def] [scope 3] [foo]
+!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [/sdagsplit-2.c]
+!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = metadata !{metadata !8, metadata !8, metadata !8, metadata !8}
+!8 = metadata !{i32 786468, null, null, metadata !"long long int", i32 0, i64 64, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [long long int] [line 0, size 64, align 32, offset 0, enc DW_ATE_signed]
+!9 = metadata !{metadata !10, metadata !11, metadata !12, metadata !13}
+!10 = metadata !{i32 786689, metadata !4, metadata !"a", metadata !5, i32 16777218, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [a] [line 2]
+!11 = metadata !{i32 786689, metadata !4, metadata !"b", metadata !5, i32 33554434, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [b] [line 2]
+!12 = metadata !{i32 786689, metadata !4, metadata !"c", metadata !5, i32 50331650, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [c] [line 2]
+!13 = metadata !{i32 786688, metadata !4, metadata !"res", metadata !5, i32 4, metadata !8, i32 0, i32 0} ; [ DW_TAG_auto_variable ] [res] [line 4]
+!14 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}
+!15 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}
+!16 = metadata !{metadata !"clang version 3.6.0 "}
+!17 = metadata !{i32 2, i32 26, metadata !4, null}
+!18 = metadata !{i32 2, i32 39, metadata !4, null}
+!19 = metadata !{i32 2, i32 52, metadata !4, null}
+!20 = metadata !{i32 4, i32 4, metadata !4, null}
+!21 = metadata !{i32 4, i32 14, metadata !4, null}
+!22 = metadata !{i32 5, i32 9, metadata !23, null}
+!23 = metadata !{i32 786443, metadata !1, metadata !4, i32 5, i32 9, i32 0, i32 0} ; [ DW_TAG_lexical_block ] [/sdagsplit-2.c]
+!24 = metadata !{i32 8, i32 1, metadata !4, null} ; [ DW_TAG_imported_declaration ]