diff mbox

PATCH RFA: Build system: Use AC_SYS_LARGEFILE

Message ID mcroca7s7bn.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Nov. 2, 2010, 10:13 p.m. UTC
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.

Comments

Paolo Bonzini Nov. 3, 2010, 12:38 a.m. UTC | #1
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
Jakub Jelinek Nov. 3, 2010, 8:19 a.m. UTC | #2
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
Richard Biener Nov. 3, 2010, 10:45 a.m. UTC | #3
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
>
Paolo Bonzini Nov. 3, 2010, 10:49 a.m. UTC | #4
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
Ian Lance Taylor Nov. 3, 2010, 2:30 p.m. UTC | #5
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
Richard Biener Nov. 3, 2010, 2:41 p.m. UTC | #6
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
>
Ian Lance Taylor Nov. 3, 2010, 3:30 p.m. UTC | #7
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
diff mbox

Patch

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,