Re: Flatten tree.h and tree-core.h

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 20 December 2014 at 02:16, Michael Collison
<michael.collison@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 <amacleod@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.collison@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.collison@linaro.org
>