Re: [patch] header file re-ordering.

[Jeff Law <law at redhat dot com>]

On 10/08/2015 07:37 AM, Andrew MacLeod wrote:
> On 10/07/2015 06:02 PM, Jeff Law wrote:
> > On 10/01/2015 08:33 PM, Andrew MacLeod wrote:
> > > these are all in the main gcc directory. 297 files total.
> > >
> > > Everything bootstraps on x86_64-pc-linux-gnu and
> > > powerpc64le-unknown-linux-gnu.  All targets in config-list.mk still
> > > build. Regressions tests also came up clean.
> > >
> > > OK for trunk?
> > So as I look at this and make various spot checks, what really stands
> > out is how often something like alias.h gets included, often in places
> > that have absolutely no business/need to be looking at that file.
> > Cut-n-paste at its worst.  It happens to many others, but alias.h
> > seems to have gotten its grubby self into just about everywhere for
> > reasons unkonwn.
> >
> > I find myself also wondering if a two step approach would make this
> > easier.  Step #1 being ordering the headers, step #2 being removal of
> > the duplicates.  As you note, the downside is two checkins that would
> > affect most files in the tree.  I guess I'll keep slogging through the
> > patch as is...
> >
> > jeff
> Heres the patch for reordered headers.  Building as we speak.  Hard to
> fully verify since Ada doesn't seem to bootstrap on trunk at the moment:
>
> +===========================GNAT BUG
> DETECTED==============================+
> | 6.0.0 20151008 (experimental) (x86_64-pc-linux-gnu) GCC
> error:           |
> | in gen_lowpart_common, at
> emit-rtl.c:1399                                |
> | Error detected around
> s-regpat.adb:1029:22                               |
>
> <...>
> raised TYPES.UNRECOVERABLE_ERROR : comperr.adb:423
> ../gcc-interface/Makefile:311: recipe for target 's-regpat.o' failed
>
>
> However, the tool has been run, and I've made the minor adjustments
> required to the source files to make it work.  (ie, a few multi-line
> comments and the fact that mul-tables.c is generated on the tile* targets.
>
> So this is what it should look like.  I used -cp.    Other languages are
> bootstrapping, and I have yet to build all the targets... that'll just
> take a day.   Be nice if ada worked tho.
>
> I can run the reduction tool over the weekend (its a long weekend here
> :-) on this if you want...  the other patch is a couple of weeks out of
> date anyway now.
I find myself looking at the objc stuff and wondering if it was built. 
For example objc-act.c calls functions prototyped in fold-const.h, but 
that header is no longer included after your patch.

Similarly in objcp we remove tree.h from objcp-decl.c, but it uses TREE 
macros and I don't immediately see where those macros would be coming 
from if tree.h is no longer included.

In general, I'm worried about the objc/objcp stuff.  That in turn makes 
me wonder about the other stuff in a more general sense.  Regardless, I 
think I can take a pretty good stab at the config/ changes.


A pattern that seems to play out a lot in the target files is they liked 
to include insn-config.h, insn-codes.h, & timevar.h.  I can see how 
those typically won't be needed.  The first two are amazingly common.  A 
comment in the nds32 port indicates that insn-config.h may have been 
needed by recog.h in the past.  nds32 actually included insn-config 
twice :-)


Interestingly enough m32r, mcore & pdp11 still need insn-config....

The strangest thing I saw was rs6000 dropping an include of emit-rtl.h. 
 But presumably various powerpc targets were built, so I guess it's 
really not needed.

I'm slightly concerned about the darwin, windows and solaris bits.  The 
former primarily because Darwin has been a general source of pain, and 
in the others because I'm not sure the cross testing will exercise that 
code terribly much.

I'll go ahead and approve all the config/ bits.  Please be on the 
lookout for any fallout.

I'll try and get into more of the other patches tomorrow.

jeff