Re: flatten expr.h (version 2)

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

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 ?

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.