- User Since
- Dec 14 2017, 6:53 AM (87 w, 3 d)
Tue, Aug 13
Wed, Aug 7
IIUC, I can abandon this patch and make a separate patch(es) for adding additional fields into the llvm-dwarfdump stats output, and then make a script (and put into the utils/) parsing the fields by doing the locstats job?
@MaskRay Thanks for your review! We are still discussing on the llvm-dev mailing list how we should continue with this. As soon as we finished, I will start cleaning up the code and addressing comments.
Thu, Aug 1
@aprantl Thanks for your comment. WDYT about implementing this as a llvm-dwarfdump subtool?
Wed, Jul 31
@vsk Thanks a lot!
@vsk Thanks for the comments!
Mon, Jul 29
@vsk Thanks a lot for such nice review!
Thu, Jul 25
@vsk Thanks! As soon as we re-land the callsiteparam work I will start cleaning up the code and up streaming it!
@vsk IIUC, you think the tool will be useful ? :)
I’ll set this patch aside for now.
I think the goal of this patch is desirable for sure. In addition, I am not opposed to coexisting of this kind of stats within both of the llvm-dwarfdump and llvm-locstats, but that can be a point of discussion.
Wed, Jul 24
Thanks for working on this, it seems very useful! Any updates?
Tue, Jul 23
@vsk Thanks for this! We have discussed about this on the other threads and the stats is useful for sure!
Having this stats gives us info about the "normal" (non-entry values) debug location coverage, because those locations with the entry value are useless if do not have proper call_site and call_site_paramters debug information generated in the .debug_info section.
Jul 12 2019
@uweigand Thanks for the note and comment!
@dstenb Thanks a lot, that seems useful!
-Make a wrapper around the getCallPreservedMask() and use it in order to avoid changes in generated code
Jul 11 2019
@dstenb Thanks a lot!
LGTM! Thanks @vsk!
Jul 9 2019
Jul 8 2019
Yes, that sounds good to me!
-Remove redundant code
-Fix the debug entry value assertion from the VarLoc constructor
-Nit in a comment
Jul 5 2019
Did you consider what should be done with llvm-dwarfdump's "scope bytes covered" statistics?
I think that it sounds reasonable to add an additional field there, something like "non-entry-val scope bytes covered".
Just to clarify, as the emission of entry values is behind a non-driver flag I'm not opposed to landing these patches without changing llvm-dwarfdump, so please don't let me stand in the way. I think that a "non-entry-val scope bytes covered" field would be helpful, but someone with more say in the debug info area than me should decide that.
Jul 4 2019
-Fixing unintentional code during the rebase process
-Fix the assertion
The 'llvm-dwarfdump' does calculate the debug location statistics, but maybe we could think of reporting it with more information, since the debug location info is such important debug info. Please find a proposal for having a separate tool that will calculate only the debug location statistics on my github (https://github.com/djolertrk/llvm-locstats) and let me know if it can be useful for us. Potentially, we could add more options, functionalities, etc..
Jul 3 2019
Is this OK to go ?
Jul 2 2019
-Add support for entry values that are complex variables address (that avoids entry value not be safe for a single locations)
-Add a TODO for supporting local variables that are expressed in terms of parameters entry values
Would it perhaps make sense to add a small sentence about that in those TODOs?
@dstenb Sure! Thanks!
Can you change this comment from describing what the code does (which is fairly obvious), to why it does it?
@aprantl Sure. Thanks for the comment!
Jul 1 2019
-Clarify in the test the entry value coming from a location list
Due to very latest patches we came up with situations when we need to avoid single location represented as an entry value.
Also, having in mind the debug entry value is special case of the 'DBG_VALUE' instruction, we found it is OK in some situations to generate it after a first terminator.
Is this still OK to go ?
Nit: Use DWARF 5 in a test case.
-Avoid a single location represented as an entry value (+ add a test for that)
-Allow a debug entry value after a first terminator
Jun 27 2019
Jun 26 2019
Jun 20 2019
@aprantl The comment looks OK? :)
@aprantl This still looks OK after the rebasing? :)
-Add the comment explaining the assertion when deleting instructions