Re: flatten expr.h (version 2)

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

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.

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
>>>
>>> -/* Definitions from emit-rtl.c */
>>> -#include "emit-rtl.h"
>>> -
>>>  /* Return a memory reference like MEMREF, but with its mode widened to
>>>     MODE and adjusted by OFFSET.  */
>>>  extern rtx widen_memory_access (rtx, machine_mode, HOST_WIDE_INT);
>>>
>>> err - functions defined in emit-rtl.c should be declared in emit-rtl.h.
>>> Please fix that first.  expr.h should _only_ contain prototypes
>>> for stuff defined in expr.c.
>> oops, missed it :(
>>>
>>> Andrew did a good job with this, first cleaning up a header moving
>>> declarations to proper places and only after that flattening it.
>>>
>>> The rest of the patch looks good to me but expr.h isn't in a good
>>> shape after it.
>> I will work on it and send patch with suggested changes by tomorrow.
>>
>> Thanks,
>> Prathamesh
>>>
>>> Thanks,
>>> Richard.