Re: IPA merge part 1: localize SSA variables

[Andrew MacLeod <amacleod at redhat dot com>]

On Thu, 2006-11-16 at 14:26 +0100, Jan Hubicka wrote:

> The reason is that I don't think we should blindly have
> gimple_/gimple_set accessors for everything in tree-flow.h as it would
> just increase size of the header and give false feeling that all those
> variables are public and shall be set by everyone.  For many of fields
> we do have existing accessors and I think it makes sense to update them
> directly to use the gimple_df structure.  Also it seems to me that there
> is no need for gimple_set accessor for things wich are set/reset on one
> place only and are designed to be so.
> 

Blindness is usually not a good thing :-)

Any fields which already have accessors certainly shouldnt need another
set for no good reason, I agree.

Single set/reset by design also doesn't really benefit from them either.
I Agree as well.

> So I ended up with read only accessors to the fields shared across number of
> modules and I also found that the gimple/gimple_set scheme does not work
> too well with vectors since those do & operator on the vector parameter of
> macro requiring it to be lvalue.
> 
> Luckilly I think writes to all vectors we have are sufficiently abstracted
> (particularly it is ssa_names set inly in tree-ssa-names and
> modified_noreturn_calls set once in mark_stmt_modified and once reset in
> cfgcleanup.c)
> 
> What do you think?  I definitly have no problems with adding the missing
> set_ accessors if it looks like preferred variant.

I think in these cases we don't need the read/write accessors, but
personally I would still like to see the access parameterized, so
instead of 

     VEC_quick_push (tree, cfun->gimple_df->ssa_names, NULL_TREE);
     cfun->gimple_df->free_ssanames = NULL;

we'd see

     VEC_quick_push (tree, SSA_NAMES (cfun), NULL_TREE);
     SSA_NAMES (cfun) = NULL;

It will save us work in the long run if we ever want to have more than
one instance around, for whatever reason.

> In longer run I am not sure how far we want to go with elliminating
> global cfun accesses.  It definitly do make sense to keep them in local
> passes - we don't want to add extra argument to every single function in
> GCC. On the other hand it would be nice from IPA point of view to do,
> for instance, cfgcleanup directly on specified function after some IP
> transformation was performed.

it might make sense to have a local cfun for uses within an
optimization, especially when that optimization already has a data
structure which is 'global' within the optimization. So cfun is passed
to the highest level routine, which initializes the 'local data' for the
optimization with that value of cfun.  The rest of the optimization
routine then use that. Any routines shared between different
optimizations may still need to pass cfun tho. 

I do this to some extent in out-of-ssa with its' common data structures.
a var_map is used by virtually everything, but it is either passed to a
function, or a more encompassing data structure (like the conflict
graph)  contains a pointer to the var_map it uses. Then passing the more
encompassing structure includes all the 'other' values which might be
needed, such as the relevant var_map. it keeps you from having to pass
all these extraneous things around, and also makes it clear if there is
a dependency on a specific instance.  

(I havent woken up completely yet, so maybe that wasn't really clear
enough, I don't know :-)

Bottom line is we don't need to make more work for ourselves, but when
we are changing these things, we might as well go right to the
(perceived :-)  end solution for those cases, in my opinion.

Andrew