Re: [patch] consolidate some includes into coretypes.h

[Andrew MacLeod <amacleod@redhat.com>]

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 <amacleod@redhat.com> 
>> wrote:
>>> On 06/02/2015 09:30 AM, Richard Biener wrote:
>>>> On Tue, Jun 2, 2015 at 2:34 PM, Andrew MacLeod <amacleod@redhat.com>
>>>> wrote:
>>>>> On 06/02/2015 04:26 AM, Richard Biener wrote:
>>>>>> On Mon, Jun 1, 2015 at 11:02 PM, Andrew MacLeod 
>>>>>> <amacleod@redhat.com>
>>>>>> 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