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

Thu Oct 31 17:22:13 PDT 2019

dblaikie added a comment.

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

> @dblaikie thanks, David! This is definitely needed to be kept in mind.
>
> In D69027#1728577 <https://reviews.llvm.org/D69027#1728577>, @avl wrote:
>
> > > I see the primary value of this statistic to allow us to see change — then a human has to make judgement what that change means.
> >
> > That is exactly the reason why I propose to drop calculation of OffsetToFirstDefinition. For the already discussed example, currently visible change would be 100%->100% For the case without OffsetToFirstDefinition there would be something like 20%->30% which makes change visible.
>
>
> Regarding tracking changes and their visibility, dropping calculation of OffsetToFirstDefinition is not a panacea, especially if we talking about 'coverage buckets'. In cases when we have a large enclosing scope and a relatively small variable's scope (or coverage), we will see no changes because they will be less than 10% to exceed the bucket's threshold. 
>  So, the only statistic we can rely on is the absolute number of bytes covered by variable's DW_AT_locations. And maybe on the number of bytes covered divided by the number of bytes in an enclosing scope. Doesn't this make even significant changes not noticeable?

Depends on the definition of significance - if a change only introduces a few small changes in total variable coverage, is that significant?

But a different bucketing scheme could be used maybe based on logarithmic buckets of scope or location coverage byte count. But I'm not sure that's too important - I wouldn't expect short ranges to have very different location characteristics to longer ranges (long ranges will still get moved into registers and otehr things like in short ranges, etc) - so I'd expect any significant change to show up more evenly across short and long ranges, so overall % growth to be fairly evenly distributed.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D69027