PATCH RFA: Build system: Use AC_SYS_LARGEFILE

PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor]

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

The include/simple-object.h file I added earlier today uses off_t in
function declarations.  That breaks some 32-bit hosts because the
libiberty configure script uses AC_SYS_LARGEFILE and the gcc configure
script does not.  The size of off_t changes depending on the value of
the preprocessor macro _FILE_OFFSET_BITS, and that is changed by the
configure script when AC_SYS_LARGEFILE is used.

There are various possibilities here, but the simplest would seem to be
to use AC_SYS_LARGEFILE in the configure script for gcc.  We should use
it in libcpp also, as libcpp does file I/O and should be able to handle
large files.

This patch does that.  It has passed stage 2 in my bootstrap on
x86_64-unknown-linux-gnu, so I don't think it is horribly broken.  I of
course do not expect any changes on that target, but I don't have a
32-bit hosted build readily available.  As far as I can see this patch
is completely safe.  We've been using AC_SYS_LARGEFILE in the libiberty
configure script for quite a while.

Is this patch OK to commit if it completes bootstrap and testing?

Thanks.

Ian


gcc/:

2010-11-02  Ian Lance Taylor  <iant@google.com>

	* configure.ac: Use AC_SYS_LARGEFILE.
	* configure: Rebuild.
	* config.in: Rebuild.

libcpp/:

2010-11-02  Ian Lance Taylor  <iant@google.com>

	* configure.ac: Use AC_SYS_LARGEFILE.
	* configure: Rebuild.
	* config.in: Rebuild.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: largefile --]
[-- Type: text/x-diff, Size: 736 bytes --]

Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 166189)
+++ gcc/configure.ac	(working copy)
@@ -304,6 +304,8 @@ AC_USE_SYSTEM_EXTENSIONS
 AC_PROG_CPP
 AC_C_INLINE
 
+AC_SYS_LARGEFILE
+
 # sizeof(char) is 1 by definition.
 AC_CHECK_SIZEOF(void *)
 AC_CHECK_SIZEOF(short)
Index: libcpp/configure.ac
===================================================================
--- libcpp/configure.ac	(revision 166218)
+++ libcpp/configure.ac	(working copy)
@@ -14,6 +14,8 @@ AC_PROG_CC
 AC_PROG_CXX
 AC_PROG_RANLIB
 
+AC_SYS_LARGEFILE
+
 # See if we are building gcc with C++.
 # Do this early so setting lang to C++ affects following tests
 AC_ARG_ENABLE(build-with-cxx,

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Paolo Bonzini]

On 11/02/2010 11:13 PM, Ian Lance Taylor wrote:
> Is this patch OK to commit if it completes bootstrap and testing?

Yes, thanks.

Paolo

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Jakub Jelinek]

On Wed, Nov 03, 2010 at 01:38:14AM +0100, Paolo Bonzini wrote:
> On 11/02/2010 11:13 PM, Ian Lance Taylor wrote:
> >Is this patch OK to commit if it completes bootstrap and testing?
> 
> Yes, thanks.

It bootstrapped/regtested just fine here on both x86_64-linux (where it
should make no difference) and i686-linux (where it fixed all those
thousands of recent -flto/-fwhopr regressions).

	Jakub

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Richard Guenther]

On Wed, Nov 3, 2010 at 1:38 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/02/2010 11:13 PM, Ian Lance Taylor wrote:
>>
>> Is this patch OK to commit if it completes bootstrap and testing?
>
> Yes, thanks.

I did this once and had to revert it again because it broke bootstrap
on AIX in some obscure way.  But yes, we carry this patch in our
local GCC versions since a few major releases.

Richard.

> Paolo
>

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Paolo Bonzini]

On 11/03/2010 11:45 AM, Richard Guenther wrote:
>>> >>
>>> >>  Is this patch OK to commit if it completes bootstrap and testing?
>> >
>> >  Yes, thanks.
> I did this once and had to revert it again because it broke bootstrap
> on AIX in some obscure way.  But yes, we carry this patch in our
> local GCC versions since a few major releases.

I'll still approve the patch and wait for AIX people to continue.  BTW 
if we're to keep AIX as a supported platform, maybe we should bump the 
version to AIX 6 (released in 2007).

Paolo

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor]

Richard Guenther <richard.guenther@gmail.com> writes:

> On Wed, Nov 3, 2010 at 1:38 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 11/02/2010 11:13 PM, Ian Lance Taylor wrote:
>>>
>>> Is this patch OK to commit if it completes bootstrap and testing?
>>
>> Yes, thanks.
>
> I did this once and had to revert it again because it broke bootstrap
> on AIX in some obscure way.  But yes, we carry this patch in our
> local GCC versions since a few major releases.

Perhaps the problem then was that libiberty did not use
AC_SYS_LARGEFILE?  That was added to libiberty 2008-10-07.

