From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9905 invoked by alias); 14 Jan 2015 11:16:46 -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 9803 invoked by uid 89); 14 Jan 2015 11:16:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f174.google.com Received: from mail-lb0-f174.google.com (HELO mail-lb0-f174.google.com) (209.85.217.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 14 Jan 2015 11:16:43 +0000 Received: by mail-lb0-f174.google.com with SMTP id 10so7352751lbg.5 for ; Wed, 14 Jan 2015 03:16:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=SPFNulUKglf3KiOmK6oH1e4mRtJ9D4fg87OZDNIZxPc=; b=buXpYx5ARQk0f71CB9AOyeFfVoUlHqIihFV5a/uKcwZo+1tON6DE2Dk7qPOyyH8G9l ckRWjnfK7X1iTod0a9SavYv6JYmA1LSJYT4OkkEEpdftIDy8dl6De7XUM0jhZ0J2OyKG 8UEwT0Kjhyz7oClsjxPfKWZDGyY04PePqpIN/nyK6/5ulrBHP44wFYjoIrMM8HzIpkff z/VomwoITSgMcqrbPZ36UrIvhy4I/M5fQFw1mt5QzlQOJO3tMbzzBKsO0N87wJiEEIGu jFGL//gfR+12S1uSqZLWG4734TSOmcTi5q1Qa80CY7psQWX502DPTl5GPEdkEbcnr6at vMHw== X-Gm-Message-State: ALoCoQknT7LKgTvH6WHOX1gHFSTIGaatqEeUvWG4bG5Xj/AMFac0GyijEnTKoqa8T2Q+2CgeBkBw MIME-Version: 1.0 X-Received: by 10.112.97.163 with SMTP id eb3mr3226023lbb.47.1421234199373; Wed, 14 Jan 2015 03:16:39 -0800 (PST) Received: by 10.25.211.16 with HTTP; Wed, 14 Jan 2015 03:16:39 -0800 (PST) In-Reply-To: References: Date: Wed, 14 Jan 2015 11:18:00 -0000 Message-ID: Subject: Re: flatten expr.h (version 2) From: Prathamesh Kulkarni To: Richard Biener Cc: Michael Collison , gcc Patches , Andrew MacLeod Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg01018.txt.bz2 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. > > 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.