[llvm] r249487 - DebugInfo: Include the decl_line/decl_file in subprogram definitions if they differ from those in the declaration

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 7 09:46:39 PDT 2015


On Wed, Oct 7, 2015 at 9:37 AM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

> 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.
>

Sorry, yes, poor phrasing - by 'mistake' I didn't mean "in violation of the
DWARF spec" just unnecessary and verbose/extra data.


>
>
> 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. J
>

Ah, rightio


>   Being legal syntactically, I'd expect we would cope just fine, however.
>

Well, I could imagine a debugger not being implemented to do the
DW_AT_specification fallback - but instead look directly for both
attributes, so I imagine a debugger/DWARF consumer /could/ trip over this,
though I'd be a little surprised.


> No, my suggestion is just because it looked weird to me, and as a human
> reading a dwarfdump it would be a double-take.
>

Ah, OK. Yeah, it is a bit quirky, but I think the analogy with
DW_AT_abstract_origin's a pretty good one - just expect that anything you
don't see here is in the other DIE, etc.


> 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> wrote:
>
>
>
>
>
> On Wed, Oct 7, 2015 at 9:07 AM, Robinson, Paul <
> 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] On
> Behalf
> > Of David Blaikie via llvm-commits
> > Sent: Tuesday, October 06, 2015 5:04 PM
> > To: 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 = !{!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
> > 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/55635386/attachment.html>


More information about the llvm-commits mailing list