On 10/31/2013 12:26 PM, David Malcolm wrote:
[Shamelessly hijacking Andrew's thread about gimple.h refactoring,
since this seems on-topic for that, and I'm keen to hear from Andrew on
how the following would interact with his work - I *think* our two
cleanups are orthogonal.
Mostly orthogonal anyway... just stomping on the same bits :-).
Since you hijacked a planning thread, do you plan to take this any
further, or make this change and move on to something else?
It is a start, but it doesnt do the rest of the work that needs doing to
really take advantage of it... which will be extensive.
for instance, we should change:
static inline void
! gimple_call_set_lhs (gimple gs, tree lhs)
{
- GIMPLE_CHECK (gs, GIMPLE_CALL);
gimple_set_op (gs, 0, lhs);
to
static inline void
! gimple_call_set_lhs (gimple_statement_call *gs, tree lhs)
{
gimple_set_op (gs, 0, lhs);
but then every location that calls it needs an appropriate change:
! gimple call;
! call = gimple_build_call_vec (build_fold_addr_expr_loc (0,
alias), vargs);
gimple_call_set_lhs (call, atree);
--- 1518,1524 ----
! gimple_statement_call *call;
! call = as_a<gimple_statement_call> (gimple_build_call_vec
(build_fold_addr_expr_loc (0, alias), vargs));
gimple_call_set_lhs (call, atree);
And in fact there is a ripple effect to then change
gimple_build_call_vec to simply return a gimple_statement_call *... Then
this doesn't look as ugly either...
! gimple_statement_call *call;
! call = gimple_build_call_vec (build_fold_addr_expr_loc (0,
alias), vargs);
gimple_call_set_lhs (call, atree);
that is looking much better :-)
Leaving the names as they are should be ok, but the I'd also add a
typedef for the pointer without the 'statement' in the name.. ie
typedef gimple_statement_call *gimple_call;
That seems in line with what we do with 'gimple' right now and the short
form is the type we'd normally use.
That adds a touch of difficulty with "as_a", since that requires the
type name, not the shorthand pointer.... so you have something like
gimple_call call = as_a <gimple_statement_call> blah().
I think as the changes to use the gimple_call type are pushed through
all the callers and callee's, the requirement of as_a and the long name
being ugly begins to rapidly disappear... it'll only exist in the core
creation routines and no one will usually see it. So I don't *think*
this is an issue... but it is an ugly transition if its only partially
done.
And eventually we can pull the accessor routines and others into the
class itself:
gimple_call call;
call = gimple_build_call_vec (build_fold_addr_expr_loc (0,
alias), vargs);
call->set_lhs (atree);
Which results in a similar feel to the new gimple_type, gimple_decl,
etc. classes with my upcoming tree wrappers. Changing gimple statements
to behave something like this is actually mentioned in the plan, but out
in the future once trees have been taken care of.
I would also plan to create instances for each of the gimple statements
that don't have them now, like gimple_statement_assign. Its lumped in as
a general GSS_WITH_MEM_OPS, so there is no gimple_statement_assign
class/struct to use.
It would really be nice to use the DEFGSCODE macro and gimple.def to
make this happen automagically somehow... Its tantalizingly close now I
think, especially combined with the info in gsstruct.def... Although if
we are moving to proper classes eventually its probably better to
explicitly write the required class for a statement kind.
That all said, this change enables that work to proceed if someone wants
to do it.
My question is: Is anyone going to do it, and if so, who and when? :-)
Again, as noted in the earlier patch series, the names of the structs
are rather verbose. I would prefer to also rename them all to eliminate
the "_statement" component:
"gimple_statement_base" -> "gimple_base"
"gimple_statement_phi" -> "gimple_phi"
"gimple_statement_omp" -> "gimple_omp"
etc, but I didn't do this to mimimize the patch size. But if the core
maintainers are up for that, I can redo the patch series with that
change also, or do that as a followup.
As mentioned, I'd rather see the short names be typedefs for pointers to
the long names since we're typically using the pointers.
Other than stomping on the same bits at the moment (less so once
gimple-stmt.[ch] is split out), these changes are completely orthogonal,
but in line with with my direction. I think It should be done sooner or
later.... My preference is to also see the follow up work carried out
as well. Or at least a plan for it.
Andrew
