Message ID | db8a236e-c195-2494-2442-c64d5655b5b0@redhat.com |
---|---|
State | New |
Headers | show |
Series | RFC: Changing AC_PROG_CC to AC_PROG_CC_C99 in top level configure | expand |
On Mon, 26 Apr 2021, Nick Clifton via Gcc-patches wrote: > Hi Guys, > > Given that gcc, gdb and now binutils are all now requiring C99 as a > minimum version of C, are there any objections to updating > configure.ac to reflect this ? This isn't an objection, since upgrading auto* for the toolchain can be complicated, but note that AC_PROG_CC_C99 is obsolete in Autoconf 2.70 and instead AC_PROG_CC enables C11 mode if supported. (So moving to the latest Autoconf and Automake releases would supersede this change.)
On 26 Apr 2021 19:32, Joseph Myers wrote: > On Mon, 26 Apr 2021, Nick Clifton via Gcc-patches wrote: > > Given that gcc, gdb and now binutils are all now requiring C99 as a > > minimum version of C, are there any objections to updating > > configure.ac to reflect this ? > > This isn't an objection, since upgrading auto* for the toolchain can be > complicated, but note that AC_PROG_CC_C99 is obsolete in Autoconf 2.70 and > instead AC_PROG_CC enables C11 mode if supported. (So moving to the > latest Autoconf and Automake releases would supersede this change.) considering how long it took before we adopted 2.69, seems unlikely we can upgrade to 2.70. plus, i think there was a flurry of regression fixes for 2.70, and ideally we'd get a 2.71 ? as long as we have 2.69, we should move to AC_PROG_CC_C99 in all projects. -mike
On Mon, Apr 26, 2021 at 2:32 PM Joseph Myers <joseph@codesourcery.com> wrote: > > On Mon, 26 Apr 2021, Nick Clifton via Gcc-patches wrote: > > > Hi Guys, > > > > Given that gcc, gdb and now binutils are all now requiring C99 as a > > minimum version of C, are there any objections to updating > > configure.ac to reflect this ? > > This isn't an objection, since upgrading auto* for the toolchain can be > complicated, but note that AC_PROG_CC_C99 is obsolete in Autoconf 2.70 and > instead AC_PROG_CC enables C11 mode if supported. (So moving to the > latest Autoconf and Automake releases would supersede this change.) But AC_PROG_CC_C99 makes sure that the compiler supports C99, and AC_PROG_CC does not. Christian
Hi Joseph, > This isn't an objection, since upgrading auto* for the toolchain can be > complicated, but note that AC_PROG_CC_C99 is obsolete in Autoconf 2.70 Ah - in which case changing to an about-to-be-obsolete macro is probably a bad idea. > and instead AC_PROG_CC enables C11 mode if supported. (So moving to the > latest Autoconf and Automake releases would supersede this change.) Makes sense. Is changing to autoconf 2.70 something that is planned for the near future ? I actually have a Fedora BZ open suggesting that the binutils be upgraded to use autoconf 2.71. I have put off doing this however as I am not an autoconf expert and I have no idea what the consequences might be. Cheers Nick
On Tue, 27 Apr 2021, Nick Clifton via Binutils wrote: > > and instead AC_PROG_CC enables C11 mode if supported. (So moving to the > > latest Autoconf and Automake releases would supersede this change.) > > Makes sense. Is changing to autoconf 2.70 something that is planned for the > near future ? It would be 2.71, and I don't know. (Simon Marchi did the previous update for binutils-gdb, I then used that as a basis for updating GCC.)
On 2021-04-26 7:32 a.m., Nick Clifton via Gdb-patches wrote:> Hi Guys, > > Given that gcc, gdb and now binutils are all now requiring C99 as a > minimum version of C, are there any objections to updating > configure.ac to reflect this ? > > Cheers > Nick > > diff --git a/configure.ac b/configure.ac > index a721316d07b..59b4194fb24 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1278,7 +1278,7 @@ else > WINDMC_FOR_BUILD="\$(WINDMC)" > fi > > -AC_PROG_CC > +AC_PROG_CC_C99 > AC_PROG_CXX > > # We must set the default linker to the linker used by gcc for the correct Hi Nick, I think this fix is obvious enough, I encourage you to push it, that will fix the build failure many people get in opcodes/ppc-dis.c. We'll just remove the line later when we upgrade to Autoconf 2.71, as simple as that. For now we use 2.69. If that matters, you have my OK for the GDB side of things. Simon
On 4/30/2021 12:36 PM, Simon Marchi via Gcc-patches wrote: > On 2021-04-26 7:32 a.m., Nick Clifton via Gdb-patches wrote:> Hi Guys, >> Given that gcc, gdb and now binutils are all now requiring C99 as a >> minimum version of C, are there any objections to updating >> configure.ac to reflect this ? >> >> Cheers >> Nick >> >> diff --git a/configure.ac b/configure.ac >> index a721316d07b..59b4194fb24 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -1278,7 +1278,7 @@ else >> WINDMC_FOR_BUILD="\$(WINDMC)" >> fi >> >> -AC_PROG_CC >> +AC_PROG_CC_C99 >> AC_PROG_CXX >> >> # We must set the default linker to the linker used by gcc for the correct > Hi Nick, > > I think this fix is obvious enough, I encourage you to push it, that > will fix the build failure many people get in opcodes/ppc-dis.c. We'll > just remove the line later when we upgrade to Autoconf 2.71, as simple > as that. For now we use 2.69. If that matters, you have my OK for the > GDB side of things. That works for me. I'd just sent Alan the trivial patch to make ppc-dis.c compile again with C89, but if we're going to update configure.ac appropriately, then it wouldn't be needed. Jeff
On Fri, Apr 30, 2021 at 03:48:00PM -0600, Jeff Law via Gcc-patches wrote: > > On 4/30/2021 12:36 PM, Simon Marchi via Gcc-patches wrote: > > On 2021-04-26 7:32 a.m., Nick Clifton via Gdb-patches wrote:> Hi Guys, > > > Given that gcc, gdb and now binutils are all now requiring C99 as a > > > minimum version of C, are there any objections to updating > > > configure.ac to reflect this ? > > > > > > Cheers > > > Nick > > > > > > diff --git a/configure.ac b/configure.ac > > > index a721316d07b..59b4194fb24 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -1278,7 +1278,7 @@ else > > > WINDMC_FOR_BUILD="\$(WINDMC)" > > > fi > > > > > > -AC_PROG_CC > > > +AC_PROG_CC_C99 > > > AC_PROG_CXX > > > > > > # We must set the default linker to the linker used by gcc for the correct > > Hi Nick, > > > > I think this fix is obvious enough, I encourage you to push it, that > > will fix the build failure many people get in opcodes/ppc-dis.c. We'll > > just remove the line later when we upgrade to Autoconf 2.71, as simple > > as that. For now we use 2.69. If that matters, you have my OK for the > > GDB side of things. > > That works for me. I'd just sent Alan the trivial patch to make ppc-dis.c > compile again with C89, but if we're going to update configure.ac > appropriately, then it wouldn't be needed. Yes, I prefer the configure fix too. If we state we require C99 in binutils then we ought to be able to use C99.. Nick, does the configure.ac change also need to go in all subdirs, to support people running make in say ld/ rather than running make in the top build dir?
> Yes, I prefer the configure fix too. If we state we require C99 in > binutils then we ought to be able to use C99.. > > Nick, does the configure.ac change also need to go in all subdirs, to > support people running make in say ld/ rather than running make in the > top build dir? For GDB, it's not supported to run gdb/configure directly, you need to use the top-level configure. Is it supported from some of the other projects in the repo? I just tried with ld, it doesn't work since it depends on bfd also being built. I tried with just bfd, it doesn't work (with the default configure options at least) because it requires zlib being built. So if all projects need to go through the top-level configure script anyway, and C99 is a baseline for all projects, then having the check only in the top-level makes sense to me. Projects that have more specific requirements can have their own checks. For example, sim/ requires C11 now. Unless the C99 check at top-level somehow does not play well with the C11 check in sim/? Like if that would cause CC to be set to "gcc -std=gnu99 -std=gnu11" or something like that. Simon
>>>>> "Simon" == Simon Marchi via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
Simon> For GDB, it's not supported to run gdb/configure directly, you need to
Simon> use the top-level configure. Is it supported from some of the other
Simon> projects in the repo?
It can be done sometimes but I think it isn't really a scenario worth
worrying about. Normally this kind of thing is only doable for sources
that are already independent of the top-level configury -- drop-in stuff
like gmp or libiconv, or roots of the tree like libiberty and gnulib.
Simon> I just tried with ld, it doesn't work since it depends on bfd also being
Simon> built. I tried with just bfd, it doesn't work (with the default
Simon> configure options at least) because it requires zlib being built.
Yeah, most of the things that are "really" in-tree have dependencies and
can't be built independently of the rest.
Tom
On Mon, May 03, 2021 at 10:47:15AM -0400, Simon Marchi wrote: > > Yes, I prefer the configure fix too. If we state we require C99 in > > binutils then we ought to be able to use C99.. > > > > Nick, does the configure.ac change also need to go in all subdirs, to > > support people running make in say ld/ rather than running make in the > > top build dir? > > For GDB, it's not supported to run gdb/configure directly, you need to > use the top-level configure. Is it supported from some of the other > projects in the repo? > > I just tried with ld, it doesn't work since it depends on bfd also being > built. I tried with just bfd, it doesn't work (with the default > configure options at least) because it requires zlib being built. I wasn't talking about running configure, I was talking about running make. For example, you configure and make binutils as usual, then after making a change to ld/ files, run make in the ld build dir. I don't tend to do that myself but I do run "make check" sometimes in a subdir expecting to get the same results in that subdir as if "make check" was run from the top level. But I should have just tried it myself rather than asking. CC, CPP and others are inherited from the top level and appear with -std=gnu99 in the subdir Makefiles. So it seems all the AC_PROG_CC in subdir configure.ac can stay as they are. > > So if all projects need to go through the top-level configure script > anyway, and C99 is a baseline for all projects, then having the check > only in the top-level makes sense to me. Projects that have more > specific requirements can have their own checks. For example, sim/ > requires C11 now. Unless the C99 check at top-level somehow does not > play well with the C11 check in sim/? Like if that would cause CC to be > set to "gcc -std=gnu99 -std=gnu11" or something like that. > > Simon
On 2021-05-03 5:51 p.m., Alan Modra wrote: > I wasn't talking about running configure, I was talking about running > make. For example, you configure and make binutils as usual, then > after making a change to ld/ files, run make in the ld build dir. I > don't tend to do that myself but I do run "make check" sometimes in a > subdir expecting to get the same results in that subdir as if "make > check" was run from the top level. Ah yeah, that works just fine. During my edit-build-cycle, I typically only "make" in gdb/, or whatever I'm working on at the moment. It saves some precious milliseconds! > But I should have just tried it myself rather than asking. CC, CPP > and others are inherited from the top level and appear with -std=gnu99 > in the subdir Makefiles. So it seems all the AC_PROG_CC in subdir > configure.ac can stay as they are. Yes, the top-level passes CC=... and others when it configures the subdirectories. The subdirectories remember how they were configures (in config.status), so that even if a "make" in the subdirectory ends up re-running configure (because configure changed), the CC value will be remembered. Simon
Hi Guys,
On 4/30/21 7:36 PM, Simon Marchi wrote:
> I think this fix is obvious enough, I encourage you to push it,
OK - I have pushed the patch to the mainline branches of both
the gcc and binutils-gfdb repositories.
Cheers
Nick
On 2021-05-04 8:42 a.m., Nick Clifton wrote: > Hi Guys, > > On 4/30/21 7:36 PM, Simon Marchi wrote: >> I think this fix is obvious enough, I encourage you to push it, > > OK - I have pushed the patch to the mainline branches of both > the gcc and binutils-gfdb repositories. > > Cheers > Nick > Thanks Nick! Simon
On 2021-05-04 8:42 a.m., Nick Clifton wrote: > Hi Guys, > > On 4/30/21 7:36 PM, Simon Marchi wrote: >> I think this fix is obvious enough, I encourage you to push it, > > OK - I have pushed the patch to the mainline branches of both > the gcc and binutils-gdb repositories. Thanks Nick! Incidentally, I checked the AC_PROG_CC_C99 change on both binutils and gcc mainline using gcc-4.9. To build gcc on x86_64 I found the following patch necessary to avoid lots of error: uninitialized const member ‘stringop_algs::stringop_strategy::max’ error: uninitialized const member ‘stringop_algs::stringop_strategy::alg’ when compiling config/i386/i386-options.c. These can't be cured by configuring with --disable-stage1-checking. diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 97d6f3863cb..cc3b1b6d666 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -73,8 +73,8 @@ struct stringop_algs { const enum stringop_alg unknown_size; const struct stringop_strategy { - const int max; - const enum stringop_alg alg; + int max; + enum stringop_alg alg; int noalign; } size [MAX_STRINGOP_ALGS]; };
Alan Modra via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > On 2021-05-04 8:42 a.m., Nick Clifton wrote: >> Hi Guys, >> >> On 4/30/21 7:36 PM, Simon Marchi wrote: >>> I think this fix is obvious enough, I encourage you to push it, >> >> OK - I have pushed the patch to the mainline branches of both >> the gcc and binutils-gdb repositories. > > Thanks Nick! Incidentally, I checked the AC_PROG_CC_C99 change on > both binutils and gcc mainline using gcc-4.9. > > To build gcc on x86_64 I found the following patch necessary to avoid > lots of > error: uninitialized const member ‘stringop_algs::stringop_strategy::max’ > error: uninitialized const member ‘stringop_algs::stringop_strategy::alg’ > when compiling config/i386/i386-options.c. These can't be cured by > configuring with --disable-stage1-checking. > > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h > index 97d6f3863cb..cc3b1b6d666 100644 > --- a/gcc/config/i386/i386.h > +++ b/gcc/config/i386/i386.h > @@ -73,8 +73,8 @@ struct stringop_algs > { > const enum stringop_alg unknown_size; > const struct stringop_strategy { > - const int max; > - const enum stringop_alg alg; > + int max; > + enum stringop_alg alg; > int noalign; > } size [MAX_STRINGOP_ALGS]; > }; does this relate to / fix PR 100246 (which seems to fire for some GCC versions as well as older clang)? Iain
On Wed, May 05, 2021 at 08:05:29AM +0100, Iain Sandoe wrote: > Alan Modra via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > On 2021-05-04 8:42 a.m., Nick Clifton wrote: > > > Hi Guys, > > > > > > On 4/30/21 7:36 PM, Simon Marchi wrote: > > > > I think this fix is obvious enough, I encourage you to push it, > > > > > > OK - I have pushed the patch to the mainline branches of both > > > the gcc and binutils-gdb repositories. > > > > Thanks Nick! Incidentally, I checked the AC_PROG_CC_C99 change on > > both binutils and gcc mainline using gcc-4.9. > > > > To build gcc on x86_64 I found the following patch necessary to avoid > > lots of > > error: uninitialized const member ‘stringop_algs::stringop_strategy::max’ > > error: uninitialized const member ‘stringop_algs::stringop_strategy::alg’ > > when compiling config/i386/i386-options.c. These can't be cured by > > configuring with --disable-stage1-checking. > > > > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h > > index 97d6f3863cb..cc3b1b6d666 100644 > > --- a/gcc/config/i386/i386.h > > +++ b/gcc/config/i386/i386.h > > @@ -73,8 +73,8 @@ struct stringop_algs > > { > > const enum stringop_alg unknown_size; > > const struct stringop_strategy { > > - const int max; > > - const enum stringop_alg alg; > > + int max; > > + enum stringop_alg alg; > > int noalign; > > } size [MAX_STRINGOP_ALGS]; > > }; > > does this relate to / fix PR 100246 (which seems to fire for some GCC > versions as well > as older clang)? Yes, looks like the same issue. I started making a similar fix to the one you attached to the PR, then laziness kicked in after noticing the errors were only given on the const elements.
Alan Modra <amodra@gmail.com> wrote: > On Wed, May 05, 2021 at 08:05:29AM +0100, Iain Sandoe wrote: >> Alan Modra via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >> >>> On 2021-05-04 8:42 a.m., Nick Clifton wrote: >>>> Hi Guys, >>>> >>>> On 4/30/21 7:36 PM, Simon Marchi wrote: >>>>> I think this fix is obvious enough, I encourage you to push it, >>>> >>>> OK - I have pushed the patch to the mainline branches of both >>>> the gcc and binutils-gdb repositories. >>> >>> Thanks Nick! Incidentally, I checked the AC_PROG_CC_C99 change on >>> both binutils and gcc mainline using gcc-4.9. >>> >>> To build gcc on x86_64 I found the following patch necessary to avoid >>> lots of >>> error: uninitialized const member ‘stringop_algs::stringop_strategy::max’ >>> error: uninitialized const member ‘stringop_algs::stringop_strategy::alg’ >>> when compiling config/i386/i386-options.c. These can't be cured by >>> configuring with --disable-stage1-checking. >>> >>> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h >>> index 97d6f3863cb..cc3b1b6d666 100644 >>> --- a/gcc/config/i386/i386.h >>> +++ b/gcc/config/i386/i386.h >>> @@ -73,8 +73,8 @@ struct stringop_algs >>> { >>> const enum stringop_alg unknown_size; >>> const struct stringop_strategy { >>> - const int max; >>> - const enum stringop_alg alg; >>> + int max; >>> + enum stringop_alg alg; >>> int noalign; >>> } size [MAX_STRINGOP_ALGS]; >>> }; >> >> does this relate to / fix PR 100246 (which seems to fire for some GCC >> versions as well >> as older clang)? > > Yes, looks like the same issue. I started making a similar fix to the > one you attached to the PR, then laziness kicked in after noticing the > errors were only given on the const elements. OK, so this has been applied to master already IIUC - I will queue up a run to check master is OK with the clang versions that fail. It will be needed on GCC11.2 as well. thanks Iain
Alan Modra <amodra@gmail.com> wrote: > On Wed, May 05, 2021 at 08:05:29AM +0100, Iain Sandoe wrote: >> Alan Modra via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >>> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h >>> index 97d6f3863cb..cc3b1b6d666 100644 >>> --- a/gcc/config/i386/i386.h >>> +++ b/gcc/config/i386/i386.h >>> @@ -73,8 +73,8 @@ struct stringop_algs >>> { >>> const enum stringop_alg unknown_size; >>> const struct stringop_strategy { >>> - const int max; >>> - const enum stringop_alg alg; >>> + int max; >>> + enum stringop_alg alg; >>> int noalign; >>> } size [MAX_STRINGOP_ALGS]; >>> }; >> >> does this relate to / fix PR 100246 (which seems to fire for some GCC >> versions as well >> as older clang)? > > Yes, looks like the same issue. I started making a similar fix to the > one you attached to the PR, then laziness kicked in after noticing the > errors were only given on the const elements. I added a third variant to the PR (as below), which preserves the const-ness but provides a CTOR. TBH, I have no especial preference for the solution, but it would be nice to commit one of them :-) cheers Iain The condition is because this header gets pulled in by gcov stuff which is built with a C compiler. diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 97d6f38..a417c93 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -73,6 +73,11 @@ struct stringop_algs { const enum stringop_alg unknown_size; const struct stringop_strategy { +#ifdef __cplusplus + stringop_strategy(int _max = -1, enum stringop_alg _alg = libcall, + int _noalign = false) + : max (_max), alg (_alg), noalign (_noalign) {} +#endif const int max; const enum stringop_alg alg; int noalign;
diff --git a/configure.ac b/configure.ac index a721316d07b..59b4194fb24 100644 --- a/configure.ac +++ b/configure.ac @@ -1278,7 +1278,7 @@ else WINDMC_FOR_BUILD="\$(WINDMC)" fi -AC_PROG_CC +AC_PROG_CC_C99 AC_PROG_CXX # We must set the default linker to the linker used by gcc for the correct