| Message ID | 20120816115551.GA13453@google.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 16 Aug 2012, Diego Novillo wrote: > Richi, this implements your idea for fixing PR 54281. I don't > have an old enough compiler. Could you please test it in your > system? > > I debated whether to remove the GENERATOR_FILE predicate from the > inclusion (some files include gmp.h unconditionally). I think it > could be removed, but only a minority of files build with > GENERATOR_FILE set, so it didn't seem harmful. > > OK if tests pass? It fixes it. Thus, ok from my side (if your testing succeeds). Thanks, Richard. > > Diego. > > > 2012-08-16 Diego Novillo <dnovillo@google.com> > Richard Guenther <rguenther@suse.de> > > PR bootstrap/54281 > * double-int.h: Move including of gmp.h ... > * system.h: ... here. > * realmpfr.h: Do not include gmp.h. > * tree-ssa-loop-niter.c: Do not include gmp.h. > > fortran/ChangeLog > * gfortran.h: Do not include gmp.h. > > diff --git a/gcc/double-int.h b/gcc/double-int.h > index 3d9aa2c..7ea0528 100644 > --- a/gcc/double-int.h > +++ b/gcc/double-int.h > @@ -20,10 +20,6 @@ along with GCC; see the file COPYING3. If not see > #ifndef DOUBLE_INT_H > #define DOUBLE_INT_H > > -#ifndef GENERATOR_FILE > -#include <gmp.h> > -#endif > - > /* A large integer is currently represented as a pair of HOST_WIDE_INTs. > It therefore represents a number with precision of > 2 * HOST_BITS_PER_WIDE_INT bits (it is however possible that the > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h > index 7c4c0a4..611d16d 100644 > --- a/gcc/fortran/gfortran.h > +++ b/gcc/fortran/gfortran.h > @@ -1681,7 +1681,6 @@ gfc_intrinsic_sym; > EXPR_COMPCALL Function (or subroutine) call of a procedure pointer > component or type-bound procedure. */ > > -#include <gmp.h> > #include <mpfr.h> > #include <mpc.h> > #define GFC_RND_MODE GMP_RNDN > diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h > index ab234e9..ada876e 100644 > --- a/gcc/realmpfr.h > +++ b/gcc/realmpfr.h > @@ -22,7 +22,10 @@ > #ifndef GCC_REALGMP_H > #define GCC_REALGMP_H > > -#include <gmp.h> > +/* Note that we do not include gmp.h. It is included in system.h > + because it wrecks intl.h when compiling in C++ mode. > + See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 for details. */ > + > #include <mpfr.h> > #include <mpc.h> > #include "real.h" > diff --git a/gcc/system.h b/gcc/system.h > index 9e7d503..0ccd991 100644 > --- a/gcc/system.h > +++ b/gcc/system.h > @@ -1037,4 +1037,8 @@ helper_const_non_const_cast (const char *p) > #define DEBUG_VARIABLE > #endif > > +#ifndef GENERATOR_FILE > +#include <gmp.h> > +#endif > + > #endif /* ! GCC_SYSTEM_H */ > diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c > index c719a74..4c67c26 100644 > --- a/gcc/tree-ssa-loop-niter.c > +++ b/gcc/tree-ssa-loop-niter.c > @@ -38,7 +38,6 @@ along with GCC; see the file COPYING3. If not see > #include "flags.h" > #include "diagnostic-core.h" > #include "tree-inline.h" > -#include "gmp.h" > > #define SWAP(X, Y) do { affine_iv *tmp = (X); (X) = (Y); (Y) = tmp; } while (0) > > >
On 12-08-16 09:08 , Richard Guenther wrote: > It fixes it. > > Thus, ok from my side (if your testing succeeds). Worked here too. Committed to trunk. Diego.
On 12-08-16 09:08 , Richard Guenther wrote: > On Thu, 16 Aug 2012, Diego Novillo wrote: > >> Richi, this implements your idea for fixing PR 54281. I don't >> have an old enough compiler. Could you please test it in your >> system? >> >> I debated whether to remove the GENERATOR_FILE predicate from the >> inclusion (some files include gmp.h unconditionally). I think it >> could be removed, but only a minority of files build with >> GENERATOR_FILE set, so it didn't seem harmful. >> >> OK if tests pass? > > It fixes it. > > Thus, ok from my side (if your testing succeeds). So, I had failed to include Ada in my testing and my patch breaks it. In several ada/*.c files, we forcefully include system.h as a C header: #ifdef __cplusplus extern "C" { #endif ... #include "system.h" ... #ifdef __cplusplus } #endif Eric, is this really needed now? We are including gmp.h from system.h due to PR 54281. This is causing Ada builds to fail. Would it be OK to remove all the '#ifdef __cplusplus' conditionals from ada/*.c or is there something I'm missing? Thanks. Diego.
On Thu, Aug 16, 2012 at 07:55:51AM -0400, Diego Novillo wrote: > Richi, this implements your idea for fixing PR 54281. I don't > have an old enough compiler. Could you please test it in your > system? > > I debated whether to remove the GENERATOR_FILE predicate from the > inclusion (some files include gmp.h unconditionally). I think it > could be removed, but only a minority of files build with > GENERATOR_FILE set, so it didn't seem harmful. > > OK if tests pass? This patch breaks building with gmp, cloog and isl in subdirectories of the source tree. echo timestamp > s-options gawk -f ../../trunk/gcc/opt-functions.awk -f ../../trunk/gcc/opt-read.awk \ -f ../../trunk/gcc/opth-gen.awk \ < optionlist > tmp-options.h /bin/sh ../../trunk/gcc/../move-if-change tmp-options.h options.h echo timestamp > s-options-h TARGET_CPU_DEFAULT="" \ HEADERS="auto-host.h ansidecl.h" DEFINES="" \ /bin/sh ../../trunk/gcc/mkconfig.sh bconfig.h g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../trunk/gcc -I../../trunk/gcc/build -I../../trunk/gcc/../include -I../../trunk/gcc/../libcpp/include -I/home/magfr/src/gcc/build/./gmp -I/home/magfr/src/gcc/trunk/gmp -I/home/magfr/src/gcc/build/./mpfr -I/home/magfr/src/gcc/trunk/mpfr -I/home/magfr/src/gcc/trunk/mpc/src -I../../trunk/gcc/../libdecnumber -I../../trunk/gcc/../libdecnumber/bid -I../libdecnumber -DCLOOG_INT_GMP -I/home/magfr/src/gcc/build/./cloog/include -I/home/magfr/src/gcc/trunk/cloog/include -I../trunk/cloog/include -I/home/magfr/src/gcc/build/./isl/include -I/home/magfr/src/gcc/trunk/isl/include \ -o build/genconstants.o ../../trunk/gcc/genconstants.c In file included from /usr/include/sys/resource.h:25:0, from /usr/include/sys/wait.h:32, from ../../trunk/gcc/system.h:351, from ../../trunk/gcc/genconstants.c:29: /usr/include/bits/resource.h:133:18: error: declaration does not declare anything [-fpermissive] In file included from ../../trunk/gcc/genconstants.c:29:0: ../../trunk/gcc/system.h:443:23: error: declaration of C function `void* sbrk(int)' conflicts with In file included from ../../trunk/gcc/system.h:253:0, from ../../trunk/gcc/genconstants.c:29: /usr/include/unistd.h:1067:14: error: previous declaration `void* sbrk(intptr_t)' here In file included from ../../trunk/gcc/genconstants.c:29:0: ../../trunk/gcc/system.h:447:48: error: new declaration `char* strstr(const char*, const char*)' In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0, from ../../trunk/gcc/system.h:207, from ../../trunk/gcc/genconstants.c:29: /usr/include/string.h:323:22: error: ambiguates old declaration `const char* strstr(const char*, const char*)' In file included from ../../trunk/gcc/genconstants.c:29:0: ../../trunk/gcc/system.h:499:34: error: declaration of C function `const char* strsignal(int)' conflicts with In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0, from ../../trunk/gcc/system.h:207, from ../../trunk/gcc/genconstants.c:29: /usr/include/string.h:566:14: error: previous declaration `char* strsignal(int)' here In file included from ../../trunk/gcc/system.h:639:0, from ../../trunk/gcc/genconstants.c:29: ../../trunk/gcc/../include/libiberty.h:110:36: error: new declaration `char* basename(const char*)' In file included from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/../../../../include/c++/4.7.0/cstring:44:0, from ../../trunk/gcc/system.h:207, from ../../trunk/gcc/genconstants.c:29: /usr/include/string.h:603:28: error: ambiguates old declaration `const char* basename(const char*)' make[3]: *** [build/genconstants.o] Error 1 make[3]: Leaving directory `/home/magfr/src/gcc/build/gcc' make[2]: *** [all-stage1-gcc] Error 2 make[2]: Leaving directory `/home/magfr/src/gcc/build' make[1]: *** [stage1-bubble] Error 2 make[1]: Leaving directory `/home/magfr/src/gcc/build' make: *** [all] Error 2 The problem seems to be that during configure time the test scripts include system.h, but it fails to add the gmp include directory to the compiler flags so the checks for sbrk, strstr and so on fails with configure:10294: checking whether sbrk is declared configure:10317: gcc -c -g -I../../trunk/gcc -I../../trunk/gcc/../include conftest.c >&5 In file included from conftest.c:125:0: ../../trunk/gcc/system.h:1041:17: fatal error: gmp.h: No such file or directory compilation terminated. configure:10317: $? = 1 Reverting this patch makes the compiler build. /MF
On Thu, Aug 16, 2012 at 1:50 PM, Magnus Fromreide <magfr@lysator.liu.se> wrote: > On Thu, Aug 16, 2012 at 07:55:51AM -0400, Diego Novillo wrote: >> Richi, this implements your idea for fixing PR 54281. I don't >> have an old enough compiler. Could you please test it in your >> system? >> >> I debated whether to remove the GENERATOR_FILE predicate from the >> inclusion (some files include gmp.h unconditionally). I think it >> could be removed, but only a minority of files build with >> GENERATOR_FILE set, so it didn't seem harmful. >> >> OK if tests pass? > > This patch breaks building with gmp, cloog and isl in subdirectories of the > source tree. Working on a fix. It also exposed problems in Ada. Diego.
[Sorry for the delay] > So, I had failed to include Ada in my testing and my patch breaks it. > In several ada/*.c files, we forcefully include system.h as a C header: > > #ifdef __cplusplus > extern "C" { > #endif > ... > #include "system.h" > ... > #ifdef __cplusplus > } > #endif > > > Eric, is this really needed now? We are including gmp.h from system.h > due to PR 54281. This is causing Ada builds to fail. > > Would it be OK to remove all the '#ifdef __cplusplus' conditionals from > ada/*.c or is there something I'm missing? The conditionals cannot be removed for the time being because the foreign language interface of the Ada part of the compiler is hardcoded for C. Barring a massive switch to the Ada language for the GNU project, we would need to switch the foreign language interface to C++, which might introduce various maintenance issues in the short term (Arno CCed).
> The conditionals cannot be removed for the time being because the foreign > language interface of the Ada part of the compiler is hardcoded for C. > > Barring a massive switch to the Ada language for the GNU project, we would need > to switch the foreign language interface to C++, which might introduce various > maintenance issues in the short term (Arno CCed). Yes, that would be a large hearthquake for both the compiler and the run-time, which is unwanted at this stage. I guess the solution for now is to more selectively export the various symbols as C symbols and include header files as is. Arno
On 2012-08-19 07:18 , Arnaud Charlet wrote: > >> The conditionals cannot be removed for the time being because the >> foreign language interface of the Ada part of the compiler is >> hardcoded for C. >> >> Barring a massive switch to the Ada language for the GNU project, >> we would need to switch the foreign language interface to C++, >> which might introduce various maintenance issues in the short term >> (Arno CCed). > > Yes, that would be a large hearthquake for both the compiler and the > run-time, which is unwanted at this stage. I guess the solution for > now is to more selectively export the various symbols as C symbols > and include header files as is. Thanks. One potential issue I see here is that as we start using more C++ headers (and more C++ in existing headers), we will reach a point in which including something like system.h inside an extern "C" {} block will fail. None of the GCC headers should be included as C code anymore. Diego.
diff --git a/gcc/double-int.h b/gcc/double-int.h index 3d9aa2c..7ea0528 100644 --- a/gcc/double-int.h +++ b/gcc/double-int.h @@ -20,10 +20,6 @@ along with GCC; see the file COPYING3. If not see #ifndef DOUBLE_INT_H #define DOUBLE_INT_H -#ifndef GENERATOR_FILE -#include <gmp.h> -#endif - /* A large integer is currently represented as a pair of HOST_WIDE_INTs. It therefore represents a number with precision of 2 * HOST_BITS_PER_WIDE_INT bits (it is however possible that the diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 7c4c0a4..611d16d 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -1681,7 +1681,6 @@ gfc_intrinsic_sym; EXPR_COMPCALL Function (or subroutine) call of a procedure pointer component or type-bound procedure. */ -#include <gmp.h> #include <mpfr.h> #include <mpc.h> #define GFC_RND_MODE GMP_RNDN diff --git a/gcc/realmpfr.h b/gcc/realmpfr.h index ab234e9..ada876e 100644 --- a/gcc/realmpfr.h +++ b/gcc/realmpfr.h @@ -22,7 +22,10 @@ #ifndef GCC_REALGMP_H #define GCC_REALGMP_H -#include <gmp.h> +/* Note that we do not include gmp.h. It is included in system.h + because it wrecks intl.h when compiling in C++ mode. + See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54281 for details. */ + #include <mpfr.h> #include <mpc.h> #include "real.h" diff --git a/gcc/system.h b/gcc/system.h index 9e7d503..0ccd991 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -1037,4 +1037,8 @@ helper_const_non_const_cast (const char *p) #define DEBUG_VARIABLE #endif +#ifndef GENERATOR_FILE +#include <gmp.h> +#endif + #endif /* ! GCC_SYSTEM_H */ diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c index c719a74..4c67c26 100644 --- a/gcc/tree-ssa-loop-niter.c +++ b/gcc/tree-ssa-loop-niter.c @@ -38,7 +38,6 @@ along with GCC; see the file COPYING3. If not see #include "flags.h" #include "diagnostic-core.h" #include "tree-inline.h" -#include "gmp.h" #define SWAP(X, Y) do { affine_iv *tmp = (X); (X) = (Y); (Y) = tmp; } while (0)
Richi, this implements your idea for fixing PR 54281. I don't have an old enough compiler. Could you please test it in your system? I debated whether to remove the GENERATOR_FILE predicate from the inclusion (some files include gmp.h unconditionally). I think it could be removed, but only a minority of files build with GENERATOR_FILE set, so it didn't seem harmful. OK if tests pass? Diego. 2012-08-16 Diego Novillo <dnovillo@google.com> Richard Guenther <rguenther@suse.de> PR bootstrap/54281 * double-int.h: Move including of gmp.h ... * system.h: ... here. * realmpfr.h: Do not include gmp.h. * tree-ssa-loop-niter.c: Do not include gmp.h. fortran/ChangeLog * gfortran.h: Do not include gmp.h.