Re: [patch 1/2] add gimplfy-be.[ch] for iterator-aware BE-only gimplification routines.

[Andrew MacLeod <amacleod at redhat dot com>]

On 11/14/2013 10:41 AM, Richard Biener wrote:
> On Thu, Nov 14, 2013 at 4:10 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
> > This patch splits out the force_gimple_operand  parts of gimplify.[ch] into
> > their own file which will prevent the front ends from having to see
> > iterators, and breaks the annoying dependency cycle between gimple.h,
> > gimplify.h and gimple-iterator.h.  I suspect more stuff may end up here, but
> > this is all that is needed for now.
> >
> > There were also a few gimplification related things still hanging around in
> > gimple.h so I moved those to gimplify.h as well.
> >
> > This also allows gimple-iterator.h to finally take ownership of "enum
> > gsi_iterator_update".
> >
> > When I originally created gimplify.h, I included gimple.h right from
> > gimplify.h thinking it would be better for the front end files, but really,
> > no. It just complicates things, so I flatten gimplify.h as well here...
> > That's the primary reason for the #include churn.  Now gimple.h is included
> > where it is needed rather than blanket including it with gimplfy.h.
> >
> > I also trimmed the #include list in gimplify.c and gimplify-be.c to only
> > include what is actually required.
> >
> > Next I will clean up what remains of gimple.h, and flatten it. Then the
> > gimple refactoring is done for now.
> >
> > patch 1 is the core changes
> > patch2 contains the resulting include changes.
> >
> > Bootstrapped on x86_64-unknown-linux-gnu with no new regressions, and stage
> > 1 built for all targets to confirm those changes.
> >
> > OK?
> Eh, it's not "backend", it's "middle-end" please.  And that should include
> gimple_regimplify_operands.

OK, I have to expose rhs_predicate_for() in gimplify.h to do that, but 
its not a big deal.  done.

>      GS_ALL_DONE = 1     /* The expression is fully gimplified.  */
>    };
> + /* Gimplify hashtable helper.  */
> +
> + struct gimplify_hasher : typed_free_remove <elt_t>
> + {
> +   typedef elt_t value_type;
>
> watch out for missing vertical space when cut & pasting (just look over
> your own patches).

Just missed that one.   These gimlpify-hasher routines seem to move 
safely into gimplify.c.

> Why put this in a header?  That's super-ugly - this all should be
> private to gimplifciation.
>
> + /* Return true if gimplify_one_sizepos doesn't need to gimplify
> +    expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize
> +    fields).  */
> + static inline bool
> + is_gimple_sizepos (tree expr)
>
> likewise.  And in C++ times it's now plain 'inline', not 'static inline'.
Yeah, except this routine is also used outside of gimplifcation at the 
moment, in tree.c of all places... for variably_modified_type_p().  
Tackling that issue is a little outside the scope of what I'm doing at 
this moment :-P.

So does that mean when "processing" an include file, we generally ought 
to strip out any 'static inline' and just make them 'inline' now?


> Oh, I see you moved it from gimple.h - oh well.
>
> Thus, ok with s/gimplify-be.[ch]/gimplify-me.[ch]/ (still ugly name).
yeah... I'm open to alternate suggestions :-)  I eventually hope to be 
rid of the file anyway....

Andrew