Re: [PR debug/47106] account used vars only once

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

On Tue, Feb 1, 2011 at 11:25 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jan 31, 2011, Richard Guenther <richard.guenther@gmail.com> wrote:
>
>>> Second, it showed that we completely failed to compute used flags after
>>> inlining. ?At first, I split the computation of used flags out of
>>> remove_unused_locals() and refrain from actually removing variables, but
>>> then I decided calling removed_unused_locals() after inlining might get
>>> us smaller functions and more accurate computations for additional
>>> inlining.
>
>> Err, you add remove-unused-vars in function versioning. ?Which means
>> if that is useful we copied a function with unused locals - that's the place
>> this should (eventually) be fixed, not this one.
>
> No, it just means we need part of the computation fo
> remove_unused_vars(). ?Maybe my assumption above, that it would make a
> difference in terms of function size and accuracy, is wrong, but we
> still need to compute what is used and what isn't.
>
> Simply setting newly-copied variables as used doesn't do it: if the
> original function has unused user variables preserved for debug info, we
> want them preserved as unused, rather than having them set to used as in
> the previous version of the patch did.
>
> I suppose we could simply copy the state of the used flag when
> duplicating a variable. ?I haven't tried that, but it's probabl worth a
> shot, to avoid having to compute used/unused variables after versioning.
>
>>> 4th: check that used flag is never accessed before initialization, and
>>> that it's uninitialized after inlining and cleared after clearing it for
>>> all referenced_vars
>
>> This is not ok (at this stage anyway)
>
> It wasn't meant to be installed anyway, it was just for the record.
> Like the 3rd patch. ?Now, if you find this useful for the next stage,
> you think it should be enabled given some (new?) checking option?
>
>> I think we should compute "used" where we need it (like
>> remove-unused-vars does). ?If expand needs it we should compute it
>> there (I think there are no other users).
>
> One of the problems is that we only compute used for referenced vars,
> but I found we sometimes used it for LOCAL_DECLs or block-scope decls
> that were not in referenced vars, so their flags weren't cleared or
> recomputed.

Did you spot any other users than remove-unused-locals and expand?
What I said above is that expand should compute a used flag for
the variables it cares about (not necessarily restricted to referenced-vars),
and _not_ use var_ann ()->used for this (but for example a bitmap of UIDs).
var_ann () should go away.  eventually.

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 Brazil Compiler Engineer
>