On Wed, Oct 17, 2012 at 9:48 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I've gone ahead and reverted it here:<br>
<br>
        D       test/CodeGenCXX/debug-info-user-def.cpp<br>
        M       test/CodeGenCXX/debug-lambda-expressions.cpp<br>
        M       test/CodeGenCXX/debug-info-template-quals.cpp<br>
        M       test/CodeGenCXX/debug-info-template-member.cpp<br>
        M       lib/CodeGen/CGDebugInfo.cpp<br>
W: -empty_dir: test/CodeGenCXX/debug-info-user-def.cpp<br>
r166109 = 7c70a2300fbd713858c31cc9cbbe074bdc233412 (refs/remotes/origin/master)<br>
<br>
Feel free to rebase some test suite changes on top of that.</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Alexey: If you need anything else let me know.<br></blockquote><div><br></div><div>Yes, I think I still need a review for patch for PR13942.</div><div><br></div><div><a href="https://codereview.appspot.com/6720044/">https://codereview.appspot.com/6720044/</a></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Wed, Oct 17, 2012 at 10:14 AM, Robinson, Paul<br>
<<a href="mailto:Paul.Robinson@am.sony.com">Paul.Robinson@am.sony.com</a>> wrote:<br>
> My goal was reducing the size of .debug_info, however 158009 did not achieve<br>
> this<br>
><br>
> and I was trying to eliminate the bloat that it caused.<br>
><br>
><br>
><br>
> If the artificial entries are useful for some other purpose then reverting<br>
> 158009 is fine.<br>
><br>
> If we do that, then my patch #1 becomes unnecessary.  Depending on what we<br>
> decide<br>
><br>
> 'nodebug' means, patch #2 may also be unnecessary, although I observe that<br>
> there<br>
><br>
> really are no tests for 'nodebug' on a method and we might want to hang onto<br>
> that part.<br>
><br>
><br>
><br>
>> What I don't need is the debug info for bodies of these functions (all the<br>
>> local variables<br>
><br>
>> etc.).<br>
><br>
><br>
><br>
> When we are talking about class methods, what happens is that there is a<br>
> declaration<br>
><br>
> DIE which is a child of the class DIE.  This DIE has no code range.  When<br>
> the method is<br>
><br>
> defined, there is a definition DIE with code ranges, and DW_AT_specification<br>
> pointing<br>
><br>
> back to the declaration DIE.  (The problem with 158009 is that there was no<br>
> declaration<br>
><br>
> DIE, so Clang created one on the fly.)  So, if you are thinking of<br>
> suppressing the<br>
><br>
> definition DIE, you won't get code ranges for the artificial methods.  This<br>
> is already<br>
><br>
> the case for 'nodebug' methods, there is a declaration DIE but no definition<br>
> DIE.<br>
><br>
><br>
><br>
> I think this dual-DIE scheme does not apply to standalone functions.  It<br>
> looks like there<br>
><br>
> is no DIE at all for 'nodebug' standalone functions.<br>
><br>
><br>
><br>
> I think most artificial functions/methods don't really have a lot of child<br>
> DIEs?  They are<br>
><br>
> usually pretty straightforward and don't declare local variables and so<br>
> forth.  For an<br>
><br>
> artificial constructor, if there is a DIE at all, you _do_ want to preserve<br>
> the formal-parameter<br>
><br>
> DIEs, but that's about all you get anyway.<br>
><br>
><br>
><br>
> So if you want definition DIEs, but no children (other than parameters),<br>
> maybe for the<br>
><br>
> artificial/nodebug methods/functions we want to generate the<br>
> declaration+defintion DIEs<br>
><br>
> but then treat the bodies as if we have -gline-tables-only in effect?  Would<br>
> that put the<br>
><br>
> right code ranges on the subprogram DIEs?<br>
><br>
> --paulr<br>
><br>
><br>
><br>
> P.S. offsite the rest of the day, will try to check back this evening.<br>
><br>
><br>
><br>
> ________________________________<br>
> From: Alexey Samsonov [<a href="mailto:samsonov@google.com">samsonov@google.com</a>]<br>
> Sent: Wednesday, October 17, 2012 9:39 AM<br>
> To: Eric Christopher<br>
> Cc: Robinson, Paul; <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
><br>
> Subject: Re: [cfe-commits] [PATCH] PR14097, no debug info for<br>
> artificial/'nodebug' methods<br>
><br>
><br>
><br>
> On Wed, Oct 17, 2012 at 7:57 AM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>
> wrote:<br>
>><br>
>> > Yes it looks like our patches are at cross-purposes.<br>
>> ><br>
>> > I assume that your patch gets the desired effect because there is now a<br>
>> > DIE that<br>
>> > points to the subprogram's generated code?  That's not good for me,<br>
>> > because if<br>
>> > there's a class method definition DIE, it insists on having a<br>
>> > declaration DIE too.<br>
>> > I'm not sure whether the same is true for standalone functions.<br>
>> ><br>
>> > I wonder if there is a way we could produce an artificial subprogram<br>
>> > DIE, but then<br>
>> > have LLVM not actually emit it to the DWARF.  That is, suppress the<br>
>> > excess DWARF<br>
>> > in LLVM rather than in Clang.  If lives long enough to build the address<br>
>> > ranges (and<br>
>> > also .debug_line?) it would meet your needs, and then if it doesn't<br>
>> > actually get emitted<br>
>> > to .debug_info, it would meet my needs.<br>
><br>
><br>
> What are your needs? Do you want to keep the .debug_info smaller, or this<br>
> DIE may<br>
> be malformed and cause problems for you? I'm somewhat afraid that data in<br>
> .debug_line would not be enough for me. This depends on the way a tool maps<br>
> instruction address to compile unit. One of the way is:<br>
> 1) get the compile unit DIE<br>
> 2) get a DWARF line table for this DIE<br>
> 3) parse the table and build address ranges for this compile unit.<br>
><br>
> This (step 3) is of course inefficient.<br>
> Currently LLVM's DebugInfo lib does the following:<br>
> 1) get the compile unit DIE<br>
> 2) get all subprogram DIEs for this CU<br>
> 3) collects address ranges for these subprograms.<br>
><br>
> So, for such an approach, I need the debug_info entries for artificial<br>
> functions and methods.<br>
> generated by Clang. I even may need entries for functions with disabled<br>
> debug<br>
> info. What I don't need is the debug info for bodies of these functions (all<br>
> the local variables<br>
> etc.).<br>
><br>
> Probably the best way is to use .debug_ranges section and add<br>
> a single DW_AT_ranges into the compile unit DIE. In this way we'll be able<br>
> to get all the instruction address ranges for compile unit without scanning<br>
> any<br>
> additional DIEs (or using weird .debug_aranges section).<br>
><br>
>><br>
>><br>
>> I'm more than happy to revert r158009, I originally added it to reduce<br>
>> the amount of debug information since often people would not be trying<br>
>> to add break points or debug through an artificial function. Alexey's<br>
>> patch and motivation, however, lead me to believe that this was<br>
>> originally incorrect as we may want a backtrace with information and<br>
>> line numbers that includes artificial functions like global<br>
>> constructors.<br>
>><br>
>> Either of you have an argument against reverting it?<br>
><br>
><br>
> I have not.<br>
><br>
>><br>
>><br>
>> -eric<br>
><br>
><br>
><br>
> --<br>
> Alexey Samsonov, MSK<br>
><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div><br>