[patch] Add tree-ssa-loop.h and friends.

Andrew MacLeod amacleod@redhat.com
Tue Oct 8 11:38:00 GMT 2013


On 10/08/2013 05:57 AM, Richard Biener wrote:
> On Mon, Oct 7, 2013 at 3:39 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
>> On 10/07/2013 04:44 AM, Richard Biener wrote:
>>> On Thu, Oct 3, 2013 at 4:11 AM, Andrew MacLeod <amacleod@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.
>
we've got a bit of a mess in a number of places... I've cleaned up a few 
of the easy ones I found.

This file is required for  record_niter_bound(), 
max_loop_iterations_int() and estimated_loop_iterations_int().. so all 
bounds estimations.  There was enough of an infrastructural requirement 
for these routines within the file that I decided it was beyond the 
scope of what I'm doing at the moment to split them out.

Eventually I'd like to break this up into modules and make sure that 
thing aren't creeping in from the wrong places, and restructure a 
bit...  Loop suffer from that (like here), cfg has a couple of places 
where either rtl or generic cfg routines care calling into a routine 
that can understand SSA. I noted one in one of the earlier patches that 
I'll have to get back to.    This sort of thing has in fact prompted me 
to start looking now at our include web and what should be within the 
logical modules so we can more easily identify these sorts of things.

Too many things to clean up!! I think I could be doing this sort of 
thing for the next 8 months easily :-)

I'd prefer to come back and revisit this with a followup to try to split 
tree-ssa-loop-niter.c into another component, maybe loop-niter.[ch]...

Andrew




More information about the Gcc-patches mailing list