In any case I committed the patch.

Ian

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Richard Guenther]

On Wed, Nov 3, 2010 at 3:30 PM, Ian Lance Taylor <iant@google.com> wrote:
> Richard Guenther <richard.guenther@gmail.com> writes:
>
>> On Wed, Nov 3, 2010 at 1:38 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>> On 11/02/2010 11:13 PM, Ian Lance Taylor wrote:
>>>>
>>>> Is this patch OK to commit if it completes bootstrap and testing?
>>>
>>> Yes, thanks.
>>
>> I did this once and had to revert it again because it broke bootstrap
>> on AIX in some obscure way.  But yes, we carry this patch in our
>> local GCC versions since a few major releases.
>
> Perhaps the problem then was that libiberty did not use
> AC_SYS_LARGEFILE?  That was added to libiberty 2008-10-07.
>
> In any case I committed the patch.

FYI, http://gcc.gnu.org/ml/gcc-patches/2009-02/msg00706.html

Richard.

> Ian
>

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor]

Richard Guenther <richard.guenther@gmail.com> writes:

> On Wed, Nov 3, 2010 at 3:30 PM, Ian Lance Taylor <iant@google.com> wrote:
>> Richard Guenther <richard.guenther@gmail.com> writes:
>>
>>> I did this once and had to revert it again because it broke bootstrap
>>> on AIX in some obscure way.  But yes, we carry this patch in our
>>> local GCC versions since a few major releases.
>>
>> Perhaps the problem then was that libiberty did not use
>> AC_SYS_LARGEFILE?  That was added to libiberty 2008-10-07.
>>
>> In any case I committed the patch.
>
> FYI, http://gcc.gnu.org/ml/gcc-patches/2009-02/msg00706.html

Thanks.  I checked a bunch of .c files in gcc and libcpp, and they all
reliably #included "config.h" first.  I didn't check every file, but I
didn't find any exceptions to the rule.  So I think this is fixable.

David, can you let us know what fails when bootstrapping current
mainline on AIX?

Ian

PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[David Edelsohn]

> Richard Guenther <richard.guenther@gmail.com> writes:
>
> > On Wed, Nov 3, 2010 at 3:30 PM, Ian Lance Taylor <iant@google.com> wrote:
> >> Richard Guenther <richard.guenther@gmail.com> writes:
> >>
> >>> I did this once and had to revert it again because it broke bootstrap
> >>> on AIX in some obscure way. ÂBut yes, we carry this patch in our
> >>> local GCC versions since a few major releases.
> >>
> >> Perhaps the problem then was that libiberty did not use
> >> AC_SYS_LARGEFILE? ÂThat was added to libiberty 2008-10-07.
> >>
> >> In any case I committed the patch.
> >
> > FYI, http://gcc.gnu.org/ml/gcc-patches/2009-02/msg00706.html
>
> Thanks.  I checked a bunch of .c files in gcc and libcpp, and they all
> reliably #included "config.h" first.  I didn't check every file, but I
> didn't find any exceptions to the rule.  So I think this is fixable.
>
> David, can you let us know what fails when bootstrapping current
> mainline on AIX?

Sorry, I never received this email.  I do not know if gnu.org dropped
it or Gmail decided that email from google.com was SPAM again.  I also
did not see this thread because I was traveling.

Guess what?  AIX bootstrap is broken.  To enable LARGE FILES support
on AIX, the definition

#define _LARGE_FILES 1

must come first before any system header files are included, otherwise
the 64 bit IO symbols starts conflicting, e.g.,

In file included from /farm/dje/src/src/gcc/gengtype-parse.c:21:
/farm/dje/src/src/gcc/system.h:55:1: warning: "freopen" redefined
In file included from /farm/dje/src/src/gcc/system.h:42,
                 from /farm/dje/src/src/gcc/gengtype-parse.c:21:
/gsa/yktgsa-p4/06/gnu/aix/5.3/power/bin/../lib/gcc/powerpc-ibm-aix5.3.0.0/4.1.3/
include/stdio.h:111:1: warning: this is the location of the previous definition
In file included from /farm/dje/src/src/gcc/system.h:236,
                 from /farm/dje/src/src/gcc/gengtype-lex.l:26:
/usr/include/unistd.h:171: error: conflicting types for 'lseek64'
/usr/include/unistd.h:169: error: previous declaration of 'lseek64' was here
In file included from /usr/include/unistd.h:737,
                 from /farm/dje/src/src/gcc/system.h:236,
                 from /farm/dje/src/src/gcc/gengtype-lex.l:26:
/usr/include/sys/lockf.h:64: error: conflicting types for 'lockf64'
/usr/include/sys/lockf.h:62: error: previous declaration of 'lockf64' was here
In file included from /farm/dje/src/src/gcc/system.h:236,
                 from /farm/dje/src/src/gcc/gengtype-lex.l:26:
