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

[Alexandre Oliva <aoliva@redhat.com>]

On Mar  9, 2015, Jeff Law <law@redhat.com> wrote:

> On 03/09/15 08:38, Richard Biener wrote:
>> On Fri, Mar 6, 2015 at 7:04 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> On Feb 26, 2015, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> 
>>>> So far, all the differences I looked at were caused by padding at the
>>>> end of BBs, and by jump stmts without line numbers at the end of BBs,
>>>> both right after the debug reset stmts the proposed patch introduces.
>>> 
>>> Further investigation showed there were other sources of spurious
>>> differences:
>>> 
>>> - copies arising from the expansion of PHI nodes; source code
>>> information associated with these copies points at the source of the
>>> copy, which is hardly useful and sensible.
>> 
>> Care to explain?  We spend quite some resources to maintain them
>> (locations on PHI args, that is).

The situation I remember, for looking more extensively into, involved a
variable local to the inlined function, a copy of that variable to a
caller's variable, and that caller's variable being further modified
elsewhere.

We coalesced the two variables into one, removed the copy statement
altogether, and the location from the initial set within the inline
function made it to PHI nodes related with the caller's variable, and
that survived various PHI recomputations due to various CFG
reorganizations.  When the time came to expand the PHI nodes, copies
were required and line number info internal to the inlined function was
added to the copy arising from the edge that had in it the SSA name set
inside the inline function.  And so the ranges of the inline function
were extended far past its actual end, because a coalesced copy from one
of its variables was optimized away and then reintroduced with line
number info pertaining to the inlined function.

> I almost responded to that claim as well, but then thought better of
> it as that patch (AFAICT) wasn't proposed for inclusion, but was being
> used for testing purposes.

The patch that removed line number annotation from copies arising from
expanding PHI nodes was included only for reference, yes.  The upthread
patch for PR58315 still stands.

> Expansion of the PHI into copies should have locations which point to
> the source of the various PHI args.  Those are quite meaningful.

I can see those can be meaningful in many cases.  Not in the ones I
looked at as part of investigating the coverage changes, though.  We
have a substantial number of copies that arise from processes like the
one I described above, and those are most certainly not sensible, mainly
because the relevant line number info was dropped on the floor along
with copies optimized away due to coalescing of variables from different
scopes.

-- 
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