Page MenuHomePhabricator

[llvm-dwarfdump] Fix statistics for "vars"
AbandonedPublic

Authored by vsk on Apr 3 2020, 5:00 PM.

Details

Summary

In llvm-dwwarfdump --statistics output, "vars" refers to parameters and
local variables. Fix an issue where the scope bytes covered metric for
"vars" is unexpectedly low because parameter coverage isn't included.

Diff Detail

Event Timeline

vsk created this revision.Apr 3 2020, 5:00 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: MaskRay. · View Herald Transcript

I think that it was intentional. The "vars" fields refer only to local variables, and I agree we should try renaming this...

This is one of those things where I am fine either way, we just should document what exactly the fields mean. (Could we even sneak the field description into the JSON output in a meaningful way?)

vsk abandoned this revision.Apr 6 2020, 2:43 PM

Sorry for the noise, this patch isn't correct for the reason @djtodoro pointed out.

I will just add that we should try renaming the JSON fields (with a meaningful name) as well as the variables used to compute this numbers, since I am also always confused what the fields refers to :)

( at least, we can add a TODO marker there, and address that when we get a chance)

I think it would be reasonable to bump the version number and move to a coherent naming scheme. I came up with the original names in hurry while preparing for a dev meeting talk, I'm not wed to them :-)

OK then :)
I'll try to find some time to rename that in the following days (if you do not have time to do that).

vsk added a comment.Apr 8 2020, 11:13 AM

I think it'd be nice if the stats fields were a little shorter and clearer. Unfortunately I'm not sure I'll be able to revisit this soon.

Please take a look: D77789.