From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20342 invoked by alias); 14 Jan 2015 12:09:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 20195 invoked by uid 89); 14 Jan 2015 12:09:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Wed, 14 Jan 2015 12:09:01 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id B156FABFC; Wed, 14 Jan 2015 12:08:57 +0000 (UTC) Date: Wed, 14 Jan 2015 12:17:00 -0000 From: Richard Biener To: Prathamesh Kulkarni cc: Michael Collison , gcc Patches , Andrew MacLeod Subject: Re: flatten expr.h (version 2) In-Reply-To: Message-ID: References: User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2015-01/txt/msg01033.txt.bz2 On Wed, 14 Jan 2015, Prathamesh Kulkarni wrote: > On 14 January 2015 at 11:16, Prathamesh Kulkarni > wrote: > > On 14 January 2015 at 16:40, Prathamesh Kulkarni > > wrote: > >> On 14 January 2015 at 14:34, Richard Biener wrote: > >>> On Wed, 14 Jan 2015, Prathamesh Kulkarni wrote: > >>> > >>>> On 13 January 2015 at 22:02, Prathamesh Kulkarni > >>>> wrote: > >>>> > On 13 January 2015 at 16:06, Prathamesh Kulkarni > >>>> > wrote: > >>>> >> On 13 January 2015 at 15:34, Richard Biener 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) Otherwise this looks ok. 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 SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)