Re: flatten expr.h (version 2)

[Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>]

On 14 January 2015 at 17:31, Richard Biener <rguenther@suse.de> wrote:
> On Wed, 14 Jan 2015, Prathamesh Kulkarni wrote:
>
>> On 14 January 2015 at 11:16, Prathamesh Kulkarni
>> <prathamesh.kulkarni@linaro.org> wrote:
>> > On 14 January 2015 at 16:40, Prathamesh Kulkarni
>> > <prathamesh.kulkarni@linaro.org> wrote:
>> >> On 14 January 2015 at 14:34, Richard Biener <rguenther@suse.de> wrote:
>> >>> On Wed, 14 Jan 2015, Prathamesh Kulkarni wrote:
>> >>>
>> >>>> On 13 January 2015 at 22:02, Prathamesh Kulkarni
>> >>>> <prathamesh.kulkarni@linaro.org> wrote:
>> >>>> > On 13 January 2015 at 16:06, Prathamesh Kulkarni
>> >>>> > <prathamesh.kulkarni@linaro.org> wrote:
>> >>>> >> On 13 January 2015 at 15:34, Richard Biener <rguenther@suse.de> wrote:
>> >>>> >>> On Sun, 11 Jan 2015, Prathamesh Kulkarni wrote:
>> >>>> >>>
>> >>>> >>>> Hi,
>> >>>> >>>> This is a revamped expr.h flattening flattening patch rebased on
>> >>>> >>>> tree.h and tree-core.h flattening patch (r219402).
>> >>>> >>>> It depends upon the following patch to get committed.
>> >>>> >>>> https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00565.html
>> >>>> >>>>
>> >>>> >>>> Changes:
>> >>>> >>>> * Removed all includes except tree-core.h. Put includes required by
>> >>>> >>>> expr.h in a comment.
>> >>>> >>>> * Moved stmt.c, expmed.c prototypes to stmt.h, expmed.h respectively.
>> >>>> >>>> * Adjusted generator programs: genemit.c, gengtype.c, genopinit.c, genoutput.c.
>> >>>> >>>> * Did not put includes in gcc-plugin.h since expr.h cannot be included
>> >>>> >>>> by plugins
>> >>>> >>>> (putting them broke building a file in c-family/ since expr.h is not
>> >>>> >>>> allowed in front-ends)
>> >>>> >>>> * Affects java front-end (expr.h is allowed in java front-end).
>> >>>> >>>>
>> >>>> >>>> Bootstrapped and tested on x86_64-unknown-linux-gnu with languages:
>> >>>> >>>> all,go,ada,jit
>> >>>> >>>> Built on all targets in config-list.mk with languages: all, go.
>> >>>> >>>> OK to commit ?
>> >>>> >>>
>> >>>> >>> diff --git a/gcc/expr.c b/gcc/expr.c
>> >>>> >>> index fc22862..824541e 100644
>> >>>> >>> --- a/gcc/expr.c
>> >>>> >>> +++ b/gcc/expr.c
>> >>>> >>> @@ -41,11 +41,17 @@ along with GCC; see the file COPYING3.  If not see
>> >>>> >>>  #include "regs.h"
>> >>>> >>>  #include "hard-reg-set.h"
>> >>>> >>>  #include "except.h"
>> >>>> >>> -#include "input.h"
>> >>>> >>>  #include "function.h"
>> >>>> >>>  #include "insn-config.h"
>> >>>> >>>  #include "insn-attr.h"
>> >>>> >>>  /* Include expr.h after insn-config.h so we get HAVE_conditional_move.
>> >>>> >>> */
>> >>>> >>> +#include "hashtab.h"
>> >>>> >>> +#include "emit-rtl.h"
>> >>>> >>> +#include "expmed.h"
>> >>>> >>> +#include "stmt.h"
>> >>>> >>> +#include "statistics.h"
>> >>>> >>> +#include "real.h"
>> >>>> >>> +#include "fixed-value.h"
>> >>>> >>>  #include "expr.h"
>> >>>> >>>
>> >>>> >>> Please move the comment to the proper place
>> >>>> >> ah, my flattening tool doesn't look at comments. I will move the
>> >>>> >> comment before expr.h include, thanks.
>> >>>> >>>
>> >>>> >>> diff --git a/gcc/expr.h b/gcc/expr.h
>> >>>> >>> index a7638b8..f1be8dc 100644
>> >>>> >>> --- a/gcc/expr.h
>> >>>> >>> +++ b/gcc/expr.h
>> >>>> >>> @@ -20,7 +20,8 @@ along with GCC; see the file COPYING3.  If not see
>> >>>> >>>  #ifndef GCC_EXPR_H
>> >>>> >>>  #define GCC_EXPR_H
>> >>>> >>>
>> >>>> >>> -/* For inhibit_defer_pop */
>> >>>> >>> +/* expr.h required includes */
>> >>>> >>> +#if 0
>> >>>> >>>  #include "hashtab.h"
>> >>>> >>>  #include "hash-set.h"
>> >>>> >>>  #include "vec.h"
>> >>>> >>> @@ -29,15 +30,17 @@ along with GCC; see the file COPYING3.  If not see
>> >>>> >>>  #include "hard-reg-set.h"
>> >>>> >>>  #include "input.h"
>> >>>> >>>  #include "function.h"
>> >>>> >>> -/* For XEXP, GEN_INT, rtx_code */
>> >>>> >>>  #include "rtl.h"
>> >>>> >>> -/* For optimize_size */
>> >>>> >>>  #include "flags.h"
>> >>>> >>> -/* For tree_fits_[su]hwi_p, tree_to_[su]hwi, fold_convert, size_binop,
>> >>>> >>> -   ssize_int, TREE_CODE, TYPE_SIZE, int_size_in_bytes,    */
>> >>>> >>>  #include "tree-core.h"
>> >>>> >>> -/* For GET_MODE_BITSIZE, word_mode */
>> >>>> >>>  #include "insn-config.h"
>> >>>> >>> +#include "alias.h"
>> >>>> >>> +#include "emit-rtl.h"
>> >>>> >>> +#include "expmed.h"
>> >>>> >>> +#include "stmt.h"
>> >>>> >>> +#endif
>> >>>> >>>
>> >>>> >>> Err, please remove the #if 0 section
>> >>>> >> I kept it because if something breaks later (hopefully not!), it will
>> >>>> >> be easier to fix.
>> >>>> >> I will remove it.
>> >>>> >>>
>> >>>> >>> +
>> >>>> >>> +#include "tree-core.h"
>> >>>> >>>
>> >>>> >>> Why?  The original comment says
>> >>>> >>>
>> >>>> >>> -/* For tree_fits_[su]hwi_p, tree_to_[su]hwi, fold_convert, size_binop,
>> >>>> >>> -   ssize_int, TREE_CODE, TYPE_SIZE, int_size_in_bytes,    */
>> >>>> >>>
>> >>>> >>> but all those are declared in tree.h.  Which means the files including
>> >>>> >>> expr.h must already include tree.h.
>> >>>> >>>
>> >>>> >>> If that's not the reason we need to include tree-core.h from expr.c
>> >>>> >>> please add a comment explaining why.
>> >>>> >> bt-load.c fails to compile because it includes expr.h but does not
>> >>>> >> include tree.h
>> >>>> >> I will place tree.h include in all files that include expr.h and rebuild.
>> >>>> > This is not going to work, since tree.h is now flattened. Shall also
>> >>>> > require including all headers required by
>> >>>> > tree.h in all files that include expr.h. Could we retain tree-core.h
>> >>>> > in expr.h for now ?
>> >>>> > Or should I insert tree.h (along with tree.h required includes) in all
>> >>>> > files that include expr.h ?
>> >>>> I am including tree.h along with required includes in all files that
>> >>>> include expr.h.
>> >>>> This removes all includes from expr.h.
>> >>>
>> >>> Good.
>> >> The attached patch, refactors expr.h so that it only exports
>> >> prototypes from expr.c.
>> >> I kept int_expr_size() in explow.c instead of moving it to expr.c because it
>> >> called tree_expr_size() which is static in explow.c and has other callers too.
>> > Not really. It only has two callers expr_size() and int_expr_size().
>> > I will move definitions of int_expr_size(), tree_expr_size() and
>> > expr_size() to expr.c
>> > in followup patch.
>> This version moves the definitions of int_expr_size(), tree_expr_size() and
>> expr_size() to expr.c.
>> Bootstrapped on x86_64-unknown-linux-gnu.
>> Please let me know if there are any other changes required.
>> If the patch is fine, I will restart building on all targets in config-list.mk
>
> The new dojump.h and explow.h headers requires a copyright notice (see
> other headers for a boilerplate to copy)
Thanks.
>
> Otherwise this looks ok.
Thanks, I will start building on all targets in config-list.mk and
assuming there
are no build errors, shall I commit it ?

Regards,
Prathamesh
>
> Thanks,
> Richard.
>
>
>> Thanks,
>> Prathamesh
>> >>
>> >> Changes:
>> >> * New header files dojump.h, explow.h.
>> >> * Modified generator programs: gengtype.c, genemit.c, genopinit.c,
>> >> genoutput.c, config/tilepro/gen-mul-tables.cc.
>> >> * Affects java front-end (java/builtins.c includes expr.h).
>> >>
>> >> Bootstrapped and tested on x86_64-unknown-linux-gnu with languages:
>> >> all, go, ada, jit, obj-c++.
>> >> Build in progress on all targets in config-list.mk with languages: all, go.
>> >> Assuming the build goes fine, OK to commit ?
>> >>
>> >> Thanks,
>> >> Prathamesh
>> >>>
>> >>> Richard.
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
> Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)