On Thu, Sep 5, 2013 at 2:57 PM, Andrew MacLeod <amacl...@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