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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

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

> > I have my patch based on two assumptions:
> > 
> >   We only care about lexically (or statically) scoped languages. Otherwise detecting a variable's scope is very problematic.
> >   In a lexically scoped language the end of a variable's scope should match the end of the enclosing lexical scope.
> > 
> > That's why I a bit confused about why would we need to make some adjustments to the end of a range. Could you kindly clarify what did you mean?
>
> Current solution with showing coverage for VarScope - OffsetToFirstDefinition looks like a mix of two concepts:
>
> 1. variable scope. Block where a variable is defined.
> 2. variable lifetime. The region from definition till last use.
>
>   I think the explicit separation of these two things would allow understanding statistics better. See the following example of current behavior.


I'm not sure this answers the question which was about your statement: "The end of the variable's address ranges should also be encountered."

& I'm not sure those are two ideal concepts.

In C++ & other scope-based languages, the scope of a variable is from the point of declaration to the end of the enclosing scope. In the DWARF the first point there is lost - so we make a heuristic guess at it.

The region from definition till last use would also be a guess - the DWARF doesn't have information about the last use. Only about the last live location (which may be past the last use).

We know that the variable should be live until the end of the enclosing scope (as per the language) but we don't know where it should start - so there are (if I'm unedrstanding this discussion correctyl) two statistics - one for each end of the heuristic range: one measuring from the start of the enclosing scope (the earliest the variable could be declared) and the other measuring from the first reported location (that's the statistic that's being discussed/fixed here).

I don't think pruning the end of the latter list improves the quality of the statistic - since scope based languages, as @krisb said, have the variable accessible (at the language level) to the end of the enclosing scope, so we know that coverage bytes missing in that tail are missed opportunities for greater variable coverage.

>> If the other metric is valuable, I suggest implementing it in a separate patch. But could you give an example when calculating coverage against full scope provides additional knowledge which does not follow from currently existing metrics?
> 
> suppose we have such case with incomplete debug info:
> 
>   DW_TAG_lexical_block
>   
>   DW_AT_ranges  (0x00000000
>      [0x000000000000010, 0x0000000000000197))
>   
>   DW_TAG_variable
>     DW_AT_location  (0x0000018e
>        [0x0000000000000183,  0x0000000000000197): DW_OP_reg0 RAX)
> 
> 
> And then will create a fix which improves debug info in such a way:
> 
>   DW_TAG_lexical_block
>   
>   DW_AT_ranges  (0x00000000
>      [0x000000000000010, 0x0000000000000197))
>   
>   DW_TAG_variable
>     DW_AT_location  (0x0000018e
>        [0x0000000000000100,  0x0000000000000183): DW_OP_reg1 RBX)
>        [0x0000000000000183,  0x0000000000000197): DW_OP_reg0 RAX)
> 
> 
> In both cases reported coverage there would be 100%. 
>  That hides original low coverage for that variable and makes invisible performed improvement.
>  Change in coverage looks like this: 100%->100%. If it instead would be 20%->30% then result of changes would be seen.

That's why there's the other statistic, which is bytes of enclosing scope coverage rather than the version that trims the start. ("vars scope bytes covered" / "vars scope bytes total")


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69027