Re: [vta, vta4.4] merged with trunk and 4.4 @149247, updated VTA patchset

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Jul 10, 2009 at 9:22 PM, Alexandre Oliva<aoliva@redhat.com> wrote:
> On Jul ?9, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:
>
>> The main point is to improve readability (and thus maintainability)
>> of the use sites.
>
> One of them would have exactly *one* use site: release_ssa_name.
>
> Anyhow, I'm thinking of renaming the function to
> propagate_rhs_into_debug_stmts, which would hopefully take care of the
> lack of clarity you're getting at. ?Does that work for you?
>
>>>> Note that while adjust_debug_stmts_for_stmt_move can be done lazily
>>>
>>> Can it, really?
>>>
>>> The moved DEF could very well be modified to something incorrect before
>>> we get a chance to pick up the correct value, if we do it lazily.
>
>> IMHO we (usually) shouldn't modify the value assigned to an SSA name.
>
> Huh? ?We do it all the time. ?Every time we propagate something into an
> SSA name, substitute an SSA name for a constant during say threading,
> whatever, we modify the value assigned to an SSA name. ?If, after these
> transformations (particularly threading), we were to assume that the rhs
> is still equivalent to the original rhs, we'd lose.

In threading we either duplicate some statements creating new
SSA names or the values are always constant, so the values do not change.

Maybe we have a different idea of "same value"?

>> It for example renders points-to information attached to the name
>> incorrect.
>
> I don't see that it does. ?The transformations I'm concerned with
> actually preserve points-to information. ?However, since they're
> computed from different expressions that might *not* be apropriate in
> the original context in which the original SSA DEF appeared, we can't
> wait for a later time to make the change.

How can they be not appropriate but still in correct for for the IL?
IMHO lazy removing of the bogus information should work here.

> Now, if you're suggesting that every time we touch an SSA DEF we
> generate a new SSA name, and substitute all occurrences, I guess this
> would be fine, for then the original DEF would be ssa_name_release()d
> right away (or remain in place until it's found to be dead and
> released), at which time we'd be able to use its RHS in context.

I don't say that.  I say that, given

 i_1 = 1;
 # DEBUG j = i_1;
 j_2 = i_1;

(that would be the correct initial state, right?)

Now we apply constant propagation.

i_1 = 1;
# DEBUG j = 1;
j_2 = 1;

works.  Even if we were not to propagate into the DEBUG statement,
then we'd get

i_1 = 1;
# DEBUG j = i_1;
j_2 = 1;

and i_1 will become dead and release_ssa_name()d.  At which point
we still can adjust the DEBUG stmt to its rhs.  Which even works for
invalid SSA form.

It does fail at the point we re-use the ssa name for something completely
unrelated, but as I said - I would consider that a quality of implementation
bug.

Richard.