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

[David Blaikie]

================
Comment at: lib/CodeGen/SelectionDAG/InstrEmitter.cpp:675
@@ -674,1 +674,3 @@
       MIB.addReg(0U);       // undef
+    else if (TargetRegisterInfo::isPhysicalRegister(I->second))
+      MIB.addReg(I->second, RegState::Debug);
----------------
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?

================
Comment at: test/DebugInfo/X86/sdagsplit-1.ll:2
@@ +1,3 @@
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump %t.o | FileCheck --check-prefix=CHECK-DWARF %s
+;
----------------
Same feedback as the other test case's RUN line.

================
Comment at: test/DebugInfo/X86/sdagsplit-1.ll:35
@@ +34,3 @@
+  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
----------------
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?

================
Comment at: test/DebugInfo/X86/sdagsplit-2.ll:1
@@ +1,2 @@
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump %t.o | FileCheck --check-prefix=CHECK-DWARF %s
----------------
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).

================
Comment at: test/DebugInfo/X86/sdagsplit-2.ll:2
@@ +1,3 @@
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: llvm-dwarfdump %t.o | FileCheck --check-prefix=CHECK-DWARF %s
+;
----------------
Why the custom check prefix rather than just the default CHECK?

================
Comment at: test/DebugInfo/X86/sdagsplit-2.ll:10
@@ +9,3 @@
+;    // Compile with -O1 -m32.
+;    long long foo (long long a, long long b, long long c)
+;    {
----------------
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?)

================
Comment at: test/DebugInfo/X86/sdagsplit-2.ll:15
@@ +14,3 @@
+;        return res;
+;      return res+b;
+;    }
----------------
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.

http://reviews.llvm.org/D4831