[PATCH] D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added a comment.

In D70548#1757299 <https://reviews.llvm.org/D70548#1757299>, @dblaikie wrote:

> In D70548#1756279 <https://reviews.llvm.org/D70548#1756279>, @djtodoro wrote:
>
> > @krisb Thanks for working on this!
> >
> > Let me summarize this quickly.
> >
> > This will remove mixing between calculation against enclosing and adjusted scope. We have concluded the adjustment heuristics are confusing and wrong in many cases, so we are giving up on that. The alternative is to generate the `DW_AT_start_scope` attribute, that represents exactly the adjusted scope, and having that we will be able to calculate the debug location statistics for the pieces of code where the variable is alive.
> >
> > WDYT how "easy" will be to implement the `DW_AT_start_scope` attribute? I know that it might not be used anywhere else, but I think that having location statistics against variable's life will be very useful thing.
>
>
> See my reply from D69027 <https://reviews.llvm.org/D69027> - though thinking more closely, I'm not sure start_scope would really answer the question as it seems like it has many of the same issues as the adjusted scope we're removing - it assumes the code is laid out in order. Any use of DW_AT_ranges would invalidate it (& I /think/ many & perhaps most scopes are fragmented into ranges in optimized code which is where any of these stats are interesting) & basic block reordering, etc, is sort of the nail in the coffin.
>
> "Yeah, that could be handy - but probably pretty complicated to propagate through the compiler & only have limited benefits to users (in normal debugger scenarios, rather than stats) - the difference between "this variable is out of scope" (or using another variable that would otherwise have been shadowed - that's pretty important) and "this variable's value is optimized out". The complexity of the implementation might be why it's not been implemented - possible that an implementation might be justifiable in a context of bigger improvements like is_stmt accuracy or sub-expression source range grouping, or other ideas that've been thrown around from time to time."


@dblaikie I see... There are many things to handle in order to make such stats valid.

In D70548#1758039 <https://reviews.llvm.org/D70548#1758039>, @krisb wrote:

> In D70548#1756279 <https://reviews.llvm.org/D70548#1756279>, @djtodoro wrote:
>
> > @krisb Thanks for working on this!
> >
> > Let me summarize this quickly.
> >
> > This will remove mixing between calculation against enclosing and adjusted scope. We have concluded the adjustment heuristics are confusing and wrong in many cases, so we are giving up on that. The alternative is to generate the `DW_AT_start_scope` attribute, that represents exactly the adjusted scope, and having that we will be able to calculate the debug location statistics for the pieces of code where the variable is alive.
>
>
> Thanks for summarizing this! This is perfectly correct.
>
> In D70548#1756279 <https://reviews.llvm.org/D70548#1756279>, @djtodoro wrote:
>
> > WDYT how "easy" will be to implement the `DW_AT_start_scope` attribute? I know that it might not be used anywhere else, but I think that having location statistics against variable's life will be very useful thing.
>
>
> I haven't investigated the subject in detail, but my initial guess is that life range may be computed as a union of basic blocks or their parts that 1) belong to the enclosing scope and 2) have debug value instructions that describe the variable (either with a valid location or with an undef). It looks easy to implement, but for practical use, it's probably too rough. I'm going to take another look a bit later.
>
> In D70548#1756669 <https://reviews.llvm.org/D70548#1756669>, @djtodoro wrote:
>
> > Please refactor the `llvm/utils/llvm-locstats/llvm-locstats.py` as well (there are variables containing `*FromFirstDefinition`).
>
>
> Thanks! Fixed.


@krisb Thanks for addressing the comments and considering the improvements!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70548