On Mon, Oct 7, 2013 at 3:39 PM, Andrew MacLeod <amacl...@redhat.com> wrote: > On 10/07/2013 04:44 AM, Richard Biener wrote: >> >> On Thu, Oct 3, 2013 at 4:11 AM, Andrew MacLeod <amacl...@redhat.com> >> wrote: >>> >>> this patch consolidates tree-ssa-loop*.c files with new .h files as >>> required >>> (8 in total) >>> >>> A number of the prototypes were in tree-flow.h, but there were also a few >>> in >>> cfgloop.h. tree-ssa-loop.h was created to contain a couple of common >>> structs and act as the gathering file for any generally applicable >>> tree-ssa-loop includes. tree-flow.h includes this file for now. >>> >>> There is a bit of a criss-cross mess between the cfg-* and tree-ssa-loop* >>> routines, but I'm not touching that for now. Some of that might have to >>> get >>> resolved when I try to remove tree-flow.h as a standard include file from >>> the .c files.. we'll see. >>> >>> In particular, tree-ssa-loop-niter.h exports a lot of more generally used >>> routines. loop-iv.c, loop-unroll.c and loop-unswitch.c needed to include >>> it. >>> >>> A few routines werent referenced outside their file so I made those >>> static, >>> and there was one routine stmt_invariant_in_loop_p wich was actually >>> unused. >>> >>> bootstraps on x86_64-unknown-linux-gnu and passes with no new >>> regressions. >>> OK? >> >> + enum tree_code cmp; >> + }; >> + >> + #include "tree-ssa-loop-im.h" >> + #include "tree-ssa-loop-ivcanon.h" >> >> what's the particular reason to not do includes first? That looks really >> odd (and maybe rather than doing that these includes should be done >> elsewhere, like in .c files or in a header that includes tree-ssa-loop.h). > > > I had sent a followup patch because I didn't like it either :-). required > just a little shuffling since a couple of them used the typedefs declared > before them. > >> >> You seem to export things like movement_possibility that are not >> used outside of tree-ssa-loop-im.c (in my tree at least). > > > Hmm, seems I missed that one somehow... I've been checking every function > and enum, but seemed to have managed to miss that one somehow. easy to fix. > > Yes, both those should go into the tree-ssa-loop-im.c > > >> >> Other than that the single reason we have to have all these exports >> is that loop passes have their pass structures and gate / entries >> defined in tree-ssa-loop.c instead of in the files the passes reside. >> Consider changing that as a preparatory patch - it should cut down >> the number of needed new .h files. >> > Yeah, good observation. And by shuffling a few other exported routines > which are more generally used into tree-ssa-loop.c: > for_each_index, lsm_tmp_name_add, gen_lsm_tmp_name, get_lsm_tmp_name, > tree_num_loop_insns and enhancing get_lsm_tmp_name() to accept a suffix > parameter, those loop-im and loop-ivcanon no longer need a .h file either. > > I moved the passes out of tree-ssa-loop.c that would cause new .h files > unnecessarily, but left the others since that is orthogonal and could be > followed up later. > > I just bundled it all together since it changes the original 2 patches a > bit. > > Bootstraps on x86_64-unknown-linux-gnu, testsuite regressions running. > Assuming they pass fine, OK?
Hm. Index: loop-iv.c =================================================================== *** loop-iv.c (revision 203243) --- loop-iv.c (working copy) *************** along with GCC; see the file COPYING3. *** 62,67 **** --- 62,68 ---- #include "df.h" #include "hash-table.h" #include "dumpfile.h" + #include "tree-ssa-loop-niter.h" loop-iv.c is RTL land (likewise loop-unroll.c and loop-unswitch.c), why do they need tree-ssa-loop-niter.h? Apart from that the patch is ok. Thanks, Richard. > Andrew