/usr/include/unistd.h:800: error: conflicting types for 'ftruncate64'
/usr/include/unistd.h:798: error: previous declaration of 'ftruncate64' was here
/usr/include/unistd.h:836: error: conflicting types for 'truncate64'
/usr/include/unistd.h:834: error: previous declaration of 'truncate64' was here
/usr/include/unistd.h:853: error: conflicting types for 'pread64'
/usr/include/unistd.h:850: error: previous declaration of 'pread64' was here
/usr/include/unistd.h:854: error: conflicting types for 'pwrite64'
/usr/include/unistd.h:851: error: previous declaration of 'pwrite64' was here
/usr/include/unistd.h:921: error: conflicting types for 'fclear64'
/usr/include/unistd.h:918: error: previous declaration of 'fclear64' was here
/usr/include/unistd.h:922: error: conflicting types for 'fsync_range64'
/usr/include/unistd.h:919: error: previous declaration of
'fsync_range64' was here

One can see the problem in gengtype-lex.c:

#line 2 "gengtype-lex.c"
#include <stdio.h>
...
#define YY_NO_INPUT 1
#line 25 "/farm/dje/src/src/gcc/gengtype-lex.l"
#include "bconfig.h"
#include "system.h"

bconfig.h includes auto-host.h, which defines _LARGE_FILES.

libcpp/directives.c also fails with similar errors, although it is not
obvious how headers are included in the wrong order

I have no objection to enabling LARGE FILES support in GCC, but the
headers must be included in the correct order to allow bootstrap on
AIX.

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?

Thanks, David

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Paolo Bonzini]

> bconfig.h includes auto-host.h, which defines _LARGE_FILES.
> 
> libcpp/directives.c also fails with similar errors, although it is not
> obvious how headers are included in the wrong order
> 
> I have no objection to enabling LARGE FILES support in GCC, but the
> headers must be included in the correct order to allow bootstrap on
> AIX.

Is something like this enough?

Index: libcpp/configure.ac
===================================================================
--- libcpp/configure.ac	(revision 166028)
+++ libcpp/configure.ac	(working copy)
@@ -13,6 +13,7 @@ AC_PROG_INSTALL
 AC_PROG_CC
 AC_PROG_CXX
 AC_PROG_RANLIB
+AC_SYS_LARGEFILE
 
 # See if we are building gcc with C++.
 # Do this early so setting lang to C++ affects following tests
Index: gcc/gengtype-lex.l
===================================================================
--- gcc/gengtype-lex.l	(revision 166028)
+++ gcc/gengtype-lex.l	(working copy)
@@ -23,6 +23,10 @@ along with GCC; see the file COPYING3.  
 
 %{
 #include "bconfig.h"
+
+/* stdio.h has been included already by the flex skeleton, so
+   defining _LARGE_FILES here would break bootstrap on AIX.  */
+#undef _LARGE_FILES
 #include "system.h"
 
 #define malloc xmalloc

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[David Edelsohn]

Paolo,

There is no problem in libcpp.  There is a warning about freopen
redefined in system.h, but I mistook that for another failure.

The main problem seems to be gengtype-lex.c, which is generated by
Flex.  Flex inserted

#include <stdio.h>

although gengtype-lex.l already includes

#include "bconfig.h"
#include "system.h"

in the correct order and system.h includes stdio.h.

I will try the gengtype-lex.l patch.

Thanks, David

On Tue, Nov 9, 2010 at 9:15 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>> bconfig.h includes auto-host.h, which defines _LARGE_FILES.
>>
>> libcpp/directives.c also fails with similar errors, although it is not
>> obvious how headers are included in the wrong order
>>
>> I have no objection to enabling LARGE FILES support in GCC, but the
>> headers must be included in the correct order to allow bootstrap on
>> AIX.
>
> Is something like this enough?
>
> Index: libcpp/configure.ac
> ===================================================================
> --- libcpp/configure.ac (revision 166028)
> +++ libcpp/configure.ac (working copy)
> @@ -13,6 +13,7 @@ AC_PROG_INSTALL
>  AC_PROG_CC
>  AC_PROG_CXX
>  AC_PROG_RANLIB
> +AC_SYS_LARGEFILE
>
>  # See if we are building gcc with C++.
>  # Do this early so setting lang to C++ affects following tests
> Index: gcc/gengtype-lex.l
> ===================================================================
> --- gcc/gengtype-lex.l  (revision 166028)
> +++ gcc/gengtype-lex.l  (working copy)
> @@ -23,6 +23,10 @@ along with GCC; see the file COPYING3.
>
>  %{
>  #include "bconfig.h"
> +
> +/* stdio.h has been included already by the flex skeleton, so
> +   defining _LARGE_FILES here would break bootstrap on AIX.  */
> +#undef _LARGE_FILES
>  #include "system.h"
>
>  #define malloc xmalloc
>

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor]

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

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



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: LARGE --]
[-- Type: text/x-diff, Size: 2450 bytes --]

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

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[David Edelsohn]

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
>
>
>

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor]

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

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Paolo Bonzini]

