[patch] Separate immediate uses and phi routines from tree-flow*.h

[Andrew MacLeod]

On 09/25/2013 04:49 AM, Richard Biener wrote:
> On Tue, Sep 24, 2013 at 4:39 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
>> 3 - a few routines seem like basic gimple routines, but really turn out to
>> require the operand infrastructure to implement... so they are moved to
>> tree-ssa-operands.[ch] as well.  This sort of thing showed up when removing
>> tree-ssa-operands.h from the gimple.h include file.  These were things like
>> gimple_vuse_op, gimple_vdef_op, update_stmt, and update_stmt_if_modified
> Note that things like gimple_vuse_op are on the interface border between
> gimple (where the SSA operands are stored) and SSA operands.  So
> it's not so clear for them given they access internal gimple fields directly
> but use the regular SSA operand API.
>
> I'd prefer gimple_vuse_op and gimple_vdef_op to stay in gimple.[ch].
>
> If you want to remove the tree-ssa-operands.h include from gimple.h
> (in some way makes sense), then we need a new gimple-ssa.[ch]
> for routines that operate on the "gimple in SSA" IL (not purely on
> gimple, which could be not in SSA form and not purely on the SSA
> web which could in theory be constructed over a non-GIMPLE representation).
>
> Actually I like the idea of gimple-ssa.[ch].  tree-ssa-operands.[ch]
> would then be solely the place for the operand infrastructure internals
> implementation, not a place for generic SSA utility routines related
> to SSA operands.
>
> Moving update_stmt/update_stmt_if_modified is a similar case but
> maybe less obvious (unless we have gimple-ssa.[ch]).
>
> What do you think?

Yeah, I was mulling something similar, I wasnt super thrilled that those 
were going into tree-ssa-operands.h... but they didnt belong in gimple.h 
either.   I think adding gimple-ssa.[ch] is the way to go
> (swap_tree_operands): Move prototype to tree-ssa-operands.h.
>
> rename to swap_ssa_operands and remove the !ssa_operands_active
> path?  (should be no longer needed I think)
Yeah, I think you are right. Its not being used by tree only routines, 
and it trivial in that case anyway.
>
>          (gimple_phi_arg_def, gimple_phi_arg_def_ptr, gimple_phi_arg_edge,
>          gimple_phi_arg_location, gimple_phi_arg_location_from_edge,
>          gimple_phi_arg_set_location, gimple_phi_arg_has_location, phi_nodes,
>          phi_nodes_ptr, set_phi_nodes, phi_ssa_name_p): Move to tree-phinodes.h.
>
> I always found the separation of PHI node API from gimple API odd ...
> now, PHI nodes are a SSA speciality.  Unless the gimple_phi class
> moves to tree-phinodes.h the same comments as to gimple_vuse_op
> apply.  But maybe everything will be more obvious once we separate
> the core gimple data structure from the gimple API stuff ...

Hrm. Yeah, this one isn't as clear as it might seem at first. gimple.h 
does have a few accessors for the base statement kind... and these are 
accessors for the argument which is from core-types.h.   I imagine that 
tree-phinodes.h should do all the phi-node related stuff short of the 
actual stmt itself... but its not immediately obvious. I'd like to leave 
these in tree-phinodes.h and I'll make a stab at how to manage that 
next..  it would actually be nice to get struct phi_arg_d  *out* of 
core-types.h.. but maybe thats too grand.  I can move them again later 
if it turns out the should be in the border file or elsewhere.


> Overall the patch looks ok, either massage it according to my comments
> above (thinking about gimple-ssa.[ch]) or follow up with a separate patch
> (not that moving things twice will hurt).
>
I'll do the massage., I like the gimple-ssa border files.   I suspect 
there will be more shuffling of some of these routines down the road.. 
right now its pretty high level separation thats happening. Once things 
are better clustered, I expect more appropriate fine grained breakdowns 
will become apparent.

Andrew