This is the mail archive of the
mailing list for the GCC project.
Re: [patch] The remainder of tree-flow.h refactored.
- 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: Tue, 8 Oct 2013 12:22:45 +0200
- Subject: Re: [patch] The remainder of tree-flow.h refactored.
- Authentication-results: sourceware.org; auth=none
- References: <524EF235 dot 5080803 at redhat dot com>
On Fri, Oct 4, 2013 at 6:52 PM, Andrew MacLeod <email@example.com> wrote:
> This patch clears the rest of the improperly located prototypes out of
> tree-flow.h. A bit larger than the last few, but I was pushing to clear
> this up, and its not quite as bad as it seems :-)
> Of interest:
> * tree-flow.h now contains *only* prototypes for tree-cfg.c now. Next week
> I'll rename it to tree-cfg.h when I visit all the .c files which include
> tree-flow.h to see who the true consumers are now.
> * tree-ssa-address.c only had one client for get_address_description, and
> that required exposing struct mem_address in tree-flow.h. Since that call
> was followed by a call to addr_for_mem_ref with the structure, i figured it
> was better to create a new funciton which combined that functionality. NOw
> the struct is internal only.
> * ipa_pure_const.c has only a single export... warn_function_noreturn()
> which was only used in one place, tree-cfg.c to implement the
> warn_function_noreturn pass. Although the name of the file is not really
> appropriate, I moved the pass structural stuff into ipa_pure_const.c and now
> there are no exports. I looked at numerous other options, and in the end,
> this one is cleanest. There is no other good home for the bits an pieces
> that can be shuffled. maybe ipa-func-attr.c would be a better file name
> :-P but that is a miniscule issue :_)
> * tree-cfg.c had the warn_function_noreturn pass removed from it, but I
> relocated the execute_fixup_cfg routine/pass from tree-optimize.c here. It
> makes more sense here and that was the only routines being exported from
> tree-optimize.c, and tree-cfg was the only consumer.
> * tree-dump.h was explicitly prototyping dump_function_to_file() from
> tree-cfg.c, presumably to avoid including all of tree-flow.h. I removed that
> and include tree-flow.h from tree-dump.c. Seems like a backwards step
> until one realizes that tree-flow.h is now the header file for tree-cfg.c.
> * tree-ssa-dom.c::tree_ssa_dominator_optimize() was calling
> ssa_name_values.release() from tree-ssa-threadedge.c, and was the only
> consumer. The call immediately before is to threadedge_finalize_values()
> which performs that operation, I deleted the use from tree-ssa-dom.c. I
> would like to have made the vec<tree> ssa_name_values static within the file
> too, but then the SSA_NAME_VALUE macro to access it would have to make a
> call into a function in the .c file to access it, and I don't know what the
> performance hit would be... I expect it would be un-noticable to abstract
> that out... but I left it for the moment.
> * There was a bit of a mess between gimplify.c, gimple.c, and tree.c. In the
> end, I
> - relocated the tree copying and unsharing routines from gimplify.c to
> tree.c. The uses were sporadic throughout the compiler, and weren't always
> used by things that should be gimple aware. They were pure tree routines,
> so that seemed like an appropriate place for them. Maybe something like
> tree-share.[ch] would be better... that could be dealt with during a
> tree.[ch] examination.
> - force_gimple_operand_gsi_1and force_gimple_operand_gsi were the only
> routines in gimplify.c that operated with gimple statement iterators from
> gimple.c. The include files had this chicken and egg problem where I
> couldn't resolve the ordering very well. IN the end, I decided to relocate
> those 2 routines into gimple.c where there is a good understanding of
> gimple-iterators. I expect the gimple iterators will be split from
> gimple.[ch] when I process gimple.h, so they may be relocatable again later.
> - gimple.h declared some typedefs and enums that were gimplification
> specific, so I moved those to the new gimplfy.h as well.
> This boostraps on x86_64-unknown-linux-gnu and has no new regressions. OK?
graphite.h should be unnecessary with moving the pass struct like you
did for other loop opts. Likewise tree-parloops.h (well, ok, maybe
you need parallelized_function_p, even though it's implementation is
gross ;)). Likewise tree-predcom.h.
unvisit_body isn't generic enough to warrant moving out of gimplify.c
(the only user).
The force_gimple_operand_gsi... routines are in gimplify.c because they ...
gimplify! And you moved them but not force_gimple_operand[_1]!?
The rest looks reasonable (though the patch is big and I only had
a quick look).