On 20 December 2014 at 02:16, Michael Collison <michael.colli...@linaro.org> wrote: > The reason I included tree-core.h in all the .c files was the requirement in > tree.h (now flattened to the .c files) for fold-const.h. In tree.h there are > inline functions such as fold_build_pointer_plus_hwi_loc which reference > functions in fold-const.h. The moment you include fold-const.h you require > the 'enum tree_code' definition from tree-core.h for fold-const.h. > > How would you suggest I get around this? Could you put a forward declaration for enum tree_code in fold-const.h: enum tree_code; I believe tree and const_tree are in coretypes.h
Thanks, Prathamesh > > > On 12/19/2014 11:24 AM, Andrew MacLeod wrote: >> >> On 12/19/2014 01:12 PM, Richard Biener wrote: >>> >>> On December 19, 2014 2:44:00 PM CET, Andrew MacLeod <amacl...@redhat.com> >>> wrote: >>>> >>>> On 12/19/2014 06:20 AM, Richard Biener wrote: >>>>> >>>>> On Fri, Dec 19, 2014 at 1:37 AM, Michael Collison >>>>> <michael.colli...@linaro.org> wrote: >>>>>> >>>>>> This patch flattens tree.h and tree-core.h. This work is part of the >>>> >>>> GCC >>>>>> >>>>>> Re-Architecture effort being led by Andrew MacLeod. >>>>>> >>>>>> I removed the includes in tree.h and tree-core.h except for the >>>> >>>> include of >>>>>> >>>>>> tree-core.h in tree.h. >>>>>> >>>>>> I modified genattrtab.c, genautomata.c, genemit.c, gengtype.c, >>>> >>>> gengtype.c, >>>>>> >>>>>> genoptinit.c, genoutput.c, >>>>>> genpeep.c, genpreds.c, and optc-save-gen-awk to include the the >>>> >>>> necessary >>>>>> >>>>>> include files removed from >>>>>> tree.h and tree-core.h when generating their respective files. >>>>>> >>>>>> All other changes include the necessary include files removed from >>>> >>>> tree.h >>>>>> >>>>>> and tree-core.h. Note the patches modifies all the front-ends. >>>>>> >>>>>> I bootstrapped on x86 with all languages. I also bootstrapped on all >>>> >>>> targets >>>>>> >>>>>> listed in contrib/config-list.mk with c and c++ enabled. >>>>>> >>>>>> Is this okay for trunk? >>>>> >>>>> No - you need to rework it (substantially?). Nothing but tree.h (and >>>> >>>> gimple.h) >>>>> >>>>> may include tree-core.h directly. Instead where you added includes >>>> >>>> of >>>>> >>>>> tree-core.h you need to include tree.h. Basically tree-core.h is an >>>>> implementation >>>>> detail introduced to hide layer violations where we understand them >>>> >>>> (gimple.h). >>>> Hmm, yeah. tree-core.h is included in tree.h as planned, but then it's >>>> copy out to all the include locations anyway... maybe you just have to >>>> remove the #include "tree-core.h"'s? >>>> >>>> It also looks to me like the include ordering is still "off". ie: >>>> >>>> tree.h: >>>> >>>> #include "tree-core.h" >>>> -#include "hash-set.h" <--- 1 >>>> -#include "wide-int.h" <--- 2 >>>> -#include "inchash.h" <---3 >>>> - >>>> -/* These includes are required here because they provide declarations >>>> - used by inline functions in this file. >>>> - >>>> - FIXME - Move these users elsewhere? */ >>>> -#include "fold-const.h" <--- 4 >>>> >>>> and when the includes are flattened: >>>> >>>> --- a/gcc/varasm.c >>>> +++ b/gcc/varasm.c >>>> @@ -30,15 +30,22 @@ along with GCC; see the file COPYING3. If not see >>>> #include "coretypes.h" >>>> #include "tm.h" >>>> #include "rtl.h" >>>> +#include "hash-set.h" <--- 1 included earlier by tree-core.h, >>>> ordering still OK >>>> +#include "machmode.h" >>>> +#include "vec.h" >>>> +#include "double-int.h" >>>> +#include "input.h" >>>> +#include "alias.h" >>>> +#include "symtab.h" >>>> +#include "tree-core.h" >>>> +#include "fold-const.h" <--- 4 included earlier >>>> +#include "wide-int.h" <--- 2 >>>> +#include "inchash.h" <---3 >>>> #include "tree.h" >>>> #include "stor-layout.h" >>>> >>>> >>>> Now it may not matter for these particular includes since I doubt there >>>> >>>> are any cross dependencies, but the net result is this will see files >>>> included in a different order. I raise the point because for some files >>>> >>>> going forward it does matter... tm.h in particular is critical since >>>> other files do condition compilation on macros defined in that file. >>>> We >>>> don't eliminate unnecessary include files any more because we haven't >>>> done the analysis to know which ones are "important" yet and changing >>>> their order may change code generation, but not compilation success. >>>> >>>> we really need to get that tool going so we can reduce the include >>>> list... :-) >>>> >>>> Maybe there is a small bug in the include replicating tool? Just >>>> wondering why the ordering is arbitrarily different here. >>>> >>>> That said, we wont be able to keep it exact around tree-core.h. if we >>>> >>>> remove the #include "tree-core.h" from varasm.c those 3 includes I >>>> tagged will be seen *before* any of the code in tree-core.h is seen, >>>> but >>>> that case should be OK. Before we split tree-core.h from tree.h they >>>> were all included before the contents were seen, so we are in fact just >>>> >>>> restoring the previous ordering :-) >>>> >>>>> I suppose we should add >>>>> >>>>> #ifndef I_MAY_INCLUDE_TREE_CORE_H >>>>> #error Bad guy! >>>>> #endif >>>>> >>>>> to tree-core.h and define/undef that around the very few includes of >>>> >>>> tree-core.h >>>>> >>>>> we want to allow. I see we already include it in more places than >>>> >>>> desired. >>>>> >>>>> So - can you do a preparatory patch doing that and removing the >>>> >>>> tree-core.h >>>>> >>>>> include from everything but tree.h (allow it from expr.h until that >>>>> got flattened)? >>>>> >>>>> Andrew, I suppose my recollection of how we architected tree.h and >>>> >>>> tree-core.h >>>>> >>>>> is correct? >>>> >>>> Pretty much. tree-core.h was simply a split of tree.h to separate the >>>> structure definitions away from the accessors macros so we could >>>> provide >>>> alternative means of getting at the bits. It was never intended to be >>>> used directly, although I suppose there are occasions where it could be >>>> >>>> useful.... In theory those places that include tree-core.h directly >>>> now, and don't include tree.h could simply include tree.h. I guess a >>>> good question would be why are those files caring about the contents of >>>> >>>> tree-core.h but not tree.h... there might be something that could be >>>> factored out so they don't even need tree-core.h. (maybe they don't >>>> now....?! ) using tree.h ought to be fine tho. >>>> >>>> gimple.h doesnt need to include tree-core.h either. >>> >>> That's because it still exposes interfaces with trees, thus tree.h is >>> needed anyways. >>> Your idea was to abstract that away. >>> My point then was that we should disallow tree.h from files using >>> gimple.h. >>> For that to work without actually implementing non-tree data structures >>> Gimple.h needs to be able to look at tree >>> Implementation details. Thus tree-core.h. >>> >>> At least if that still us your plan.. >> >> >> Yes, I mean for the time being gimple.h doesn't need to look at >> tree-core.h directly. The day it is, we can add the appropriate check to >> tree-core.h to allow it... But that is some time away yet. Until then, >> tree.h is the only thing that really needs to be looking at tree-core.h... >> >> Andrew >> > > -- > Michael Collison > Linaro Toolchain Working Group > michael.colli...@linaro.org >