From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43303 invoked by alias); 4 Jun 2015 11:28:33 -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 43293 invoked by uid 89); 4 Jun 2015 11:28:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 04 Jun 2015 11:28:31 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 121FCB8BC1; Thu, 4 Jun 2015 11:28:30 +0000 (UTC) Received: from [10.10.55.9] (vpn-55-9.rdu2.redhat.com [10.10.55.9]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t54BSSMM011944; Thu, 4 Jun 2015 07:28:29 -0400 Message-ID: <5570365C.8030803@redhat.com> Date: Thu, 04 Jun 2015 12:29:00 -0000 From: Andrew MacLeod User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Richard Biener CC: gcc-patches Subject: Re: [patch] consolidate some includes into coretypes.h References: <556CC87F.2090305@redhat.com> <556DA2C2.4010704@redhat.com> <556DBB73.6070408@redhat.com> <556F02E5.2080602@redhat.com> In-Reply-To: <556F02E5.2080602@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00399.txt.bz2 On 06/03/2015 09:36 AM, Andrew MacLeod wrote: > On 06/03/2015 07:47 AM, Richard Biener wrote: >> On Tue, Jun 2, 2015 at 4:19 PM, Andrew MacLeod >> wrote: >>> On 06/02/2015 09:30 AM, Richard Biener wrote: >>>> On Tue, Jun 2, 2015 at 2:34 PM, Andrew MacLeod >>>> wrote: >>>>> On 06/02/2015 04:26 AM, Richard Biener wrote: >>>>>> On Mon, Jun 1, 2015 at 11:02 PM, Andrew MacLeod >>>>>> >>>>>> wrote: >>>>>>> >>>>>>> Bootstraps from scratch on x86_64-unknown-linux-gnu with no new >>>>>>> test >>>>>>> regressions. I also built it on all the config-list.mk targets >>>>>>> with no >>>>>>> additional compilation errors. >>>>>>> >>>>>>> OK for trunk? >>>>>> Generally the idea is sound (amend coretypes.h), but I don't like >>>>>> the >>>>>> GCC_CONFIG_H guard, why does !GENERATOR_FILE not work? >>>>> Target files also use coretypes.h. In particular, libgcc includes >>>>> it and >>>>> does not have GENERATOR_FILE set. Rather than checking for >>>>> GCC_CONFIG_H >>>>> we >>>>> could check >>>>> >>>>> #if !defined (GENERATOR_FILE) && !defined (USED_FOR_TARGET) >>>>> >>>>> I think that should work OK. >>>>>> Furthermore I don't like the special-casing in rtl.h, instead have >>>>>> coretypes.h contain sth like >>>>>> >>>>>> #ifdef GENERATOR_FILE >>>>>> ... rtl.h special-case >>>>>> #else >>>>>> ... GCC_CONFIG_H stuff >>>>>> #endif >>>>>> >>>>>> Thanks, >>>>>> Richard. >>>>> >>>>> This one is harder. I don't like the special case either, but you >>>>> cant >>>>> really figure it out in coretypes.h. The problem comes from some >>>>> generator >>>>> files which compile rtl.c and and a couple of other files, and >>>>> thus have >>>>> GENERATOR_FILE set... These run after the initial set of >>>>> generators so >>>>> insn-modes.h and friends have been created, and these includes are >>>>> now >>>>> required. the presence of rtl.h seems to be the the litmus test >>>>> and if >>>>> it >>>>> occurs in the include chain after coretypes.h, then we'll need these >>>>> files. >>>>> >>>>> I suppose you could just include those files in rtl.h directly >>>>> without >>>>> the >>>>> guard... it is probably the cleanest solution. Otherwise we'd either >>>>> have >>>>> to add a new identifying macro to a dozen generator files, or include >>>>> these >>>>> headers there, or some other such thing. >>>> Well, then include the requirements in the generator files >>>> instead? It >>>> looks >>>> backwards to add to the includes in rtl.h. >>>> >>>> Richard. >>> Except that it is rtl.h that actually has the compilation >>> requirement. I >>> could put those includes in each of the generator files which >>> require it, >>> but the list is non-trivial: >>> Each of these files can be compiled with bconfig.h instead of >>> config.h, and >>> they each include rtl.h which requires these headers: >>> genattr.c >>> genattr-common.c >>> genattrtab.c >>> genautomata.c >>> gencodes.c >>> genconditions.c >>> genconfig.c >>> genemit.c >>> genextract.c >>> genflags.c >>> genmddump.c >>> genopinit.c >>> genoutput.c >>> genpeep.c >>> genpreds.c >>> genrecog.c >>> gensupport.c >>> print-rtl.c >>> read-rtl.c >>> rtl.c >>> >>> >>> so there are 20 files which require these headers, and there are 11 >>> others >>> which do not require rtl.h nor the headers (and will fail compile if >>> they >>> are included) >>> gencheck.c >>> genconstants.c >>> genenums.c >>> genmatch.c >>> genmddeps.c >>> genmodes.c >>> ggc-none.c >>> hash-table.c >>> inchash.c >>> read-md.c >>> vec.c >>> >>> >>> I suppose one could add something like: >>> #define EARLY_GENERATOR >>> in each of the 11 and check for that macro in coretypes.h instead of >>> GENERATOR file. ThIs appears to work fine: >> I don't like that either ... >> >> which of the includes are the problematic ones? I guess only machmode.h >> (and thus wide-int.h?) Can't we just guard parts of rtl.h / wide-int.h >> properly? >> >> As a transitional measure the variant with the rtl.h includes dependent >> on GENERATOR_FILE is ok. >> > Hmm. Its not nearly as bad as I expected. rtl.h will compile if I > > 1) provide a dummy CONST_DOUBLE_FORMAT definition like gengtype.c does > 2) don't compile the wi:: specializations, and > 3) don't put struct real_value or struct fixed_value in the field > union of struct rtx_def > > This appears to bootstrap from scratch, at least on x86... I don't > know exactly what all those generator files do with RTL, but they > appear to work without double and real support, so I guess it isn't > being used. That really is just a guess tho :-P I can imagine all > kinds of nasty things :-) These files did include the support previously. > > What do you think? I'm running the testsuite right now. If it > passes everything, do you want to go ahead with this version? > > Andrew Never mind, this change blew up all over a bunch of targets, and it was not so simple to fix... Reverting back to the original guarded rtl.h, and i'll check it in after all the runs complete cleanly Andrew