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