On 09/13/2013 03:54 AM, Richard Biener wrote:
On Thu, Sep 12, 2013 at 11:09 PM, Andrew MacLeod <amacl...@redhat.com> wrote:
There are 2 parts of tre-ssa-ter.c to address.
is_replaceable_p() is also used in expr.c, It has a flag to indicate where
its being called from, and we do different checks for each one. There is a
wrapper function stmt_is_replaceable_p() in tree-ssa-ter.c which hides the
setting of the flag to expr.c. Most of the function is common, so I
extracted out the common part, and put it in tree-ssa.c.
I then moved stmt_is_replaceable() to expr.c, has it call the common routine
and then added the extra bit it needs there. Similarly tree-ssa-ter.c gets
ter_is_replaceable_p() which calls the common part, and then does its own
special checking. This removes that general export and messiness from
tree-ssa-ter.c
I think I got the logic of the function right, but you might want to double
check... It was giving me a headache when I split it :-)
Unfortunately, tree-ssa-ter.c also has 2 functions
(find_replaceable_exprs() and dump_replaceable_exprs()) which are exported
and utilized by tree-outof-ssa.c (the file is a part of the out-of-ssa
module). So I moved the prototypes from tree-ssa-live.h into a newly
created tree-ssa-ter.h file, and included it directly from
tree-outof-ssa.c, the only consumer.
I also could have just left the is_replaceable_p() function as is, put the
prototype and the 'stmt_is_replaceable_p()' wrapper function in
tree-ssa-ter.h, and then included that from expr.c... but it just seems
like an odd thing to include directly there.... but that is an option...
eventually we mighty want to look at splitting expr.c.. it seems a bit
multi-personality with some pure RTL, some tree/rtl and some ssa... even
though they all serve the same ultimate function, it is 11,000+ lines long
now :-) . A task for another day.
is_replaceable_p and friends is purely specific to the area of RTL expansion,
so putting it in tree-ssa.[ch] is definitely wrong. It doesn't make sense to
use it anywhere else.
The main driver of the RTL expansion process is in cfgexpand.c (the expand
pass itself) in gimple_expand_cfg. We have one header file related to
the RTL expansion process and is_replaceable_p would simply fit in
ssaexpand.h (which doesn't have a .c file, so leaving the stuff in the .c files
where they are now is ok).
So - I fear you have to re-do this patch (in a much simpler way).
I actually figured as much :-) I had actually done that split before I
remembered there were other exports, and considered undoing that part,
but figured since I had done it I'd get opinion :-)
Or are you suggesting we also bail on tree-ssa-ter.h and put those
prototypes in ssaexpand.h?
If we want to make this fully consistent (.h's match .c exports) , I
could put is_replaceable_p() into a new ssaexpand.c, move the 3
tree-outof-ssa prototypes in ssaexpand.h into tree-outof-ssa.h, and have
ssaexpand.h include tree-ssa-ter.h and tree-outof-ssa.h. That would
then be clean. and consistent.
actually, we could just make is_replaceable_p() static inline in
ssaexpand.h... thats not unreasonable either, but we'll probably find
functions which belong in ssaexpand.c sooner or later.
No worries, I figured these first few patches would be slower and more
painful until a reasonable formula was determined :-)
Andrew
