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