libstdc++: Use AS_IF in configure.ac

Message ID 20230601155856.305565-1-jwakely@redhat.com
State Accepted
Headers
Series libstdc++: Use AS_IF in configure.ac |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Jonathan Wakely June 1, 2023, 3:58 p.m. UTC
  Tested x86_64-linux. I'd appreciate a second set of eyeballs on this
before I push it.

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

Comments

Jonathan Wakely June 6, 2023, 3:30 p.m. UTC | #1
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
>
>
  
Hans-Peter Nilsson June 7, 2023, 2:42 p.m. UTC | #2
> 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
  
Jonathan Wakely June 7, 2023, 2:54 p.m. UTC | #3
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.
  
Jonathan Wakely June 7, 2023, 2:56 p.m. UTC | #4
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
  
Andreas Schwab June 7, 2023, 3:18 p.m. UTC | #5
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.
  
Jonathan Wakely June 7, 2023, 3:57 p.m. UTC | #6
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.
  

Patch

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)