- User Since
- Feb 1 2018, 11:54 PM (96 w, 2 d)
Thank everyone for reviewing this!
Committed as 68f464ac2ef5de8cb2e8beaeee43c435c536539e.
@djtodoro , yeah, it doesn't look good.
I also considered an option to rename the first bucket to smth like "No location info" or "Not covered" or something similar then the second can be safely named '0-9%'.
Or simply to use '0-9%' together with '0%' which might be a bit unclear, but as docs say 'The line 0% shows the number and the percentage of DIEs with no location information', it should not be confusing.
Thu, Dec 5
Sun, Nov 24
Refactored llvm/utils/llvm-locstats/llvm-locstats.py variable's names to avoid mentioning 'first def'.
Thu, Nov 21
Sat, Nov 9
When I started to work on the statistics, I also thought that the percentage buckets are to show completeness of the debug info. Unfortunately, due to BB reordering this information, sometimes not just imprecise, but rather terribly wrong. And in general case, we can't even guarantee any precision. I agree with @dblaikie; we need a way to calculate adjusted scope correctly regardless of basic block reordering to justify the notion of the adjusted scope itself. Otherwise, I'd prefer to reduce maintenance costs and to calculate debug info coverage against the full enclosing scope. In such a case, only changes in percentages would make sense, but at least this information won't be misleading. So, I tend to abandon this patch and proceed with another one that changes all the calculations against an adjusted scope to calculations against a full enclosing scope (if there are no strong objections).
Nov 1 2019
Currently (in the current master), we have the following statistics related to the discussion:
- The total number of bytes covered by DW_AT_location over all variables. We also have separate statistics for local vars only, params only and entry values for vars and params.
- The total number of bytes in variable's scopes over all variables. Variable's scope means a scope adjusted to the lowerest address in variables' DW_AT_location. Plus, separate statistics for local vars only and params only.
- The number of variables in each coverage bucket, where the coverage bucket calculated as 'the number of bytes covered by a variable's DW_AT_location' / 'the number of bytes in variable's enclosing scope'. Variable's enclosing scope means a full scope (e.g. full subprogram, lexical block, etc.) without any adjustments. There are also separate statistics for local vars and params, and all the three where entry values are excluded.
Oct 31 2019
@dblaikie thanks, David! This is definitely needed to be kept in mind.
Oct 29 2019
Doesn't the absolute number of covered bytes (i.e. ScopeBytesCovered) suite this purpose? Yes, we will see no change in coverage, but the absolute number will be changed from 14 to 97 bytes, which may be considered as an improvement. Does it make sense to you?
Oct 27 2019
Bumped version 4, updated more tests, added comment for adjustScopeToFirstDef().
Oct 21 2019
@avl, thanks for the comment.
Oct 16 2019
Oct 7 2019
Sep 11 2019
@dnsampaio, many thanks for committing this!
Sep 2 2019
Applied the comment: enable 'sha2' and 'aes' only for armv8 A profile, report warning and disable 'crypto' for other archs. Add more tests.
Aug 30 2019
I see. But this doesn't seem like a valid case, does it? Shouldn't we somehow diagnose it to not to silently ignore user-specified options?
@dnsampaio thanks for reviewing this! Could I ask you to commit the patch (since I don't have commit access yet)?
Added 'CPU.empty()' check back.
Aug 29 2019
@dnsampaio, thanks! This is definitely better. I also added ARMV8_5A as a test case.
Applied the comment & rebased.
Also changed the patch to take into account only the latest 'crypto' in the Features vector, to avoid enabling 'sha2' and 'aes' when 'crypto' was finally disabled.
Aug 15 2019
@dnsampaio, thanks, this looks better.
Applied the comment.
Aug 9 2019
Apr 11 2019
Jan 25 2019
Fixed broken test.
@asl thanks for catching this! Looks like I forgot to amend a fix for the test after testing :( sorry for that.
Jan 23 2019
@aaron.ballman yes and yes. Thanks!
Jan 22 2019
@aaron.ballman Thanks! Could I ask you to commit the patch? I don't have commit access yet.
Applied the comment.
Jan 21 2019
Jan 16 2019
Jan 15 2019
Jan 14 2019
Jan 12 2019
Jan 10 2019
Dec 21 2018
Nov 26 2018
Nov 17 2018
Applied comments and extended condition with 'ShiftAmount == 8'
Nov 16 2018
Nov 15 2018
Apr 19 2018
Applied the comments
Mar 2 2018
Feb 28 2018
Reverted unintentional changes again. Sorry for that
@teemperor, thanks! All comments are applied.
Feb 27 2018
Feb 26 2018
Reverted unintentional changes.
Feb 22 2018
Updated one more test where attributes became mismatched.
Feb 21 2018
Feb 20 2018