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

[Andrew MacLeod <amacleod@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4546 bytes --]

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

[-- Attachment #2: num7.diff --]
[-- Type: text/x-patch, Size: 1424 bytes --]

Index: rtl.h
===================================================================
--- rtl.h	(revision 223875)
+++ rtl.h	(working copy)
@@ -21,14 +21,10 @@
 #define GCC_RTL_H
 
 #include "statistics.h"
-#include "machmode.h"
 #include "input.h"
-#include "real.h"
 #include "vec.h"
-#include "fixed-value.h"
 #include "alias.h"
 #include "hashtab.h"
-#include "wide-int.h"
 #include "flags.h"
 #include "is-a.h"
 
@@ -55,6 +51,8 @@
 /* Similar, but since generator files get more entries... */
 #ifdef GENERATOR_FILE
 # define NON_GENERATOR_NUM_RTX_CODE ((int) MATCH_OPERAND)
+/* Needed to compile, but won't be used. */
+#define CONST_DOUBLE_FORMAT "ww"
 #endif
 
 /* Register Transfer Language EXPRESSIONS CODE CLASSES */
@@ -409,8 +407,10 @@
     HOST_WIDE_INT hwint[1];
     struct reg_info reg;
     struct block_symbol block_sym;
+#ifndef GENERATOR_FILE
     struct real_value rv;
     struct fixed_value fv;
+#endif
     struct hwivec_def hwiv;
   } GTY ((special ("rtx_def"), desc ("GET_CODE (&%0)"))) u;
 };
@@ -2056,6 +2056,7 @@
    integer constants, this should go away and then just pass an rtx in.  */
 typedef std::pair <rtx, machine_mode> rtx_mode_t;
 
+#ifndef GENERATOR_FILE
 namespace wi
 {
   template <>
@@ -2138,6 +2139,7 @@
 {
   return max_value (GET_MODE_PRECISION (mode), sgn);
 }
+#endif /* GENERATOR_FILE */
 
 extern void init_rtlanal (void);
 extern int rtx_cost (rtx, enum rtx_code, int, bool);