Djordje Todorovic via llvm-dev
2020-Feb-10 11:41 UTC
[llvm-dev] Enabling debug entry value production by default
Hi, Thanks you all for the collaboration! :) Paul,> This is not how tuning-controlled features are supposed to work. I will comment on the review.I see, I am working on addressing the comments from the [1]. I will update the diff asap. Thanks. Vedant, There are no entry values generated at -O0 level, but I will add a test case for it. Thanks. Best regards, Djordje On 8.2.20. 02:41, Robinson, Paul via llvm-dev wrote:> This is not how tuning-controlled features are supposed to work. I will comment on the review. > > --paulr > > > > *From:* vsk at apple.com <vsk at apple.com> *On Behalf Of * Vedant Kumar > *Sent:* Friday, February 7, 2020 3:26 PM > *To:* Robinson, Paul <paul.robinson at sony.com> > *Cc:* David Blaikie <dblaikie at gmail.com>; nikola.prica at rt-rk.com; ibaev at cisco.com; asowda at cisco.com; llvm-dev at lists.llvm.org > *Subject:* Re: [llvm-dev] Enabling debug entry value production by default > > > > The actual DWARF emission for call site parameters is gated inside of DwarfDebug::constructCallSiteEntryDIEs by `tuneForGDB() || tuneForLLDB()`. > > > > However, we are creating+updating CallSiteInfo (basically, in-memory only bookkeeping used by the backend to keep track of call sites) even when the debugger tuning is set to the Sony debugger. If this creates problems, feel free to file a bug and I'll address it. However, there's an upside to maintaining all this CallSiteInfo even if TAG_call_site_parameters won't be emitted. The backend asserts whenever a MachineInstr carrying CallSiteInfo is destroyed without the call information being updated. This provides useful test coverage. > > > > vedant > > > > On Feb 7, 2020, at 2:50 PM, Robinson, Paul <paul.robinson at sony.com <mailto:paul.robinson at sony.com>> wrote: > > > > I’ve asked someone to check with our (Sony) debugger people, likely hear back early next week. I’d say proceed with having it default on/off based on tuning as you described, and if my guys want to see it turned on, we can fix it later. I have to say, a quick skim through D73534 made me think it’s not controlled that way right now, is that still a TODO? > > Thanks for all the work to support this feature! > > --paulr > > > > *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org <mailto:llvm-dev-bounces at lists.llvm.org>> *On Behalf Of *Vedant Kumar via llvm-dev > *Sent:* Friday, February 7, 2020 2:04 PM > *To:* David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> > *Cc:* llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>; nikola.prica at rt-rk.com <mailto:nikola.prica at rt-rk.com>; ibaev at cisco.com <mailto:ibaev at cisco.com>; asowda at cisco.com <mailto:asowda at cisco.com> > *Subject:* Re: [llvm-dev] Enabling debug entry value production by default > > > > Yep, TAG_call_site_parameter and its children shouldn't require any extra relocations. Thanks! > > > > vedant > > > > > On Feb 7, 2020, at 2:01 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote: > > > > For that sort of small growth, if it doesn't add more relocations (I think the call sites need them (but they're already emitted/that's not what we're discussing enabling here), but call value descriptions don't), sounds good to me :) > > > > On Fri, Feb 7, 2020 at 12:04 PM Vedant Kumar <vedant_kumar at apple.com <mailto:vedant_kumar at apple.com>> wrote: > > Hi all, > > > > I think we've reached a state where we're ready to enable debug entry value production by default for the x86_64, ARM, and AArch64 targets. For context, this is a debug info feature that allows debuggers to recover the value of unmodified optimized-out parameters by 'going up' a stack frame and interpreting spilled values, constants, etc. to work out what was passed to the callee. There's a nice write-up about the impact of this feature in [3]. A big thank you to all of contributors who've helped us get here (I've tried to CC everyone, sincere apologies if I've accidentally left you out). > > > > I believe Djordje and I have shown that there isn't an outsized binary size or compile time impact of enabling this feature. The aggregate size of DWARF in a stage2 x86_64 RelWithDebInfo clang build grows by under 0.2%, and we didn't find a significant compile time impact. We've shared CTMark results (both compile-time & DWARF section size numbers), as well as a size analysis of stage2 builds of {llc, clang, lldb} in [1]. > > > > Djordje has also shared a nice location statistics comparison chart in [1], showing increased variable availability with entry values enabled. > > > > For my part, I instrumented lldb's DWARFExpression::Evaluate method to log instances in which a variable location is unavailable. I then used a script [4] to single-step through a stage2 RelWithDebInfo+entryvals clang binary as it was compiling sqlite3.c, printing local variables at each stop. Without entry values, lldb reported "variable not available" 883,918 times. With entry values lldb reported "variable not available" 687,899 times (a 22% drop). > > > > Does anyone have concerns about enabling this feature? > > > > thanks, > > vedant > > > > [1] [DebugInfo] Enable the debug entry values feature by default <https://reviews.llvm.org/D73534> > > [2] [CallSiteInfo] Fix the assertions regarding updating the CallSiteInfo <https://reviews.llvm.org/D73700>, [CallSiteInfo] Use the isCandidateForCallSiteEntry() when update the cs info <https://reviews.llvm.org/D74122> > > [3] https://robert.ocallahan.org/2018/11/comparing-quality-of-debug-information.html > > [4] https://gist.github.com/vedantk/7602b20a9e1d44c42d32dcca36591cc0 > > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >
Djordje Todorovic via llvm-dev
2020-Feb-20 09:31 UTC
[llvm-dev] Enabling debug entry value production by default
Hi, This has been officially enabled by default. Thank you all, Djordje On 10.2.20. 12:41, Djordje Todorovic wrote:> Hi, > > Thanks you all for the collaboration! :) > > Paul, > >> This is not how tuning-controlled features are supposed to work. I will comment on the review. > I see, I am working on addressing the comments from the [1]. I will update the diff asap. Thanks. > > Vedant, > > There are no entry values generated at -O0 level, but I will add a test case for it. Thanks. > > Best regards, > Djordje > > On 8.2.20. 02:41, Robinson, Paul via llvm-dev wrote: >> This is not how tuning-controlled features are supposed to work. I will comment on the review. >> >> --paulr >> >> >> >> *From:* vsk at apple.com <vsk at apple.com> *On Behalf Of * Vedant Kumar >> *Sent:* Friday, February 7, 2020 3:26 PM >> *To:* Robinson, Paul <paul.robinson at sony.com> >> *Cc:* David Blaikie <dblaikie at gmail.com>; nikola.prica at rt-rk.com; ibaev at cisco.com; asowda at cisco.com; llvm-dev at lists.llvm.org >> *Subject:* Re: [llvm-dev] Enabling debug entry value production by default >> >> >> >> The actual DWARF emission for call site parameters is gated inside of DwarfDebug::constructCallSiteEntryDIEs by `tuneForGDB() || tuneForLLDB()`. >> >> >> >> However, we are creating+updating CallSiteInfo (basically, in-memory only bookkeeping used by the backend to keep track of call sites) even when the debugger tuning is set to the Sony debugger. If this creates problems, feel free to file a bug and I'll address it. However, there's an upside to maintaining all this CallSiteInfo even if TAG_call_site_parameters won't be emitted. The backend asserts whenever a MachineInstr carrying CallSiteInfo is destroyed without the call information being updated. This provides useful test coverage. >> >> >> >> vedant >> >> >> >> On Feb 7, 2020, at 2:50 PM, Robinson, Paul <paul.robinson at sony.com <mailto:paul.robinson at sony.com>> wrote: >> >> >> >> I’ve asked someone to check with our (Sony) debugger people, likely hear back early next week. I’d say proceed with having it default on/off based on tuning as you described, and if my guys want to see it turned on, we can fix it later. I have to say, a quick skim through D73534 made me think it’s not controlled that way right now, is that still a TODO? >> >> Thanks for all the work to support this feature! >> >> --paulr >> >> >> >> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org <mailto:llvm-dev-bounces at lists.llvm.org>> *On Behalf Of *Vedant Kumar via llvm-dev >> *Sent:* Friday, February 7, 2020 2:04 PM >> *To:* David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> >> *Cc:* llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>; nikola.prica at rt-rk.com <mailto:nikola.prica at rt-rk.com>; ibaev at cisco.com <mailto:ibaev at cisco.com>; asowda at cisco.com <mailto:asowda at cisco.com> >> *Subject:* Re: [llvm-dev] Enabling debug entry value production by default >> >> >> >> Yep, TAG_call_site_parameter and its children shouldn't require any extra relocations. Thanks! >> >> >> >> vedant >> >> >> >> >> On Feb 7, 2020, at 2:01 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote: >> >> >> >> For that sort of small growth, if it doesn't add more relocations (I think the call sites need them (but they're already emitted/that's not what we're discussing enabling here), but call value descriptions don't), sounds good to me :) >> >> >> >> On Fri, Feb 7, 2020 at 12:04 PM Vedant Kumar <vedant_kumar at apple.com <mailto:vedant_kumar at apple.com>> wrote: >> >> Hi all, >> >> >> >> I think we've reached a state where we're ready to enable debug entry value production by default for the x86_64, ARM, and AArch64 targets. For context, this is a debug info feature that allows debuggers to recover the value of unmodified optimized-out parameters by 'going up' a stack frame and interpreting spilled values, constants, etc. to work out what was passed to the callee. There's a nice write-up about the impact of this feature in [3]. A big thank you to all of contributors who've helped us get here (I've tried to CC everyone, sincere apologies if I've accidentally left you out). >> >> >> >> I believe Djordje and I have shown that there isn't an outsized binary size or compile time impact of enabling this feature. The aggregate size of DWARF in a stage2 x86_64 RelWithDebInfo clang build grows by under 0.2%, and we didn't find a significant compile time impact. We've shared CTMark results (both compile-time & DWARF section size numbers), as well as a size analysis of stage2 builds of {llc, clang, lldb} in [1]. >> >> >> >> Djordje has also shared a nice location statistics comparison chart in [1], showing increased variable availability with entry values enabled. >> >> >> >> For my part, I instrumented lldb's DWARFExpression::Evaluate method to log instances in which a variable location is unavailable. I then used a script [4] to single-step through a stage2 RelWithDebInfo+entryvals clang binary as it was compiling sqlite3.c, printing local variables at each stop. Without entry values, lldb reported "variable not available" 883,918 times. With entry values lldb reported "variable not available" 687,899 times (a 22% drop). >> >> >> >> Does anyone have concerns about enabling this feature? >> >> >> >> thanks, >> >> vedant >> >> >> >> [1] [DebugInfo] Enable the debug entry values feature by default <https://reviews.llvm.org/D73534> >> >> [2] [CallSiteInfo] Fix the assertions regarding updating the CallSiteInfo <https://reviews.llvm.org/D73700>, [CallSiteInfo] Use the isCandidateForCallSiteEntry() when update the cs info <https://reviews.llvm.org/D74122> >> >> [3] https://robert.ocallahan.org/2018/11/comparing-quality-of-debug-information.html >> >> [4] https://gist.github.com/vedantk/7602b20a9e1d44c42d32dcca36591cc0 >> >> >> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>
Ivan Baev via llvm-dev
2020-Feb-20 16:34 UTC
[llvm-dev] Enabling debug entry value production by default
Thank you, Djordje, Vedant, Paul, for working on it! A great feature to have in LLVM by default. Cheers, Ivan On Thu, Feb 20, 2020 at 1:33 AM Djordje Todorovic via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Hi, > > This has been officially enabled by default. > > Thank you all, > Djordje > > On 10.2.20. 12:41, Djordje Todorovic wrote: > > Hi, > > > > Thanks you all for the collaboration! :) > > > > Paul, > > > >> This is not how tuning-controlled features are supposed to work. I > will comment on the review. > > I see, I am working on addressing the comments from the [1]. I will > update the diff asap. Thanks. > > > > Vedant, > > > > There are no entry values generated at -O0 level, but I will add a test > case for it. Thanks. > > > > Best regards, > > Djordje > > > > On 8.2.20. 02:41, Robinson, Paul via llvm-dev wrote: > >> This is not how tuning-controlled features are supposed to work. I > will comment on the review. > >> > >> --paulr > >> > >> > >> > >> *From:* vsk at apple.com <vsk at apple.com> *On Behalf Of * Vedant Kumar > >> *Sent:* Friday, February 7, 2020 3:26 PM > >> *To:* Robinson, Paul <paul.robinson at sony.com> > >> *Cc:* David Blaikie <dblaikie at gmail.com>; nikola.prica at rt-rk.com; > ibaev at cisco.com; asowda at cisco.com; llvm-dev at lists.llvm.org > >> *Subject:* Re: [llvm-dev] Enabling debug entry value production by > default > >> > >> > >> > >> The actual DWARF emission for call site parameters is gated inside of > DwarfDebug::constructCallSiteEntryDIEs by `tuneForGDB() || tuneForLLDB()`. > >> > >> > >> > >> However, we are creating+updating CallSiteInfo (basically, in-memory > only bookkeeping used by the backend to keep track of call sites) even when > the debugger tuning is set to the Sony debugger. If this creates problems, > feel free to file a bug and I'll address it. However, there's an upside to > maintaining all this CallSiteInfo even if TAG_call_site_parameters won't be > emitted. The backend asserts whenever a MachineInstr carrying CallSiteInfo > is destroyed without the call information being updated. This provides > useful test coverage. > >> > >> > >> > >> vedant > >> > >> > >> > >> On Feb 7, 2020, at 2:50 PM, Robinson, Paul <paul.robinson at sony.com > <mailto:paul.robinson at sony.com>> wrote: > >> > >> > >> > >> I’ve asked someone to check with our (Sony) debugger people, likely > hear back early next week. I’d say proceed with having it default on/off > based on tuning as you described, and if my guys want to see it turned on, > we can fix it later. I have to say, a quick skim through D73534 made me > think it’s not controlled that way right now, is that still a TODO? > >> > >> Thanks for all the work to support this feature! > >> > >> --paulr > >> > >> > >> > >> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org <mailto: > llvm-dev-bounces at lists.llvm.org>> *On Behalf Of *Vedant Kumar via llvm-dev > >> *Sent:* Friday, February 7, 2020 2:04 PM > >> *To:* David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com > >> > >> *Cc:* llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>; > nikola.prica at rt-rk.com <mailto:nikola.prica at rt-rk.com>; ibaev at cisco.com > <mailto:ibaev at cisco.com>; asowda at cisco.com <mailto:asowda at cisco.com> > >> *Subject:* Re: [llvm-dev] Enabling debug entry value production by > default > >> > >> > >> > >> Yep, TAG_call_site_parameter and its children shouldn't require any > extra relocations. Thanks! > >> > >> > >> > >> vedant > >> > >> > >> > >> > >> On Feb 7, 2020, at 2:01 PM, David Blaikie <dblaikie at gmail.com > <mailto:dblaikie at gmail.com>> wrote: > >> > >> > >> > >> For that sort of small growth, if it doesn't add more > relocations (I think the call sites need them (but they're already > emitted/that's not what we're discussing enabling here), but call value > descriptions don't), sounds good to me :) > >> > >> > >> > >> On Fri, Feb 7, 2020 at 12:04 PM Vedant Kumar < > vedant_kumar at apple.com <mailto:vedant_kumar at apple.com>> wrote: > >> > >> Hi all, > >> > >> > >> > >> I think we've reached a state where we're ready to enable > debug entry value production by default for the x86_64, ARM, and AArch64 > targets. For context, this is a debug info feature that allows debuggers to > recover the value of unmodified optimized-out parameters by 'going up' a > stack frame and interpreting spilled values, constants, etc. to work out > what was passed to the callee. There's a nice write-up about the impact of > this feature in [3]. A big thank you to all of contributors who've helped > us get here (I've tried to CC everyone, sincere apologies if I've > accidentally left you out). > >> > >> > >> > >> I believe Djordje and I have shown that there isn't an > outsized binary size or compile time impact of enabling this feature. The > aggregate size of DWARF in a stage2 x86_64 RelWithDebInfo clang build grows > by under 0.2%, and we didn't find a significant compile time impact. We've > shared CTMark results (both compile-time & DWARF section size numbers), as > well as a size analysis of stage2 builds of {llc, clang, lldb} in [1]. > >> > >> > >> > >> Djordje has also shared a nice location statistics > comparison chart in [1], showing increased variable availability with entry > values enabled. > >> > >> > >> > >> For my part, I instrumented lldb's > DWARFExpression::Evaluate method to log instances in which a variable > location is unavailable. I then used a script [4] to single-step through a > stage2 RelWithDebInfo+entryvals clang binary as it was compiling sqlite3.c, > printing local variables at each stop. Without entry values, lldb reported > "variable not available" 883,918 times. With entry values lldb reported > "variable not available" 687,899 times (a 22% drop). > >> > >> > >> > >> Does anyone have concerns about enabling this feature? > >> > >> > >> > >> thanks, > >> > >> vedant > >> > >> > >> > >> [1] [DebugInfo] Enable the debug entry values feature by > default <https://reviews.llvm.org/D73534> > >> > >> [2] [CallSiteInfo] Fix the assertions regarding updating > the CallSiteInfo <https://reviews.llvm.org/D73700>, [CallSiteInfo] Use > the isCandidateForCallSiteEntry() when update the cs info < > https://reviews.llvm.org/D74122> > >> > >> [3] > https://robert.ocallahan.org/2018/11/comparing-quality-of-debug-information.html > >> > >> [4] > https://gist.github.com/vedantk/7602b20a9e1d44c42d32dcca36591cc0 > >> > >> > >> > >> > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >> > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200220/c121cb83/attachment.html>