| Message ID | mcriq061q1h.fsf@google.com |
|---|---|
| State | New |
| Headers | show |
Ian, With Paolo's patch, my bootstrap has progressed into stage 2. Hopefully the Flex-generated file is the only case where header files are included in the wrong order. Paolo's patch definitely is a step in the right direction. Thanks, David On Tue, Nov 9, 2010 at 10:21 AM, Ian Lance Taylor <iant@google.com> wrote: > David Edelsohn <dje.gcc@gmail.com> writes: > >> Also, given this problem, I want to try a meta-experiment: At the GCC >> Summit, some Google developers proposed that any patch causing >> breakage immediately be reverted, following the practice at Google. >> Jakub mentioned on IRC that reverting the patch will break bootstrap >> on i386-linux and other targets. So Googlers, how do you want to >> proceed and demonstrate your own proposed policy in action? > > Although I was regularly interrupted during that discussion at the > summit, I tried a few times to say that in my opinion the policy would > only apply during the first few days after the patch was committed. > It's been a week for this patch now. > > Also there is a conflict in that this patch fixed bootstrap for > i686-unknown-linux-gnu, a primary platform, whereas you are telling us > that it breaks bootstrap for powerpc-ibm-aix5.3.0.0 a secondary > platform. The right step would have been to revert the earlier > simple_object patch, rather than this one. > > So on both those grounds I'm not sure an immediate reversion of this > patch is appropriate now, but I will do it if you ask again. > > > Another approach would be the appended patch. Does it fix the problem? > Build maintainers, any opinion? > > Ian > > >
David Edelsohn <dje.gcc@gmail.com> writes: > With Paolo's patch, my bootstrap has progressed into stage 2. > Hopefully the Flex-generated file is the only case where header files > are included in the wrong order. Paolo's patch definitely is a step > in the right direction. Paolo's patch makes me a little bit uncomfortable because it means that gengtype-lex.l is compiled with a different ABI than the rest of the compiler. Of course it's a very simple file and it doesn't use any aspect of the ABI. But it does call fopen and fclose. If a call to fseeko ever sneaks in there somehow, it will break on AIX in a rather mysterious manner. Ian
Index: configure.ac =================================================================== --- configure.ac (revision 166441) +++ configure.ac (working copy) @@ -306,6 +306,14 @@ AC_C_INLINE AC_SYS_LARGEFILE +# flex generated files do not include config.h first, so we need to +# add _LARGE_FILES to CFLAGS on AIX. +case $ac_cv_sys_large_files in + no | unknown) LFS_CFLAGS= ;; + *) LFS_CFLAGS="-D_LARGE_FILES=$ac_cv_sys_large_files" ;; +esac +AC_SUBST(LFS_CFLAGS) + # sizeof(char) is 1 by definition. AC_CHECK_SIZEOF(void *) AC_CHECK_SIZEOF(short) @@ -1768,7 +1776,7 @@ STMP_FIXINC=stmp-fixinc AC_SUBST(STMP_F # And these apply if build != host, or we are generating coverage data if test x$build != x$host || test "x$coverage_flags" != x then - BUILD_CFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS_FOR_BUILD)' + BUILD_CFLAGS='$(LFS_CFLAGS) $(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS_FOR_BUILD)' BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)' fi Index: Makefile.in =================================================================== --- Makefile.in (revision 166441) +++ Makefile.in (working copy) @@ -374,7 +378,7 @@ GCC_FOR_TARGET = $(STAGE_CC_WRAPPER) ./x # This is used instead of ALL_CFLAGS when compiling with GCC_FOR_TARGET. # It specifies -B./. # It also specifies -isystem ./include to find, e.g., stddef.h. -GCC_CFLAGS=$(CFLAGS_FOR_TARGET) $(INTERNAL_CFLAGS) $(T_CFLAGS) $(LOOSE_WARN) $(C_LOOSE_WARN) -Wold-style-definition $($@-warn) -isystem ./include $(TCFLAGS) +GCC_CFLAGS=$(CFLAGS_FOR_TARGET) $(LFS_CFLAGS) $(INTERNAL_CFLAGS) $(T_CFLAGS) $(LOOSE_WARN) $(C_LOOSE_WARN) -Wold-style-definition $($@-warn) -isystem ./include $(TCFLAGS) # --------------------------------------------------- # Programs which produce files for the target machine @@ -985,10 +989,10 @@ INTERNAL_CFLAGS = -DIN_GCC @CROSS@ # This is the variable actually used when we compile. If you change this, # you probably want to update BUILD_CFLAGS in configure.ac ALL_CFLAGS = $(T_CFLAGS) $(CFLAGS-$@) \ - $(CFLAGS) $(INTERNAL_CFLAGS) $(COVERAGE_FLAGS) $(WARN_CFLAGS) @DEFS@ + $(CFLAGS) $(LFS_CFLAGS) $(INTERNAL_CFLAGS) $(COVERAGE_FLAGS) $(WARN_CFLAGS) @DEFS@ # The C++ version. -ALL_CXXFLAGS = $(T_CFLAGS) $(CXXFLAGS) $(INTERNAL_CFLAGS) \ +ALL_CXXFLAGS = $(T_CFLAGS) $(CXXFLAGS) $(LFS_CFLAGS) $(INTERNAL_CFLAGS) \ $(COVERAGE_FLAGS) $(WARN_CXXFLAGS) @DEFS@ # Likewise. Put INCLUDES at the beginning: this way, if some autoconf macro