On 11/09/2010 05:46 PM, Ian Lance Taylor wrote:
> 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.

I agree.  I had considered this too but then went for the inferior patch 
for a reason:

Index: Makefile.in
===================================================================
--- Makefile.in	(revision 166028)
+++ Makefile.in	(working copy)
@@ -3949,7 +3949,11 @@ $(genprog:%=build/gen%$(build_exeext)):

  # Generated source files for gengtype.
  gengtype-lex.c : gengtype-lex.l
-	-$(FLEX) $(FLEXFLAGS) -o$@ $<
+	-$(FLEX) $(FLEXFLAGS) -o$@ $< && { \
+	  echo '#include "bconfig.h"' > $@.tmp; \
+	  cat $@ >> $@.tmp; \
+	  mv $@.tmp $@; \
+	}

  #

  # Remake internationalization support.

The advantage of this, if anything, is that something like that might be 
wrapped in automake's ylwrap script.

However, Ian---depending on the order in which David tests the various 
approaches, your patch is okay if it works.

Paolo

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[David Edelsohn]

Paolo's earlier patch #undef'ing _LARGE_FILES works and allows me to
bootstrap; still running testsuite.

I agree about the potential problems, although gengtype is fairly independent.

I am worried about defining _LARGE_FILES in CFLAGS causing other
problems because that will cause many other files that do not use
auto-hosts.h to be compiled with _LARGE_FILES support with unknown
affects.

Inserting bconfig.h first seems the most localized approach.

Thanks, David

On Tue, Nov 9, 2010 at 11:55 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/09/2010 05:46 PM, Ian Lance Taylor wrote:
>>
>> 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.
>
> I agree.  I had considered this too but then went for the inferior patch for
> a reason:
>
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 166028)
> +++ Makefile.in (working copy)
> @@ -3949,7 +3949,11 @@ $(genprog:%=build/gen%$(build_exeext)):
>
>  # Generated source files for gengtype.
>  gengtype-lex.c : gengtype-lex.l
> -       -$(FLEX) $(FLEXFLAGS) -o$@ $<
> +       -$(FLEX) $(FLEXFLAGS) -o$@ $< && { \
> +         echo '#include "bconfig.h"' > $@.tmp; \
> +         cat $@ >> $@.tmp; \
> +         mv $@.tmp $@; \
> +       }
>
>  #
>
>  # Remake internationalization support.
>
> The advantage of this, if anything, is that something like that might be
> wrapped in automake's ylwrap script.
>
> However, Ian---depending on the order in which David tests the various
> approaches, your patch is okay if it works.
>
> Paolo
>

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Ian Lance Taylor]

David Edelsohn <dje.gcc@gmail.com> writes:

> Inserting bconfig.h first seems the most localized approach.

I agree and I'd like to go with that approach if it works.

Thanks.

Ian

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[David Edelsohn]

Paolo,

Your Makefile patch seems to work as well, I am past building
gengtype-lex step in bootstrap.

Thanks, David

On Tue, Nov 9, 2010 at 11:55 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 11/09/2010 05:46 PM, Ian Lance Taylor wrote:
>>
>> 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.
>
> I agree.  I had considered this too but then went for the inferior patch for
> a reason:
>
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 166028)
> +++ Makefile.in (working copy)
> @@ -3949,7 +3949,11 @@ $(genprog:%=build/gen%$(build_exeext)):
>
>  # Generated source files for gengtype.
>  gengtype-lex.c : gengtype-lex.l
> -       -$(FLEX) $(FLEXFLAGS) -o$@ $<
> +       -$(FLEX) $(FLEXFLAGS) -o$@ $< && { \
> +         echo '#include "bconfig.h"' > $@.tmp; \
> +         cat $@ >> $@.tmp; \
> +         mv $@.tmp $@; \
> +       }
>
>  #
>
>  # Remake internationalization support.
>
> The advantage of this, if anything, is that something like that might be
> wrapped in automake's ylwrap script.
>
> However, Ian---depending on the order in which David tests the various
> approaches, your patch is okay if it works.
>
> Paolo
>

Re: PATCH RFA: Build system: Use AC_SYS_LARGEFILE

[Paolo Bonzini]

On 11/11/2010 05:05 PM, David Edelsohn wrote:
> Paolo,
>
> Your Makefile patch seems to work as well, I am past building
> gengtype-lex step in bootstrap.

Can you commit it if it passes?

Paolo