Re: [PR58315] reset inlined debug vars at return-to point

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Feb 26, 2015 at 1:01 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Feb 25, 2015, Jakub Jelinek <jakub@redhat.com> wrote:
>
>> On Wed, Feb 25, 2015 at 06:17:33PM -0300, Alexandre Oliva wrote:
>>> My measurements, for a not particularly unusual testcase, showed an
>>> overall reduction of 63% in compile time, as indicated yesterday.  Now,
>>> who should bear the burden of collecting evidence to back up the claims
>>> against the change?  Are those concerns enough to hold it up?
>
>> Can you e.g. run dwlocstat on some larger C++ binaries built without and
>> with your patch?  I believe dwlocstat is supposed to count only the
>> instructions where the variables or parameters are in scope, so should be
>> exactly what we care about here.
>
> Erhm...  I don't think that would cover the case you were worried about,
> namely inspecting variables of an inlined function while at a statement
> moved out of the function ranges.
>
> Anyway, I've run dwlocstat and inspected the results.  There is indeed a
> significant reduction in the coverage, so I looked into that.
>
> What I found out is that the reduction is an improvement on yet another
> long-standing var-tracking issue: if a function is called within a loop
> and we inline it, bindings from the call in one iteration of the loop
> will carry over onto the subsequent iteration until a new binding
> occurs.  This accounts for all of the coverage reductions I've
> investigated.
>
> This, in turn, suggests that introducing reset stmts when variables go
> out of scope even in local blocks will further reduce debug info,
> although in some cases it might have the opposite effect.  E.g., if the
> actual live range of a variable is scattered across multiple
> non-contiguous blocks, stopping the binding from being used in
> intervening blocks where the variable is not live will cause additional
> entries in the location list.  I've observed this effect with the
> proposed patch, too, but it removes a lot more nonsensical entries than
> it adds entries to account for not covering intervening ranges by
> accident.

Answering your questions on my mail here (because it fits I think),
and more concentrating on the effect of your patch as opposed to
debug stmt philosophy.

Your patch ends up pruning the var-tracking sets at the additional
reset-points you introduce.  You introduce them at inline boundaries
(which looks reasonable minus code-motion issues).

I claim you can achieve the same result by effectively inserting
those reset debug insns right before var-tracking itself and by
re-computing BLOCK "liveness".  That will automatically deal
with code motion that extends the life-range of an inlined BLOCK
by moving stmts (still associated with that BLOCK) across the
return point of the inlined call (and thus across your debug resets).
And it will allow var-tracking to eventually compute locations for
vars at the "entry" of that BLOCK piece.

After all you say correctly that what matters is location info
for X in places where a debug consumer can successfully lookup
X (that is, in PC ranges where the scope X was declared in is
active).  In other places there is no reason to emit location info
for X (but we might still want to compute it during var-tracking
if at a later PC range the scope will be active again).

Now I said you could do var-tracking uops for those resets somehow,
but I now realize that I have not much internal knowledge of var-tracking.
Thus consider the suggestion to insert reset debug insns at the beginning
of var-tracking and at points where a scope becomes dead (thus after
points where in a backward CFG walk a scope becomes live).

Richard.

> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer