This is the mail archive of the
mailing list for the GCC project.
Re: [patch 1/2] tree-flow.h restructuring
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Andrew MacLeod <amacleod at redhat dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Diego Novillo <dnovillo at google dot com>
- Date: Wed, 11 Sep 2013 10:45:51 +0200
- Subject: Re: [patch 1/2] tree-flow.h restructuring
- Authentication-results: sourceware.org; auth=none
- References: <522F70C3 dot 8080906 at redhat dot com>
On Tue, Sep 10, 2013 at 9:19 PM, Andrew MacLeod <firstname.lastname@example.org> wrote:
> Here's a start at restructuring the whole tree-flow.h mess that we created
> back in the original wild west tree-ssa days.
> First, almost everyone includes tree-flow.h because it became the kitchen
> sink of functionality. Really, what we ought to have is a tree-ssa.h which
> anything that uses basic tree-ssa functionality includes, and that'll be the
> main include for SSA passes. Other prototypes and such should come from
> other appropriate places. Doing this in one step is basically impossible. so
> here is that first few steps, structured so that it's easier to review.
> I changed everywhere which includes tree-flow.h to include tree-ssa.h
> instead. tree-ssa.h includes tree-flow.h first, which makes it
> functionally the same from a compiling point of view.
You mean that doing the restructure and only adding includes that are
required to make compile work again wasn't feasible...?
> I also moved everything from tree-flow.h and tree-flow-inline.h that is
> related to tree-ssa.c functions into tree-ssa.h. There were also a few
> function prototypes sprinkled in a couple of other headers which I moved
> into tree-ssa.h as well. I have verified that every exported function in
> tree-ssa.c has a proto in tree-ssa.h, and is listed in the same order as the
> .h file.
Note that in general there isn't a thing like "tree SSA" anymore - it's
"GIMPLE SSA" now ;) Not that we want gigantic renaming occuring now,
just if you invent completely new .[ch] files then keep that in mind.
That is, if you moved stuff to tree-ssa.h that has no definition in tree-ssa.c
then I'd rather have a new gimple-ssa.h that doesn't have a corresponding
.c file (for now).
> Compiling this change indicated that there were a few files which required
> functionality from tree-ssa.c which really don't belong there. In
> particular, useless_type_conversion_p and types_compatible_p are used by
> the C++ front end and other places which don't really care about SSA. So I
> moved them into more appropriate places... tree.c and tree.h
Well, those functions are only valid when we are in GIMPLE (the GIMPLE
type system is less strict than the GENERIC one), so tree.[ch] isn't
gimple.[ch] would be. But I wonder where it's used from the FEs - that's surely
looking wrong (unless it's in a langhook). But yes, the functions are not
in any way about SSA but only GIMPLE.
For now I'd say just keep them in tree-ssa.[ch] or move them to gimple.[ch].
> We'll continue moving stuff out of tree-flow.h into appropriate places,
> until all that is left makes sense where it is and doesn't include
> prototypes from other .c files.
> Once that is finished, I will go back and revisit tree-ssa.h to see what it
> actually needs for includes. Then visit every .c file which includes
> tree-ssa.h, remove it from the include list, compile and see what routines
> the file is looking for, then include the appropriate file(s). This will
> likely identify other things like types_compatible_p() which are really in
> the wrong place, and those can then be moved.
> This patch implements that starting point, and its the most painful.
> The next one shows how we proceed and is much easier.
> Do we want to proceed this way? It seems reasonable to me.
> The changes bootstrap fine, I haven't finished running the regression tests
> yet... I wanted to see any comments before proceeding.
Apart from that (well, I can certainly bear with the bulk include change...)
the rest looks fine.