[PATCH] Debug Info on Windows

[Carlo Kok]

Op 26-7-2013 20:25, David Blaikie schreef:
> On Fri, Jul 26, 2013 at 10:44 AM, Carlo Kok <ck at remobjects.com> wrote:
>> Op 26-7-2013 19:26, David Blaikie schreef:
>>
>>>>>
>>>>> Can you provide test coverage for this?
>>
>>
>>> If possible, we're trying to make a habit of including the original
>>> (C, in your case I assume) source & Clang command line (ideally
>>> produced from Clang since it's generally accessible to LLVM developers
>>> - if it's something that can only be produced by some other frontend,
>>> I'd be curious to hear about that too) in the LLVM debug info test
>>> cases if possible. (& keep the IR an exact copy of what Clang
>>> produced, rather than anything hand-reduced unless there's a good
>>> reason to do so)
>>
>>
>> Oke. I presumed that shorter was better, but sure. Fixed testcase attached.
>
> Generally shorter is better - I appreciate the effort. Still
> considering/figuring out how best to manage such test cases owing to
> how awkward it is to maintain (updating test cases when the schema
> changes, etc) the textual metadata representation of debug info.
>
>>> (oh, and your patch uses naming inconsistent with LLVM's naming
>>> convention (value should be Value))
>>
>>
>> Also updated to use V as it uses in the case before it.
>
> Sounds good
>
>> The only thing I'm
>> worried about is DIEValue::isLabel, last time I checked in some code with an
>> enum defined as part of a class I did it wrong (VC is a lot more relaxed in
>> the enum rules).
>
> Hmm, not quite sure what the concern is there, but I do have a couple
> of other questions:

Originally, scoping. In vc++ you can do enumname::element. clang doesn't 
accept that. But since this is defined in a class it should be fine.

>
> a) Eric recently checked in some changes that add a DIEString type -
> does this have any effect on your code?
> b) Could we sink this logic down into DIELabel (or
> DIEString)::EmitValue rather than having the special case up here in
> emitDIE?

DIELabel then. DIEString calls DIELabel.

> It'd still need to query the needsDwarfSectionOffsetDirective, I guess
> (I haven't sat down to figure out what that function actually means or
> how it is the right check for your situation, though, I must admit)
> but not the "isLabel" case (nor the DIELabel cast)
>
> If the code does stay where it is, the right check for the first
> condition of that' if' is probably dyn_cast<DIELabel>(V) (& saving the
> result of that cast so you don't have to cast again inside the
> conditional)

New v attached.

-------------- next part --------------
 lib/CodeGen/AsmPrinter/DIE.cpp            |  5 +++-
 test/DebugInfo/X86/coff_relative_names.ll | 40 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/lib/CodeGen/AsmPrinter/DIE.cpp b/lib/CodeGen/AsmPrinter/DIE.cpp
index 0b15412..d6ad298 100644
--- a/lib/CodeGen/AsmPrinter/DIE.cpp
+++ b/lib/CodeGen/AsmPrinter/DIE.cpp
@@ -281,7 +281,10 @@ void DIEExpr::print(raw_ostream &O) const {
 /// EmitValue - Emit label value.
 ///
 void DIELabel::EmitValue(AsmPrinter *AP, unsigned Form) const {
-  AP->EmitLabelReference(Label, SizeOf(AP, Form));
+  if (AP->MAI->needsDwarfSectionOffsetDirective()) 
+    AP->OutStreamer.EmitCOFFSecRel32(Label);
+  else
+    AP->EmitLabelReference(Label, SizeOf(AP, Form));
 }
 
 /// SizeOf - Determine size of label value in bytes.
diff --git a/test/DebugInfo/X86/coff_relative_names.ll b/test/DebugInfo/X86/coff_relative_names.ll
new file mode 100644
index 0000000..5dab619
--- /dev/null
+++ b/test/DebugInfo/X86/coff_relative_names.ll
@@ -0,0 +1,40 @@
+; RUN: llc -mtriple=i686-w64-mingw32 -filetype=asm -O0 < %s | FileCheck %s
+
+; CHECK:  	.secrel32 Linfo_string0
+; CHECK:  	.secrel32 Linfo_string1
+;
+; generated from:
+; clang -g -S -emit-llvm test.c -o test.ll
+; int main()
+; {
+; 	return 0;
+; }
+
+; ModuleID = 'test.c'
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-f80:128:128-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32-S32"
+target triple = "i686-pc-win32"
+
+; Function Attrs: nounwind
+define i32 @main() #0 {
+entry:
+  %retval = alloca i32, align 4
+  store i32 0, i32* %retval
+  ret i32 0, !dbg !10
+}
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf"="true" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9}
+
+!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.4 ", i1 false, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !""} ; [ DW_TAG_compile_unit ] [C:\Projects/test.c] [DW_LANG_C99]
+!1 = metadata !{metadata !"test.c", metadata !"C:\5CProjects"}
+!2 = metadata !{i32 0}
+!3 = metadata !{metadata !4}
+!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"main", metadata !"main", metadata !"", i32 1, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 0, i1 false, i32 ()* @main, null, null, metadata !2, i32 2} ; [ DW_TAG_subprogram ] [line 1] [def] [scope 2] [main]
+!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [C:\Projects/test.c]
+!6 = metadata !{i32 786453, i32 0, i32 0, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, i32 0} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = metadata !{metadata !8}
+!8 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
+!9 = metadata !{i32 2, metadata !"Dwarf Version", i32 3}
+!10 = metadata !{i32 3, i32 0, metadata !4, null}