[PATCH] D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D69027#1729978 <https://reviews.llvm.org/D69027#1729978>, @avl wrote:

> > if you want a % one, "bytes covered by location lists" / "bytes of enclosing scopes" (counting each scope once for each variable within it - or doing the loc-list-intersects-with-enclosing-scope earlier & just tracking the relative numbers from that point).
>
> [snip ...]
>  To have resulting percentage number inside 100% we need to count scope each time for each variable.


@avl I believe that's roughly what @dblaikie suggested, so we should be on the same page.

In D69027#1729906 <https://reviews.llvm.org/D69027#1729906>, @krisb wrote:

> Currently (in the current master), we have the following statistics related to the discussion:
>
> 1. 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.
> 2. 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.
> 3. 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.
>
>   I propose to move to the following:
>
>
>
> 1. Keep as is.
> 2. Keep but:
>   - document properly with all the assumption and limitations, as 'it's known that the adjusted scope might not accurately represent variable's scope, but we still find it useful',
>   - do not calculate it for params,
>   - apply fixes proposed in this patch (i.e. account non-contiguous ranges while calculating OffsetToFirstDefinition).
>   - TODO: there might be a field for further investigation that can improve the heuristic.
> 3. Keep but:
>   - switch to 'the number of bytes covered by a variable's DW_AT_location' / 'the number of bytes in variable's adjusted scope' for local vars,
>   - document everything related to the 'adjusted' scope.
> 4. Add new statistics: the total number of bytes in variable's enclosing scopes over all variables.
>
>   What do you think?


+ 1. I think the first set of statistics would give a decent overview of debug info quality, and the second (without any adjustments to enclosing scope sizes) would be more useful for evaluating individual changes. I worry that only having the second set of statistics would paint an unrealistically bleak picture.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69027/new/

https://reviews.llvm.org/D69027