Message ID | 20230601155856.305565-1-jwakely@redhat.com |
---|---|
State | New |
Headers | show |
Series | libstdc++: Use AS_IF in configure.ac | expand |
On Thu, 1 Jun 2023 at 16:59, Jonathan Wakely via Libstdc++ < libstdc++@gcc.gnu.org> wrote: > Tested x86_64-linux. I'd appreciate a second set of eyeballs on this > before I push it. > Pushed to trunk now. > > -- >8 -- > > This ensures that anything that depends on AC_REQUIRE is hoisted out of > the conditional block. > > The always-false test x"long_double_math_on_this_cpu" = x"yes" condition > is not altered by this commit, only changed to use the AS_IF syntax. > > libstdc++-v3/ChangeLog: > > * configure.ac: Use AS_IF. > * configure: Regenerate. > --- > libstdc++-v3/configure | 1148 +++++++++++++++++++------------------ > libstdc++-v3/configure.ac | 20 +- > 2 files changed, 590 insertions(+), 578 deletions(-) > > diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac > index 0abe54e7b9a..f3bcf7affdd 100644 > --- a/libstdc++-v3/configure.ac > +++ b/libstdc++-v3/configure.ac > @@ -266,7 +266,7 @@ AC_CHECK_HEADERS([linux/random.h], [], [], > AC_CHECK_HEADERS([xlocale.h]) > > # Only do link tests if native. Else, hardcode. > -if $GLIBCXX_IS_NATIVE; then > +AS_IF([$GLIBCXX_IS_NATIVE],[ > > # We can do more elaborate tests that assume a working linker. > CANADIAN=no > @@ -298,7 +298,7 @@ if $GLIBCXX_IS_NATIVE; then > # For iconv support. > AM_ICONV > > -else > +],[ > > # This lets us hard-code the functionality we know we'll have in the > cross > # target environment. "Let" is a sugar-coated word placed on an > especially > @@ -330,7 +330,7 @@ else > > # First, test for "known" system libraries. We may be using newlib even > # on a hosted environment. > - if test "x${with_newlib}" = "xyes"; then > + AS_IF([test "x${with_newlib}" = "xyes"],[ > os_include_dir="os/newlib" > AC_DEFINE(HAVE_HYPOT) > > @@ -386,14 +386,14 @@ else > AC_DEFINE(HAVE_USLEEP) > ;; > esac > - elif test "x$with_headers" != "xno"; then > + ],[test "x$with_headers" != "xno" ],[ > GLIBCXX_CROSSCONFIG > - fi > + ]) > > # At some point, we should differentiate between architectures > # like x86, which have long double versions, and alpha/powerpc/etc., > # which don't. For the time being, punt. > - if test x"long_double_math_on_this_cpu" = x"yes"; then > + AS_IF([test x"long_double_math_on_this_cpu" = x"yes"],[ > AC_DEFINE(HAVE_ACOSL) > AC_DEFINE(HAVE_ASINL) > AC_DEFINE(HAVE_ATAN2L) > @@ -417,8 +417,8 @@ else > AC_DEFINE(HAVE_SQRTL) > AC_DEFINE(HAVE_TANL) > AC_DEFINE(HAVE_TANHL) > - fi > -fi > + ]) > +]) > > # Check for _Unwind_GetIPInfo. > GCC_CHECK_UNWIND_GETIPINFO > @@ -449,7 +449,7 @@ case "$target" in > #error no need for long double compatibility > #endif > ], [ac_ldbl_compat=yes], [ac_ldbl_compat=no]) > - if test "$ac_ldbl_compat" = yes; then > + AS_IF([test "$ac_ldbl_compat" = yes],[ > AC_DEFINE([_GLIBCXX_LONG_DOUBLE_COMPAT],1, > [Define if compatibility should be provided for > -mlong-double-64.]) > > port_specific_symbol_files="\$(top_srcdir)/config/os/gnu-linux/ldbl-extra.ver" > @@ -485,7 +485,7 @@ case "$target" in > fi > ;; > esac > - fi > + ]) > esac > AC_SUBST(LONG_DOUBLE_COMPAT_FLAGS) > AC_SUBST(LONG_DOUBLE_128_FLAGS) > -- > 2.40.1 > >
> Date: Tue, 6 Jun 2023 16:30:12 +0100 > From: Jonathan Wakely via Gcc-patches <gcc-patches@gcc.gnu.org> > On Thu, 1 Jun 2023 at 16:59, Jonathan Wakely via Libstdc++ < > libstdc++@gcc.gnu.org> wrote: > > > Tested x86_64-linux. I'd appreciate a second set of eyeballs on this > > before I push it. > > > > Pushed to trunk now. ...as r14-1581-g97a5e8a2a48d16, after which (apparently) *all* linking libstdc++ tests for cris-elf (a "newlib target") get (for example): FAIL: 17_intro/freestanding.cc (test for excess errors) Excess errors: /x/cris-elf/pre/cris-elf/bin/ld: cannot find -liconv: No such file or directory (deduced from libstdc++.log and the commits in the range ce2188e4320c..585c660f041c where 4144 regressions in libstdc++ were introduced for cris-elf) From the generated configure and a brief RTFM for AS_IF, it looks almost like AS_IF was "miscompiled" and behaving literally AS_IF (!) in that the condition TEST1 (here [$GLIBCXX_IS_NATIVE] seems to be emitted *after* the RUN-IF-TRUE1 clause (the next 31 lines). Not obvious what went wrong. I even tried regenerating configure. HTH. brgds, H-P
On Wed, 7 Jun 2023 at 15:42, Hans-Peter Nilsson <hp@axis.com> wrote: > > Date: Tue, 6 Jun 2023 16:30:12 +0100 > > From: Jonathan Wakely via Gcc-patches <gcc-patches@gcc.gnu.org> > > > On Thu, 1 Jun 2023 at 16:59, Jonathan Wakely via Libstdc++ < > > libstdc++@gcc.gnu.org> wrote: > > > > > Tested x86_64-linux. I'd appreciate a second set of eyeballs on this > > > before I push it. > > > > > > > Pushed to trunk now. > > ...as r14-1581-g97a5e8a2a48d16, after which (apparently) > *all* linking libstdc++ tests for cris-elf (a "newlib > target") get (for example): > > FAIL: 17_intro/freestanding.cc (test for excess errors) > Excess errors: > /x/cris-elf/pre/cris-elf/bin/ld: cannot find -liconv: No such file or > directory > > (deduced from libstdc++.log and the commits in the range > ce2188e4320c..585c660f041c where 4144 regressions in > libstdc++ were introduced for cris-elf) > Gah. I tested building cris-elf but didn't run any tests. I *thought* I compared the configure results before and after the patch too, but I guess I missed something, or it didn't show up where I looked. > From the generated configure and a brief RTFM for AS_IF, it > looks almost like AS_IF was "miscompiled" and behaving > literally AS_IF (!) in that the condition TEST1 (here > [$GLIBCXX_IS_NATIVE] seems to be emitted *after* the > RUN-IF-TRUE1 clause (the next 31 lines). Not obvious what > went wrong. I even tried regenerating configure. HTH. > > Let's just revert it then. The manual says we should use AS_IF, but what we had previously was working well enough. I'll figure out what happened here later.
On Wed, 7 Jun 2023 at 15:54, Jonathan Wakely <jwakely@redhat.com> wrote: > > > On Wed, 7 Jun 2023 at 15:42, Hans-Peter Nilsson <hp@axis.com> wrote: > >> > Date: Tue, 6 Jun 2023 16:30:12 +0100 >> > From: Jonathan Wakely via Gcc-patches <gcc-patches@gcc.gnu.org> >> >> > On Thu, 1 Jun 2023 at 16:59, Jonathan Wakely via Libstdc++ < >> > libstdc++@gcc.gnu.org> wrote: >> > >> > > Tested x86_64-linux. I'd appreciate a second set of eyeballs on this >> > > before I push it. >> > > >> > >> > Pushed to trunk now. >> >> ...as r14-1581-g97a5e8a2a48d16, after which (apparently) >> *all* linking libstdc++ tests for cris-elf (a "newlib >> target") get (for example): >> >> FAIL: 17_intro/freestanding.cc (test for excess errors) >> Excess errors: >> /x/cris-elf/pre/cris-elf/bin/ld: cannot find -liconv: No such file or >> directory >> >> (deduced from libstdc++.log and the commits in the range >> ce2188e4320c..585c660f041c where 4144 regressions in >> libstdc++ were introduced for cris-elf) >> > > Gah. I tested building cris-elf but didn't run any tests. > > I *thought* I compared the configure results before and after the patch > too, but I guess I missed something, or it didn't show up where I looked. > > > >> From the generated configure and a brief RTFM for AS_IF, it >> looks almost like AS_IF was "miscompiled" and behaving >> literally AS_IF (!) in that the condition TEST1 (here >> [$GLIBCXX_IS_NATIVE] seems to be emitted *after* the >> RUN-IF-TRUE1 clause (the next 31 lines). Not obvious what >> went wrong. I even tried regenerating configure. HTH. >> >> > Let's just revert it then. The manual says we should use AS_IF, but what > we had previously was working well enough. I'll figure out what happened > here later. > Reverted as r14-1607-g000f8b9a6a0ec7
On Jun 07 2023, Jonathan Wakely via Gcc-patches wrote: > Let's just revert it then. The manual says we should use AS_IF, but what we > had previously was working well enough. I'll figure out what happened here > later. I think AS_IF is doing its job here: moving the expansion of AC_REQUIRE'd macros out of the bodies. But many of those expansions actually need to remain under the $GLIBCXX_IS_NATIVE conditional, so it is not appropriate at this place.
On Wed, 7 Jun 2023 at 16:19, Andreas Schwab wrote: > On Jun 07 2023, Jonathan Wakely via Gcc-patches wrote: > > > Let's just revert it then. The manual says we should use AS_IF, but what > we > > had previously was working well enough. I'll figure out what happened > here > > later. > > I think AS_IF is doing its job here: moving the expansion of > AC_REQUIRE'd macros out of the bodies. But many of those expansions > actually need to remain under the $GLIBCXX_IS_NATIVE conditional, so it > is not appropriate at this place. > > Ah yes, that makes sense. Thanks.
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 0abe54e7b9a..f3bcf7affdd 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -266,7 +266,7 @@ AC_CHECK_HEADERS([linux/random.h], [], [], AC_CHECK_HEADERS([xlocale.h]) # Only do link tests if native. Else, hardcode. -if $GLIBCXX_IS_NATIVE; then +AS_IF([$GLIBCXX_IS_NATIVE],[ # We can do more elaborate tests that assume a working linker. CANADIAN=no @@ -298,7 +298,7 @@ if $GLIBCXX_IS_NATIVE; then # For iconv support. AM_ICONV -else +],[ # This lets us hard-code the functionality we know we'll have in the cross # target environment. "Let" is a sugar-coated word placed on an especially @@ -330,7 +330,7 @@ else # First, test for "known" system libraries. We may be using newlib even # on a hosted environment. - if test "x${with_newlib}" = "xyes"; then + AS_IF([test "x${with_newlib}" = "xyes"],[ os_include_dir="os/newlib" AC_DEFINE(HAVE_HYPOT) @@ -386,14 +386,14 @@ else AC_DEFINE(HAVE_USLEEP) ;; esac - elif test "x$with_headers" != "xno"; then + ],[test "x$with_headers" != "xno" ],[ GLIBCXX_CROSSCONFIG - fi + ]) # At some point, we should differentiate between architectures # like x86, which have long double versions, and alpha/powerpc/etc., # which don't. For the time being, punt. - if test x"long_double_math_on_this_cpu" = x"yes"; then + AS_IF([test x"long_double_math_on_this_cpu" = x"yes"],[ AC_DEFINE(HAVE_ACOSL) AC_DEFINE(HAVE_ASINL) AC_DEFINE(HAVE_ATAN2L) @@ -417,8 +417,8 @@ else AC_DEFINE(HAVE_SQRTL) AC_DEFINE(HAVE_TANL) AC_DEFINE(HAVE_TANHL) - fi -fi + ]) +]) # Check for _Unwind_GetIPInfo. GCC_CHECK_UNWIND_GETIPINFO @@ -449,7 +449,7 @@ case "$target" in #error no need for long double compatibility #endif ], [ac_ldbl_compat=yes], [ac_ldbl_compat=no]) - if test "$ac_ldbl_compat" = yes; then + AS_IF([test "$ac_ldbl_compat" = yes],[ AC_DEFINE([_GLIBCXX_LONG_DOUBLE_COMPAT],1, [Define if compatibility should be provided for -mlong-double-64.]) port_specific_symbol_files="\$(top_srcdir)/config/os/gnu-linux/ldbl-extra.ver" @@ -485,7 +485,7 @@ case "$target" in fi ;; esac - fi + ]) esac AC_SUBST(LONG_DOUBLE_COMPAT_FLAGS) AC_SUBST(LONG_DOUBLE_128_FLAGS)