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

[Richard Biener <richard dot guenther at gmail dot com>]

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.

    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).

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'.

Oh, I see you moved it from gimple.h - oh well.

Thus, ok with s/gimplify-be.[ch]/gimplify-me.[ch]/ (still ugly name).

Thanks,
Richard.


> Andrew
>
> PS Interestingly, many back end files still need gimplify.h, and the vast
> majority of them are actually only looking for 'unshare_expr'... which is
> used by both the front ends and back ends.  (Some front end files are also
> including gimplify.h simply to get that routine.)
>
> It is obviously a very tree specific thing since it duplicates most of a
> tree, but is only required because gimple uses unshared trees for
> expressions and such. Its not really part of the gimplification process, but
> it is utilized by it.  I am wondering if perhaps we ought to split this code
> out as well and make "tree-unshare.[ch]"...