Flatten tree.h and tree-core.h

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:

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.  At the moment, no 
one except tree.h should have to.   maybe we just clamp on that?
#ifndef GCC_TREE_H
#error tree-core.h should only be included from tree.h
#endif

That doesn't actually prevent someone from including it, but if it is 
included, it would have to be included after tree.h, making it a no-op 
anyway.  we ought to be able to police ourselves for those.

If we allow it in a few other places temporarily like expr.h until 
flattened, just code that into the #ifndef.. ie
#if  !defined GCC_TREE_H && !defined GCC_EXPR_H
and remove the clause when flattening happens.

Hows that seem? It just leaves all the onus in tree-core.h instead of 
pushing it out to the place(s) that use it.

Andrew

On 12/19/2014 08:44 AM, Andrew MacLeod 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).

I also meant to mention that I've been trying to make flattening 
plugin-neutral these days too. so when a file is flattened, replicate 
any includes that are removed to gcc-plugin.h as well.. if possible.  
We're truning it into a giant accumulator that a plugin can use across 
releases to shield them from include restructuring as much as possible.

Anyway, it looks like tree-core.h provided a few that aren't currently 
in gcc-plugin.h:

#include "statistics.h"
#include "double-int.h"
#include "real.h"
#include "fixed-value.h"
#include "alias.h"
#include "flags.h"
#include "symtab.h"

and tree.h provided:

#include "wide-int.h"
#include "inchash.h"
#include "fold-const.h"

Maybe add those to gcc-plugin.h and make sure it compiles OK?

That way any plugin that use to include tree.h will still compile 
without having to figure out what include files are missing again. I'm a 
bit surprised the plugin testcases worked, presumably some of the 
includes weren't critical to parsing  tree.h or were included elsewhere.

Thanks
Andrew

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

Richard.

 At the moment, no 
>one except tree.h should have to.   maybe we just clamp on that?
>#ifndef GCC_TREE_H
>#error tree-core.h should only be included from tree.h
>#endif
>
>That doesn't actually prevent someone from including it, but if it is 
>included, it would have to be included after tree.h, making it a no-op 
>anyway.  we ought to be able to police ourselves for those.
>
>If we allow it in a few other places temporarily like expr.h until 
>flattened, just code that into the #ifndef.. ie
>#if  !defined GCC_TREE_H && !defined GCC_EXPR_H
>and remove the clause when flattening happens.
>
>Hows that seem? It just leaves all the onus in tree-core.h instead of 
>pushing it out to the place(s) that use it.
>
>Andrew

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

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?

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
>

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
>

On 12/19/2014 03:46 PM, Michael Collison 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?

Its only the 3 functions right?
convert_to_ptrofftype_loc(),  fold_build_pointer_plus_loc(), and 
fold_build_pointer_plus_hwi_loc() ?

And its just because they are lnlines.

The direct approach is to move them to tree.c, then exported via 
tree.h...  When I look at what they do and where they are called from,  
the better solution might be to move the functions to fold-const.c, and 
export from fold-const.h.   All they are doing are acting as wrappers 
and calling a fold_* routine...  They are also called from within 
fold-const.c as well...  Seems like maybe they belong there.  (And move 
the accompanying #defines that don't require the _loc extention to 
fold_const.h.)

This will also break the annoying dependency of tree.h needing 
fold-const.h to compile...

Andrew

Andrew,

Yes it appears those are the three offenders. I like the approach of 
moving them to fold-const.c and exporting from fold-const.h. I will try 
that approach.

On 12/19/2014 02:10 PM, Andrew MacLeod wrote:
> On 12/19/2014 03:46 PM, Michael Collison 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?
>
> Its only the 3 functions right?
> convert_to_ptrofftype_loc(),  fold_build_pointer_plus_loc(), and 
> fold_build_pointer_plus_hwi_loc() ?
>
> And its just because they are lnlines.
>
> The direct approach is to move them to tree.c, then exported via 
> tree.h...  When I look at what they do and where they are called 
> from,  the better solution might be to move the functions to 
> fold-const.c, and export from fold-const.h.   All they are doing are 
> acting as wrappers and calling a fold_* routine...  They are also 
> called from within fold-const.c as well...  Seems like maybe they 
> belong there.  (And move the accompanying #defines that don't require 
> the _loc extention to fold_const.h.)
>
> This will also break the annoying dependency of tree.h needing 
> fold-const.h to compile...
>
> Andrew
>