From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116504 invoked by alias); 9 Jul 2015 17:48:40 -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 116476 invoked by uid 89); 9 Jul 2015 17:48:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS 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, 09 Jul 2015 17:48:37 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id C2061371B44 for ; Thu, 9 Jul 2015 17:48:36 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-49.phx2.redhat.com [10.3.113.49]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t69HmaEZ029339; Thu, 9 Jul 2015 13:48:36 -0400 Message-ID: <559EB40E.5020808@redhat.com> Date: Thu, 09 Jul 2015 17:48:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Andrew MacLeod , gcc-patches Subject: Re: [patch 0/9] Flattening and initial module rebuilding References: <559BD6B3.2080207@redhat.com> <559C50F0.9090807@redhat.com> <559C668C.4050600@redhat.com> <559DA7A5.2050109@redhat.com> <559DDFAA.50400@redhat.com> <559EAA03.9030506@redhat.com> In-Reply-To: <559EAA03.9030506@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00800.txt.bz2 On 07/09/2015 11:06 AM, Andrew MacLeod wrote: > On 07/08/2015 10:42 PM, Andrew MacLeod wrote: >> On 07/08/2015 06:43 PM, Jeff Law wrote: >>>> predict.h is actually required by gimple.h for a few reasons, enum >>>> be_predictor is used in parameter lists and a few inlines use the >>>> TAKEN, >>>> NOT_TAKEN macros >>>> Its also needed by cfghooks.h, and betwen those 2 files, its just >>>> needed >>>> by a very good chunk of the backend. .. 219 of the 263 files which >>>> include backend.h need it. >>>> We could move the 2 enums and TAKEN/NOT_TAKEN to coretypes or something >>>> like that and it would probably cut the requirements for it by a *lot*. >>> Might be something for a follow-up (moving the enums). >>> >>> >> blah, not so trivial. One of the primary things predict.h does is >> create enum br_predictor by including predict,def.. so moving that >> enum doesnt really make sense. >> >> Fixing gimple,h isn't too bad, I could split the prediction stuff out >> into gimple-predict.h and include it in the 4 places its needed (as >> you might be able to tell, Ive tried this :-) >> >> However, it doesn't do much good. cfghooks.h is included by >> basic-block.h.. which is needed virtually everywhere :-P >> >> There are just 2 entries in the hooks table which require 'enum >> br_predictor', but I dont think it makes sense to move things out of >> the cfghook structure.. I suppose once could create a prediction_hooks >> structure.. and put it in predict_hooks.h... but I think thats >> probably going to far to avoid including a file which makes some >> logical sense.. >> >> The other option is to pull cfghooks.h out of basic-block.h and >> include it seperately on its own.. What is the reason its in there >> now? It appears to not have a cyclic dependency which is the usual >> reason to have an include in the middle of a file. Or perhaps the >> reason no longer exists? There is a comment at the top of cfghooks.h : >> /* Only basic-block.h includes this. */ >> but no rationale. >> >> I moved it to the very bottom of the file and everything still seems >> to compile fine I can try flattening it out of basic-block.h and >> only including it in places that need it... that should eliminate the >> need to put predict.h in a lot of places I would think. >> > > ok, so the results are in. a bit painful to unravel :-) these are the > steps > - Splitting the prediction bits of gimple.h out to gimple-predict.h and > putting that file where it matters (5 files as it turns out) > - Next split cfghooks.h out from basic-block.h and put it in the files > that it is needed in. > - then I moved predict.h out of backend.h I added it as an include to > cfghooks.h and gimple-predict.h and adjusted source files accordingly.. > - Finally, try to remove the extraneous cfghooks.h and predict.h files. > > caveat, I did no reductions on config/* files nor on languages beyond > stage1 builds... havent gotten to enhancing to tool to deal with that > yet.. its coming as a part of the general include reduction.. so I'll > havdle these bits then. . > Then result using numbers which exclude those caveat files: > predict.h ends up in 68 files out of the original 179 it was in by > itself. . ( ie excluding files with cfghooks.h or gimple-predict.h) > cfghooks.h ends up in 89 of the original 268 that basic-block was > present in. > > The total result affect 227 files. > > Now, predict.h is *still* more pervasive than it needs to be, but thats > a different patch :-). There are a set of routines in there like > optimize_{fucntion,loop,edge,bb}_for{speed,size}_p() that are the > reason half the files need it. those should probably be moved > somewhere else since they aren't really prediction related :-P Maybe a > better spot will show up when the rest of the include reductions are done. > > I've attached a patch. It bootstraps on x86_64-unknown-linux-gnu, and > I'm running regressions. To be safe, I'll run config-list.mk overnight > to be sure. > assuming its all fine, OK for trunk then? OK assuming everything is fine with your overnight run. BTW, you're showing your age -- a .Z file... Thankfully gzip will handle that just fine. (And yes, I realize the irony here that gzip is probably considered ancient as well given bzip2 and xz). jeff