[llvm] r249487 - DebugInfo: Include the decl_line/decl_file in subprogram definitions if they differ from those in the declaration
Robinson, Paul via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 7 09:37:57 PDT 2015
I think that would be a mistake - the way that DW_AT_specification is …
DW_AT_specification says the completing declaration "does not need to duplicate information" which is way weaker than requiring redundant attributes to be omitted. So "mistake" seems a bit strong here.
I imagine you have a specific problem, though, with the PS4 debugger not being able to cope with this?
Honestly I have no idea what our debugger would do with this; our merge-and-test cycle is nowhere near that fast, you just committed this yesterday. ☺ Being legal syntactically, I'd expect we would cope just fine, however.
No, my suggestion is just because it looked weird to me, and as a human reading a dwarfdump it would be a double-take. But if you feel that strongly about it, well, okay.
--paulr
From: David Blaikie [mailto:dblaikie at gmail.com]
Sent: Wednesday, October 07, 2015 9:26 AM
To: Robinson, Paul
Cc: llvm-commits at lists.llvm.org; Eric Christopher
Subject: Re: [llvm] r249487 - DebugInfo: Include the decl_line/decl_file in subprogram definitions if they differ from those in the declaration
On Wed, Oct 7, 2015 at 9:17 AM, David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote:
On Wed, Oct 7, 2015 at 9:07 AM, Robinson, Paul <Paul_Robinson at playstation.sony.com<mailto:Paul_Robinson at playstation.sony.com>> wrote:
Resending because reply-all didn't cc llvm-commits...
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org<mailto:llvm-commits-bounces at lists.llvm.org>] On Behalf
> Of David Blaikie via llvm-commits
> Sent: Tuesday, October 06, 2015 5:04 PM
> To: llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
> Subject: [llvm] r249487 - DebugInfo: Include the decl_line/decl_file in
> subprogram definitions if they differ from those in the declaration
>
> Author: dblaikie
> Date: Tue Oct 6 19:04:16 2015
> New Revision: 249487
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249487&view=rev
> Log:
> DebugInfo: Include the decl_line/decl_file in subprogram definitions if
> they differ from those in the declaration
>
> This is handy for some AutoFDO stuff, and seems like a minor improvement
> to correctness (otherwise a debug info consumer might think the decl
> line/file of the def was the same as that of the declaration - though
> what a consumer might use that for, I'm not sure - maybe "list <func>"
> would've misbehaved with the old behavior?) and at a minor cost (in my
> experiment, with fission, without type units, without compression, 0.01%
> growth in debug info in the executable/objects, 0.02% growth in the .dwo
> files).
>
> Added:
> llvm/trunk/test/DebugInfo/Generic/def-line.ll
> Modified:
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> llvm/trunk/test/DebugInfo/X86/concrete_out_of_line.ll
> llvm/trunk/test/DebugInfo/X86/dwarf-public-names.ll
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=249487&r1=2494
> 86&r2=249487&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Tue Oct 6 19:04:16
> 2015
> @@ -1151,6 +1151,14 @@ bool DwarfUnit::applySubprogramDefinitio
> "definition DIE was created in "
> "getOrCreateSubprogramDIE");
> DeclLinkageName = SPDecl->getLinkageName();
> + unsigned DeclID =
> + getOrCreateSourceID(SPDecl->getFilename(), SPDecl-
> >getDirectory());
> + unsigned DefID = getOrCreateSourceID(SP->getFilename(), SP-
> >getDirectory());
> + if (DeclID != DefID)
> + addUInt(SPDie, dwarf::DW_AT_decl_file, None, DefID);
> +
> + if (SP->getLine() != SPDecl->getLine())
> + addUInt(SPDie, dwarf::DW_AT_decl_line, None, SP->getLine());
Please emit both together, not potentially just one or the other.
(Based on the Principle of Least Surprise; these attributes are
commonly considered a pair, even if it is syntactically legal to
provide only one or the other.)
I think that would be a mistake - the way that DW_AT_specification is specified is similar to entities with DW_AT_abstract_origin - the idea being that most attributes are inherited from the declaration to the definition, except where the definition overrides them. (the wording is a bit vague about which things should be considered inherited and which should not - it provides some examples of obvious things not to inherit, and then basically leaves it up to the implementation, it seems)
Oh, and GCC already does this (not sure for how long it's been doing it - I imagine for a while, given the way one might implement it to naturally collapse matching attribute values between decl and def) so there's some precedent for consumers being able to cope with it.
I imagine you have a specific problem, though, with the PS4 debugger not being able to cope with this?
Again, it's not a huge size growth, and including both or neither might reduce the abbrev growth slightly (less variety in abbrevs) so it's not a technical difficulty, just a philosophical one.
- Dave
--paulr
> }
>
> // Add function template parameters.
>
> Added: llvm/trunk/test/DebugInfo/Generic/def-line.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/DebugInfo/Generic/def-line.ll?rev=249487&view=auto
> ==========================================================================
> ====
> --- llvm/trunk/test/DebugInfo/Generic/def-line.ll (added)
> +++ llvm/trunk/test/DebugInfo/Generic/def-line.ll Tue Oct 6 19:04:16 2015
> @@ -0,0 +1,95 @@
> +; REQUIRES: object-emission
> +
> +; RUN: %llc_dwarf < %s -filetype=obj | llvm-dwarfdump -debug-dump=info -
> | FileCheck %s
> +
> +; Given the following source, ensure that the decl_line/file is correctly
> +; emitted and omitted on definitions if it mismatches/matches the
> declaration
> +
> +; struct foo {
> +; static void f1() {
> +; }
> +; static void f2();
> +; static void f3();
> +; };
> +; void foo::f2() {
> +; f1(); // just to ensure f1 is emitted
> +; }
> +; #line 1 "bar.cpp"
> +; void foo::f3() {
> +; }
> +
> +; Skip the declarations
> +; CHECK: DW_TAG_subprogram
> +; CHECK: DW_TAG_subprogram
> +; CHECK: DW_TAG_subprogram
> +
> +; CHECK: DW_TAG_subprogram
> +; CHECK-NOT: {{DW_TAG|NULL|DW_AT_decl_file}}
> +; CHECK: DW_AT_decl_line {{.*}}7
> +; CHECK-NOT: {{DW_TAG|NULL|DW_AT_decl_file}}
> +; CHECK: DW_AT_specification {{.*}}f2
> +; CHECK-NOT: {{DW_TAG|NULL|DW_AT_decl_file}}
> +
> +; CHECK: DW_TAG_subprogram
> +; CHECK-NOT: {{DW_TAG|NULL|DW_AT_decl_line|DW_AT_decl_file}}
> +; CHECK: DW_AT_specification {{.*}}f1
> +
> +; CHECK: DW_TAG_subprogram
> +; CHECK-NOT: {{DW_TAG|NULL}}
> +; CHECK: DW_AT_decl_file {{.*}}bar.cpp
> +; CHECK-NOT: {{DW_TAG|NULL}}
> +; CHECK: DW_AT_decl_line {{.*}}1
> +; CHECK-NOT: {{DW_TAG|NULL}}
> +; CHECK: DW_AT_specification {{.*}}f3
> +
> +$_ZN3foo2f1Ev = comdat any
> +
> +; Function Attrs: uwtable
> +define void @_ZN3foo2f2Ev() #0 align 2 {
> +entry:
> + call void @_ZN3foo2f1Ev(), !dbg !19
> + ret void, !dbg !20
> +}
> +
> +; Function Attrs: nounwind uwtable
> +define linkonce_odr void @_ZN3foo2f1Ev() #1 comdat align 2 {
> +entry:
> + ret void, !dbg !21
> +}
> +
> +; Function Attrs: nounwind uwtable
> +define void @_ZN3foo2f3Ev() #1 align 2 {
> +entry:
> + ret void, !dbg !22
> +}
> +
> +attributes #0 = { uwtable "disable-tail-calls"="false" "less-precise-
> fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-
> leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-
> protector-buffer-size"="8" "target-cpu"="x86-64" "target-
> features"="+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }
> +attributes #1 = { nounwind uwtable "disable-tail-calls"="false" "less-
> precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-
> elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-
> protector-buffer-size"="8" "target-cpu"="x86-64" "target-
> features"="+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" }
> +
> +!llvm.dbg.cu<http://llvm.dbg.cu> = !{!0}
> +!llvm.module.flags = !{!16, !17}
> +!llvm.ident = !{!18}
> +
> +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1,
> producer: "clang version 3.8.0 (trunk 249440) (llvm/trunk 249465)",
> isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2,
> retainedTypes: !3, subprograms: !11)
> +!1 = !DIFile(filename: "def-line.cpp", directory: "/tmp/dbginfo")
> +!2 = !{}
> +!3 = !{!4}
> +!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", file: !1,
> line: 1, size: 8, align: 8, elements: !5, identifier: "_ZTS3foo")
> +!5 = !{!6, !9, !10}
> +!6 = !DISubprogram(name: "f1", linkageName: "_ZN3foo2f1Ev", scope:
> !"_ZTS3foo", file: !1, line: 2, type: !7, isLocal: false, isDefinition:
> false, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false)
> +!7 = !DISubroutineType(types: !8)
> +!8 = !{null}
> +!9 = !DISubprogram(name: "f2", linkageName: "_ZN3foo2f2Ev", scope:
> !"_ZTS3foo", file: !1, line: 4, type: !7, isLocal: false, isDefinition:
> false, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false)
> +!10 = !DISubprogram(name: "f3", linkageName: "_ZN3foo2f3Ev", scope:
> !"_ZTS3foo", file: !1, line: 5, type: !7, isLocal: false, isDefinition:
> false, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: false)
> +!11 = !{!12, !13, !15}
> +!12 = distinct !DISubprogram(name: "f2", linkageName: "_ZN3foo2f2Ev",
> scope: !"_ZTS3foo", file: !1, line: 7, type: !7, isLocal: false,
> isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized:
> false, function: void ()* @_ZN3foo2f2Ev, declaration: !9, variables: !2)
> +!13 = distinct !DISubprogram(name: "f3", linkageName: "_ZN3foo2f3Ev",
> scope: !"_ZTS3foo", file: !14, line: 1, type: !7, isLocal: false,
> isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized:
> false, function: void ()* @_ZN3foo2f3Ev, declaration: !10, variables: !2)
> +!14 = !DIFile(filename: "bar.cpp", directory: "/tmp/dbginfo")
> +!15 = distinct !DISubprogram(name: "f1", linkageName: "_ZN3foo2f1Ev",
> scope: !"_ZTS3foo", file: !1, line: 2, type: !7, isLocal: false,
> isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized:
> false, function: void ()* @_ZN3foo2f1Ev, declaration: !6, variables: !2)
> +!16 = !{i32 2, !"Dwarf Version", i32 4}
> +!17 = !{i32 2, !"Debug Info Version", i32 3}
> +!18 = !{!"clang version 3.8.0 (trunk 249440) (llvm/trunk 249465)"}
> +!19 = !DILocation(line: 8, column: 3, scope: !12)
> +!20 = !DILocation(line: 9, column: 1, scope: !12)
> +!21 = !DILocation(line: 3, column: 3, scope: !15)
> +!22 = !DILocation(line: 2, column: 1, scope: !13)
>
> Modified: llvm/trunk/test/DebugInfo/X86/concrete_out_of_line.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/DebugInfo/X86/concrete_out_of_line.ll?rev=249487&r
> 1=249486&r2=249487&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/test/DebugInfo/X86/concrete_out_of_line.ll (original)
> +++ llvm/trunk/test/DebugInfo/X86/concrete_out_of_line.ll Tue Oct 6
> 19:04:16 2015
> @@ -18,12 +18,14 @@
> ; CHECK: DW_AT_name {{.*}} "~nsAutoRefCnt"
>
> ; CHECK: DW_TAG_subprogram
> +; CHECK-NEXT: DW_AT_decl_line {{.*}}18
> ; CHECK-NEXT: DW_AT_{{.*}}linkage_name {{.*}}D2
> ; CHECK-NEXT: DW_AT_specification {{.*}} "~nsAutoRefCnt"
> ; CHECK-NEXT: DW_AT_inline
> ; CHECK-NOT: DW_AT
> ; CHECK: DW_TAG
> ; CHECK: DW_TAG_subprogram
> +; CHECK-NEXT: DW_AT_decl_line {{.*}}18
> ; CHECK-NEXT: DW_AT_{{.*}}linkage_name {{.*}}D1
> ; CHECK-NEXT: DW_AT_specification {{.*}} "~nsAutoRefCnt"
> ; CHECK-NEXT: DW_AT_inline
>
> Modified: llvm/trunk/test/DebugInfo/X86/dwarf-public-names.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/DebugInfo/X86/dwarf-public-
> names.ll?rev=249487&r1=249486&r2=249487&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/test/DebugInfo/X86/dwarf-public-names.ll (original)
> +++ llvm/trunk/test/DebugInfo/X86/dwarf-public-names.ll Tue Oct 6
> 19:04:16 2015
> @@ -43,7 +43,7 @@
>
> ; Skip the output to the header of the pubnames section.
> ; LINUX: debug_pubnames
> -; LINUX-NEXT: unit_size = 0x00000128
> +; LINUX-NEXT: unit_size = 0x0000012a
>
> ; Check for each name in the output.
> ; LINUX-DAG: "ns"
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151007/e381402f/attachment.html>
More information about the llvm-commits
mailing list