On 09/25/2013 04:49 AM, Richard Biener wrote:
On Tue, Sep 24, 2013 at 4:39 PM, Andrew MacLeod <amacl...@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
