| Message ID | 556F02E5.2080602@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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);