Re: [patch 0/9] Flattening and initial module rebuilding

[Andrew MacLeod]

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

NOT_TAKEN macros

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*.

be_predictor is used in parameter lists and a few inlines use the TAKEN, Its also needed by cfghooks.h, and betwen those 2 files, its just needed

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.

   /* Only basic-block.h includes this.  */
but no rationale.

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 : 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.

- Finally, try to remove the extraneous cfghooks.h and predict.h files.


Then result using numbers which exclude those caveat files:


The total result affect 227 files.




assuming its all fine, OK for trunk then?

Andrew

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.. 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. . 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. 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.

predict.patch.Z
Description: Unix compressed data