On Tue, Dec 4, 2012 at 4:23 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Mon, Dec 3, 2012 at 9:14 PM, Lawrence Crowl <cr...@googlers.com> wrote: >> On 12/3/12, Diego Novillo <dnovi...@google.com> wrote: >>> On 2012-12-01 20:44 , Lawrence Crowl wrote: >>>> Index: gcc/gimple-fold.c >>>> =================================================================== >>>> --- gcc/gimple-fold.c (revision 193902) >>>> +++ gcc/gimple-fold.c (working copy) >>>> @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. >>>> #include "tree-ssa-propagate.h" >>>> #include "target.h" >>>> #include "gimple-fold.h" >>>> +#include "gimplify-ctx.h" >>>> >>>> /* Return true when DECL can be referenced from current unit. >>>> FROM_DECL (if non-null) specify constructor of variable DECL was >>>> taken from. >>>> Index: gcc/tree-mudflap.c >>>> =================================================================== >>>> --- gcc/tree-mudflap.c (revision 193902) >>>> +++ gcc/tree-mudflap.c (working copy) >>>> @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. >>>> #include "ggc.h" >>>> #include "cgraph.h" >>>> #include "gimple.h" >>>> +#include "gimplify-ctx.h" >>>> >>>> extern void add_bb_to_loop (basic_block, struct loop *); >>>> >>>> Index: gcc/tree-inline.c >>>> =================================================================== >>>> --- gcc/tree-inline.c (revision 193902) >>>> +++ gcc/tree-inline.c (working copy) >>>> @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. >>>> #include "value-prof.h" >>>> #include "tree-pass.h" >>>> #include "target.h" >>>> +#include "gimplify-ctx.h" >>> >>> I don't follow. It seems that factoring into gimplify-ctx.h does >>> not actually buy much. The files using it are just including >>> *another* file. Whereas previously, they were getting that >>> content from gimple.h. >>> >>> Unless we can stop including gimple.h from these files, I don't >>> see a lot of gain in this factoring. Am I missing something? >> >> There at least 70 files that include gimple.h, and only 5 that need >> gimple-ctx.h. By splitting it out, at least 65 files will not need >> to parse the gimplify_ctx struct, the gimple_temp_hash_elt struct, >> the gimplify_hasher template struct, and may not need to include >> hash-table.h. >> >> It's all about avoiding superfluous compilation in other files. > > But it's backward - gimple.h is the core file as it provides GIMPLE. > The gimplifier and its context certainly requires to know about > GIMPLE. > > Btw, if we still want to split it I'd rather have gimplify.h (and also > put all exports from gimplify.c there, not only gimplify_ctx). Still > I don't think it would buy us much.
We talked offline about this with Lawrence. I was not too convinced about adding this new header file, it seemed forced but I was struggling with a better alternative. Perhaps gimplify.h is a better way, but I'm not sure either. We decided that since it's in the branch, it may not matter much for now. However, another thing occurred to me in the meantime: merges. The new file will probably cause merge problems. If we are not completely convinced that it's a good change, we are making our life harder for no gain. Lawrence, you're going to hate me, but would you mind just leaving the ctx structure in gimple.h for now? Sorry! I think it's going to be easier to just leave it there for now until we come up with a good split for the constructs in that file. Diego.
