Re: RFC - Next refactoring steps

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

On Thu, Sep 5, 2013 at 2:57 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
> Now that tree.h is split in two, there are some follow up things that will
> facilitate the deforestation of gimple.  I've also thrown in a couple of
> structuring issues for good measure.
>
> What are your thoughts on these subjects?

Jumping in from the side I suggest you start de-foresting existing headers
where they say

/* In foobar.c */
...

to simply add a foobar.h with the contents that follow.  Bonus points if you
actually verify all definitions from X.c are declaed in X.h (the /* In ... */
annotations are hopelessly out-of-date in some cases).

More bonus points if you avoid pass-xyz.h files but instead move code
useful to multiple passes to more appropriate places.  That said, definitely
avoid pass-xyz.h ;)

Richard.

> 1 -  I think we ought to split out the data structures from gimple.h into
> gimple-core.h, like we did with tree.h
>
> What is left as gimple.[ch] which should really be called gimple-stmt.[ch]
> since it implements just the tcc_statement class of trees.  we could do that
> change now, or I'm ok leaving it like that for a while since I can just
> include gimple-type.h and gimple-decl.h and other new files from the top of
> gimple.h.  That won't  really affect my work.  I think it probably ought to
> be done for clarity eventually.    gimple.h would then simply become a
> collector of "gimple-blah.h" files which are required for a complete gimple
> system.
>
> 2 - all the uses of TREE_NULL to refer to an empty/nonexistent object...  it
> is no longer in tree-core.h, which I think is correct. what should we start
> using instead in converted files?  options are:
>   a)  0
>   b) NULL
>   c) something we define as 0, like GIMPLE_NULL
>
> I prefer a) I think, since its consistent with things like if (!ptr), but b)
> is fine as well.   I'm not too fond of option c). I figured we'd see what
> others like... maybe something else? :-)
>
> 3) remove tree.h from other .h files
>   Now that tree.h is split, there are a few .h files which directly include
> tree.h themselves. It would be nice if we could remove the implementation
> requirement of tree.h to ease moving to gimple.h. The 4 files are :
>  ipa-utils.h   lto-streamer.h  streamer-hooks.h  tree-streamer.h
> It should be possible to not directly include tree.h itself in these files.
> Worst case, the header file is included  after tree.h from the .C files..
> that seems to be the way most of the other include files avoid including
> tree.h directly.
>
>
> 4 - Access to tree checking.
>    Since trees are still being used under the covers, we still need to be
> able to do tree_checking...  I dont think we need the tree checking macros
> per se, but I do need access to the same basic checking functions.. like
> tree_check, tree-check2, tree_check_failed, etc.
>
> the basic inline tree_check* set of functions use TREE_CODE, so having
> access to them is no good anyway from a gimple.h file, so I pretty much need
> to rewrite those functions for gimple use, but there are a bunch of routines
> in tree.c that I still need the prototypes for.  Things like
> tree_class_check_failed() and tree_contains_struct_check_failed().  I dont
> see the point ina speerate header file for those, but maybe we could put the
> prototypes in tree-core.h.  I dont think I like that either.. but it is an
> option.
>
> 5 - This is more of a meta subject. but is on the radar and relates to the
> tree-checking issue.
>
> There is still some interface stuff from trees that is required by the
> gimple system, and will be as long as trees are used under the covers.  For
> example, tree.h defines STRIP_NOPS() which is used in a lot of places.   Say
> we add a strip_nops() method to the GimpleExpression class. gimple-expr.h
> needs to implement that functionality, but can't access tree.h.  tree.h
> defines STRIP_NOPS as
>     (EXP) = tree_strip_nop_conversions (CONST_CAST_TREE (EXP))
> tree_strip_nop_conversions() is defined in tree.c and the protoype is in
> tree.h.. where it belongs.
>
> So there is no way to access this function call within a file converted to
> gimple. So, we need the prototype somewhere we can get at it and call it
> from strip_nops().
>
> Other examples are the tree build routines. From gimple converted files,  we
> still need to be able to build trees under the covers. The interface will
> building gimple expressions, but under the covers the gimple implementation
> needs to be able to access those build routines.
>
> One thought I had was to provide a gimple-tree.[ch] files which wrap up all
> these sorts of issues in one place.  The gimple-tree.h file provides a
> gimple-only interface via prototypes to something like:
> void gimple_strip_nops (GimpleValue Expr);
> GimpleValue gimple_build_expression2 (GimpleCode code, GimpleValue Expr1,
> GimpleValue Expr2);
>
> Then in gimple-tree.c we break the rules.  This one .c file would include
> tree.h so that it has access to all the tree stuff, and provides
> implementations of these functions. This would be the only place that tree.h
> is included in the gimple ecosystem.  It has the drawback of an extra layer
> of functions calls for most things, but maybe thats ok for the abstraction
>
>  We could also put all the tree checking stuff here as an option. Provide a
> copy of the tree checking inlines in gimple-tree.h as well as the prototypes
> for the functions in tree.c
>
> At least then all the overlap and ickyness ends up in one place... At least
> as a start
>
> Any other thoughts on how to deal with this sort of thing?
>
> Andrew